Remove NoWarn for NU1511 false positives

[akoeplinger]

The upstream nuget issue was fixed, remove the warning suppressions.

Also unify the comments for the remaining genuine NU1511 suppressions so that we use the same text everywhere.
In some cases we were able to remove the suppression by making a P2P reference .NETFramework only, since the assembly was moved to the base shared framework in .NET 11.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR removes now-obsolete NU1511 suppressions (previously used for false positives) and standardizes the rationale comments for the remaining intentional NU1511 suppressions. It also avoids NU1511 in a few cases by restricting inbox project-to-project (P2P) references to .NET Framework only.

Changes:

• Removed NU1511 NoWarn entries where the warning is no longer expected (false-positive cases).
• Adjusted a few test projects to only P2P-reference inbox libraries on $(TargetFrameworkIdentifier) == '.NETFramework'.
• Unified the suppression comments for projects that still intentionally suppress NU1511.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj	Drops NU1511 suppression; makes the P2P to the library .NETFramework-only.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/System.Text.Json.SourceGeneration.Tests.targets	Removes NU1511 suppression and reshapes P2P references for .NETFramework only.
src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj	Drops NU1511 suppression; makes the P2P to DiagnosticSource .NETFramework-only.
src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricOuterLoopTests/MetricOuterLoop1.Tests.csproj	Drops NU1511 suppression; makes the P2P to DiagnosticSource .NETFramework-only.
src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricOuterLoopTests/MetricOuterLoop.Tests.csproj	Drops NU1511 suppression; makes the P2P to DiagnosticSource .NETFramework-only.
src/libraries/Microsoft.Extensions.Logging/tests/Common/Microsoft.Extensions.Logging.Tests.csproj	Adds standardized NU1511 suppression rationale comment.
src/libraries/Microsoft.Extensions.Hosting.Abstractions/tests/Microsoft.Extensions.Hosting.Abstractions.Tests.csproj	Updates to standardized NU1511 suppression rationale comment.
src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/Microsoft.Extensions.DependencyInjection.Tests.csproj	Adds standardized NU1511 suppression rationale comment.
src/installer/tests/TestUtils/TestUtils.csproj	Removes NU1511 suppression previously used for a transitive-P2P false positive.
src/installer/tests/Microsoft.NET.HostModel.Tests/Microsoft.NET.HostModel.Tests.csproj	Removes NU1511 suppression previously used for a transitive-P2P false positive.
src/installer/tests/Microsoft.DotNet.CoreSetup.Packaging.Tests/Microsoft.DotNet.CoreSetup.Packaging.Tests.csproj	Removes NU1511 suppression previously used for a transitive-P2P false positive.
src/installer/tests/HostActivation.Tests/HostActivation.Tests.csproj	Removes NU1511 suppression previously used for a transitive-P2P false positive.
src/installer/tests/AppHost.Bundle.Tests/AppHost.Bundle.Tests.csproj	Removes NU1511 suppression previously used for a transitive-P2P false positive.

[github-actions]

Copilot Code Review

Holistic Assessment

Motivation: Justified cleanup. The upstream NuGet bug (NuGet/Home#14103) producing false-positive NU1511 warnings has been fixed, and the workaround suppressions are now dead weight. Unifying comments and narrowing P2P references to .NETFramework-only where the assembly moved to the shared framework in .NET 11 is also sound.

Approach: The right approach — remove suppressions that are no longer needed, make P2P references conditional where appropriate, and standardize the remaining genuine suppressions with consistent comments.

Summary: ✅ LGTM. The changes are straightforward infrastructure cleanup that correctly narrows workarounds to only the scenarios that still need them.

---

Detailed Findings

Detailed Findings

✅ NU1511 suppression removal — Correct for false positives

The 5 installer test projects (AppHost.Bundle.Tests, HostActivation.Tests, Microsoft.DotNet.CoreSetup.Packaging.Tests, Microsoft.NET.HostModel.Tests, TestUtils) and the System.Text.Json.SourceGeneration.Tests.targets all had NU1511 suppressions referencing the now-fixed NuGet bug. Removing them is correct.

✅ P2P references made .NETFramework-only — Correct approach

For System.Diagnostics.DiagnosticSource (3 test projects), System.Threading.AccessControl, and System.Text.Json source generation tests, making the P2P references conditional on '$(TargetFrameworkIdentifier)' == '.NETFramework' is the right pattern. These assemblies are part of the shared framework on .NET Core/11+, so the P2P is only needed for .NETFramework where you need the implementation directly.

✅ Comment unification — Good consistency improvement

The remaining genuine NU1511 suppressions (DI.Tests, Hosting.Abstractions.Tests, Logging.Tests) now all use the same two-line comment explaining why the suppression exists and linking to the outstanding NuGet issue (NuGet/Home#14121).

💡 xUnit1004 suppression also removed — Verify this is intentional

AppHost.Bundle.Tests.csproj and Microsoft.NET.HostModel.Tests.csproj had <NoWarn>$(NoWarn);NU1511;xUnit1004</NoWarn>. Removing the entire line also removes the xUnit1004 suppression. SigningTests.cs in the HostModel tests has [Theory(Skip = "Temporarily disabled...")] which could trigger xUnit1004. The CET test in AppHost uses xUnit v3's conditional skip (SkipType/SkipUnless) and likely wouldn't trigger it. Since TreatWarningsAsErrors is enabled repo-wide, an unconditional Skip triggering xUnit1004 would become a build error. This may be intentional (to surface that the skip should be resolved), but worth confirming.

Note

This review was generated by GitHub Copilot.

Generated by Code Review for issue #130033 · ● 26.8M · ◷