Skip to content

NuGet.Protocol nullability v2#7285

Open
nkolev92 wants to merge 6 commits intodevfrom
dev-nkolev92-protocol2
Open

NuGet.Protocol nullability v2#7285
nkolev92 wants to merge 6 commits intodevfrom
dev-nkolev92-protocol2

Conversation

@nkolev92
Copy link
Copy Markdown
Member

@nkolev92 nkolev92 commented Apr 15, 2026

Bug

Progress: NuGet/Home#14851

Description

Continues the NuGet.Protocol nullable annotation work. Removes #nullable disable from 8 more files and annotates public API surfaces.

Files migrated

  • HttpSourceCacheContext.csRootTempFolder made string?
  • NullSourceCacheContext.cs – fixed WithRefreshCacheTrue()/Clone() to use Instance property instead of _instance field (avoids returning null before first access)
  • PackageDownloadContext.csDirectDownloadDirectory, ClientPolicyContext, PackageSourceMapping made nullable; directDownloadDirectory and packageSourceMappingConfiguration parameters made nullable
  • RemoteSourceDependencyInfo.cs – annotated as non-null; contentUri marked with NULL_INC (no runtime check added — see note below); fixed Equals(object) which was incorrectly casting to PackageDependencyInfo instead of RemoteSourceDependencyInfo
  • SourceCacheContext.csClone() now assigns _generatedTempFolder via field instead of through the GeneratedTempFolder property (which has side effects); Dispose uses captured local
  • SourcePackageDependencyInfo.csSource, DownloadUri, PackageHash made nullable (all optional per XML docs)
  • UserAgent.cs – removed #nullable disable (no annotation changes needed)
  • UserAgentStringBuilder.cs_vsInfo, _osInfo, _ciInfo fields and WithVisualStudioSKU parameter made nullable

NULL_INC note

RemoteSourceDependencyInfo.ContentUri is annotated as non-null (string!) in the public API, but no runtime ArgumentNullException guard is added in the constructor. This property maps to the packageContent field in the NuGet V3 registration API, which is required by protocol — so null is not expected in practice. However, since the previous code silently accepted null and we lack telemetry to be certain no caller passes it, a runtime throw is deferred. The property carries a NULL_INC XML remark to track this.

Bug fix

RemoteSourceDependencyInfo.Equals(object) was casting to PackageDependencyInfo instead of RemoteSourceDependencyInfo, meaning it always returned false for valid comparisons.

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@dotnet-policy-service dotnet-policy-service Bot added Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed and removed Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed labels Apr 22, 2026
nkolev92 and others added 4 commits April 24, 2026 18:16
…NC remark

Remove the newly-added ArgumentNullException guard for contentUri to avoid
introducing a new throw in a previously-permissive code path. The non-null
annotation is kept but marked with NULL_INC for future telemetry-guided follow-up.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Covers annotation philosophy (non-null bias), PublicAPI.Shipped.txt update
rules, NULL_INC escape hatch criteria, and step-by-step migration workflow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ContentUri maps to the required packageContent field in the NuGet V3
registration API, so null is not expected in practice.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nkolev92 nkolev92 force-pushed the dev-nkolev92-protocol2 branch from 264bd08 to 39a08fd Compare April 25, 2026 01:16
@nkolev92 nkolev92 marked this pull request as ready for review April 25, 2026 01:16
@nkolev92 nkolev92 requested a review from a team as a code owner April 25, 2026 01:16
@nkolev92 nkolev92 requested review from jeffkl and kartheekp-ms April 25, 2026 01:16
nkolev92 and others added 2 commits April 24, 2026 18:27
Add null-forgiving operator on DownloadUri access in test assertions where
the value is known non-null from the test setup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…stead of null

The dependencies parameter is non-null after nullable enablement. These tests
don't use dependencies, so Enumerable.Empty<PackageDependency>() is correct.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant