Add NU5052 pack warning for non-restricted package IDs#7487
Open
nkolev92 wants to merge 8 commits into
Open
Conversation
Emit NU5052 warning during pack when a package ID does not adhere to the restricted character set (ASCII letters, digits, dots, dashes, underscores with no consecutive dots or dashes). The warning is gated by SdkAnalysisLevel >= 11.0.100 using SdkAnalysisLevelMinimums.IsEnabled(). - Add PackageIdValidator.IsRestrictedPackageId() with restricted regex - Emit NU5052 in PackCommandRunner.BuildPackage() before builder.Save() - Plumb SdkAnalysisLevel/UsingMicrosoftNETSdk from MSBuild targets through PackTask/PackTaskLogic to PackArgs - Add unit tests for IsRestrictedPackageId and integration tests for NU5052 emission with SdkAnalysisLevel gating Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the separate IsRestrictedPackageId() method and its regex with an IsValidPackageId(string, bool useRestrictedCharacterSet) overload that reuses the existing IdRegex and adds a fast char-loop for the restricted character check. Benchmarks showed the regex approach was 4x slower than baseline while the char-loop adds only ~15% overhead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename NonRestrictedPackageId to PackageIdWithInvalidCharacters for clarity. Consolidate the four SdkAnalysisLevel gating tests into a single Theory with MemberData. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace MemberData with InlineData for the gating theory. Add tests verifying that NoWarn suppresses NU5052 and TreatWarningsAsErrors elevates it to an error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merge the two separate NU5052 theories into one with all dimensions (packageId, sdkAnalysisLevel, usingMicrosoftNETSdk, expectWarning). Remove restricted PackageIdValidator tests that duplicate existing IsValidPackageId tests (consecutive separators, invalid start char, null). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Verify that dotnet pack with a non-ASCII PackageId emits NU5052 on .NET 11 (SDK_NEXT) and does not on .NET 10. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Only emit the restricted package ID warning (NU5052) when UsingMicrosoftNETSdk is true. Non-SDK projects (nuget.exe pack) do not emit this warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jeffkl
previously approved these changes
Jun 18, 2026
jeffkl
left a comment
jeffkl
left a comment
Contributor
There was a problem hiding this comment.
The language of the warning can be improved, I put my suggestion in the hat for consideration.
Member
Author
|
@copilot address all feedback. |
Contributor
Addressed feedback in commit f636a1e:
|
martinrrm
approved these changes
Jun 22, 2026
jebriede
approved these changes
Jun 23, 2026
jebriede
left a comment
jebriede
left a comment
Contributor
There was a problem hiding this comment.
Approved with suggestion.
| Directory.CreateDirectory(Path.GetDirectoryName(outputPath)); | ||
|
|
||
| // Warn if the package ID doesn't adhere to the restricted character set (NU5052) | ||
| if (!symbolsPackage && _packArgs.UsingMicrosoftNETSdk && !PackageIdValidator.IsValidPackageId(builder.Id, useRestrictedCharacterSet: true)) |
Contributor
There was a problem hiding this comment.
nit: IsValidPackageId runs before the cheaper IsEnabled gate, so we validate even
when the warning is suppressed. Checking IsEnabled first would skip that work and let
the two nested ifs collapse into one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
Progress: NuGet/Home#14949
Description
Design at NuGet/Home#14950.
I am marking this PR as ready for review as the main framework of the PR has been approved.
The only open questions are around whether we put this in 10.0.400 or whether 11.0.100 is good enough.
Also as of right now, we're not enabling this for non SDK projects, so the open question around VS is not implemented. Once those are done, we can make tweaks either in this or a future PR.
Implements pack warning NU5052 when a package ID does not adhere to the restricted character set. Package IDs must start with a letter, digit, or underscore and contain only ASCII letters, digits, dots (
.), dashes (-), and underscores (_) with no consecutive dots or dashes. The warning is gated bySdkAnalysisLevel >= 11.0.100.Design decisions
Char-loop over regex for restricted ID validation. The regex from the announcement (
^[A-Za-z0-9_](?!.*[.-]{2})[A-Za-z0-9.-]{0,99}$) was attempted, but benchmarking against ~1,979 real package IDs showed the negative lookahead made it 4x slower than the existingIdRegexbaseline:Since
IdRegexalready runs during pack validation, the newIsValidPackageId(string, bool useRestrictedCharacterSet)overload reuses that result and adds only a char-loop — +15% overhead instead of 4x.Copilot instructions updated. Added guidance to
docs/so that SdkAnalysisLevel gating is done correctly viaSdkAnalysisLevelMinimums.IsEnabled()in code.Public API
PackageIdValidator.IsValidPackageId(string packageId, bool useRestrictedCharacterSet)— new overload onNuGet.PackagingPR Checklist