NuGet.Packaging nullability - Signing APIs (#7219)

Author: nkolev92

.github/copilot-instructions.md

@@ -14,3 +14,15 @@
 ## Project-Specific Rules
 
 - All files in the repository are nullable by default (project-level nullable enable). No need to add `#nullable enable` directives to individual files.
+
+## Nullable Migration Rules
+
+- **Shipped.txt format must be precise** — e.g. `string![]!` not `string![]`, `byte[]?` not `byte?[]`. Always match the format of existing base class entries in the same file.
+- **`~` (oblivious) entries get replaced in place** — replace the `~` prefixed line with the annotated line in `PublicAPI.Shipped.txt`. Do not add to `PublicAPI.Unshipped.txt`.
+- **Internal types don't need Shipped.txt updates** — only public API surfaces require `PublicAPI.Shipped.txt` changes.
+- **Don't suppress nullability with `!`** when the value genuinely can be null — make the type honest, let callers handle it.
+- **Covariant return nullability** — `byte[]` override of `byte[]?` base is valid in C# 9+. Use it when a subclass guarantees non-null.
+- **`Debug.Assert(x != null)` + `x!` can be replaced** by removing both when the parameter is non-null typed and all callers are nullable-enabled.
+- **`required` on private/internal types** is cleaner than `null!` field initializers.
+- **TryCreate/TryGet patterns** — out params need `?`, callers use `!` after the success guard. Out parameters that are guaranteed non-null when the method returns true should be annotated with `[NotNullWhen(true)]`. Don't annotate `[NotNullWhen]` unless it's actually true for all code paths.
+- **Work in batches** — group related files, fix source, fix cascading, build, repeat. If this means we need multiple pull requests for enabling nullable, that's fine. Don't try to do it all in one go.

src/NuGet.Core/NuGet.Packaging/PackageExtraction/PackageHelper.cs

@@ -172,7 +172,7 @@ public static async Task<IEnumerable<ZipFilePair>> GetInstalledPackageFilesAsync
             if (!string.IsNullOrEmpty(packageDirectory))
             {
                 var packageFiles = await packageReader.GetPackageFilesAsync(packageSaveMode, cancellationToken);
-                var entries = packageReader.EnumeratePackageEntries(packageFiles, packageDirectory);
+                var entries = packageReader.EnumeratePackageEntries(packageFiles, packageDirectory!);
                 installedPackageFiles = entries.Where(e => e.IsInstalled());
             }
 

src/NuGet.Core/NuGet.Packaging/PackageExtractor.cs

@@ -946,7 +946,7 @@ public static async Task<IEnumerable<string>> CopySatelliteFilesAsync(
             var nupkgFilePath = packagePathResolver.GetInstalledPackageFilePath(packageIdentity);
             if (File.Exists(nupkgFilePath))
             {
-                using (var packageReader = new PackageArchiveReader(nupkgFilePath))
+                using (var packageReader = new PackageArchiveReader(nupkgFilePath!))
                 {
                     return await CopySatelliteFilesAsync(
                         packageReader,

src/NuGet.Core/NuGet.Packaging/PackagePathResolver.cs

@@ -1,8 +1,6 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-#nullable disable
-
 using System;
 using System.Globalization;
 using System.IO;
@@ -87,14 +85,14 @@ public virtual string GetInstallPath(PackageIdentity packageIdentity)
             return Path.Combine(_rootDirectory, GetPackageDirectoryName(packageIdentity));
         }
 
-        public virtual string GetInstalledPath(PackageIdentity packageIdentity)
+        public virtual string? GetInstalledPath(PackageIdentity packageIdentity)
         {
             var installedPackageFilePath = GetInstalledPackageFilePath(packageIdentity);
 
             return string.IsNullOrEmpty(installedPackageFilePath) ? null : Path.GetDirectoryName(installedPackageFilePath);
         }
 
-        public virtual string GetInstalledPackageFilePath(PackageIdentity packageIdentity)
+        public virtual string? GetInstalledPackageFilePath(PackageIdentity packageIdentity)
         {
             return PackagePathHelper.GetInstalledPackageFilePath(packageIdentity, this);
         }

src/NuGet.Core/NuGet.Packaging/PublicAPI/net472/PublicAPI.Shipped.txt

@@ -1,8 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-#nullable disable
-
+using System.Diagnostics.CodeAnalysis;
 using System.IO;
 
 // ZIP specification: http://www.pkware.com/documents/casestudies/APPNOTE.TXT
@@ -31,9 +30,9 @@ internal sealed class CentralDirectoryHeader
         internal ushort InternalFileAttributes { get; private set; }
         internal uint ExternalFileAttributes { get; private set; }
         internal uint RelativeOffsetOfLocalHeader { get; private set; }
-        internal byte[] FileName { get; private set; }
-        internal byte[] ExtraField { get; private set; }
-        internal byte[] FileComment { get; private set; }
+        internal byte[] FileName { get; private set; } = null!;
+        internal byte[] ExtraField { get; private set; } = null!;
+        internal byte[] FileComment { get; private set; } = null!;
 
         // This property is not part of the ZIP specification.
         internal long OffsetFromStart { get; private set; }
@@ -46,7 +45,7 @@ internal uint GetSizeInBytes()
                 FileCommentLength;
         }
 
-        internal static bool TryRead(BinaryReader reader, out CentralDirectoryHeader header)
+        internal static bool TryRead(BinaryReader reader, [NotNullWhen(returnValue: true)] out CentralDirectoryHeader? header)
         {
             header = null;
 

src/NuGet.Core/NuGet.Packaging/PublicAPI/net8.0/PublicAPI.Shipped.txt

@@ -2,8 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
 using System;
 
 namespace NuGet.Packaging.Signing

src/NuGet.Core/NuGet.Packaging/Signing/Archive/CentralDirectoryHeader.cs

@@ -1,8 +1,6 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-#nullable disable
-
 using System;
 using System.Collections.Generic;
 using System.Globalization;
@@ -24,7 +22,7 @@ internal sealed class EndOfCentralDirectoryRecord
         internal uint SizeOfCentralDirectory { get; private set; }
         internal uint OffsetOfStartOfCentralDirectory { get; private set; }
         internal ushort FileCommentLength { get; private set; }
-        internal byte[] FileComment { get; private set; }
+        internal byte[] FileComment { get; private set; } = null!; // Set to non-null in the static Read method.
 
         // This property is not part of the ZIP specification.
         internal long OffsetFromStart { get; private set; }

src/NuGet.Core/NuGet.Packaging/Signing/Archive/Crc32.cs

@@ -1,9 +1,8 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-#nullable disable
-
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.IO;
 
 // ZIP specification: http://www.pkware.com/documents/casestudies/APPNOTE.TXT
@@ -23,7 +22,7 @@ protected ExtraField(ushort headerId, ushort dataSize, byte[] data)
             Data = data;
         }
 
-        internal static bool TryRead(CentralDirectoryHeader header, out IReadOnlyList<ExtraField> extraFields)
+        internal static bool TryRead(CentralDirectoryHeader header, [NotNullWhen(returnValue: true)] out IReadOnlyList<ExtraField>? extraFields)
         {
             extraFields = null;
 
@@ -46,7 +45,7 @@ internal static bool TryRead(CentralDirectoryHeader header, out IReadOnlyList<Ex
                 out extraFields);
         }
 
-        internal static bool TryRead(LocalFileHeader header, out IReadOnlyList<ExtraField> extraFields)
+        internal static bool TryRead(LocalFileHeader header, [NotNullWhen(returnValue: true)] out IReadOnlyList<ExtraField>? extraFields)
         {
             extraFields = null;