fix(analyzers): remove incorrect AOT rules TUnit0300/0301/0302 (#5722)#5746
fix(analyzers): remove incorrect AOT rules TUnit0300/0301/0302 (#5722)#5746
Conversation
…301/0302 (#5722) These rules produced false positives even outside AOT contexts and the underlying claims (generics in InheritsTests, ValueTuple, conversion ops) do not actually break AOT.
There was a problem hiding this comment.
Code Review
This is a clean, well-executed removal of three inaccurate Roslyn analyzer rules (, , ). The PR is straightforward and the rationale is solid.
What's Done Well
Roslyn release-tracking conventions are correctly followed. Adding "Removed Rules" entries to AnalyzerReleases.Unshipped.md is required for NuGet analyzer packages so that tooling can warn consumers about suppressed rule IDs that no longer exist. This is the right approach.
Complete cleanup. All four artifact types are removed together: the analyzer class, its tests, the DiagnosticDescriptor fields in Rules.cs, and the Resources.resx strings. No dangling references remain.
The dead code identified in the issue is correctly characterized.
TUnit0301was guarded by#if !NET, making it a no-op on every modern .NET target — a firing analyzer that literally never fired.TUnit0302had resource strings but noDiagnosticDescriptorand no detection logic — a ghost rule.HasAotCompatibleAttributereferenced an attribute (AotCompatibleAttribute) that TUnit never defined, so it would always returnfalse.
Minor Observations (Non-blocking)
1. Check AnalyzerReleases.Shipped.md for prior entries.
If these rules were ever included in a prior NuGet release, they should appear in Shipped.md with a status of Removed or similar, not just Unshipped.md. If they were only ever in pre-release builds that never shipped, the current approach is fine. Worth a quick sanity-check to confirm the release history is consistent.
2. Consumers with #pragma warning disable TUnit0300/0301/0302 will get a new warning.
Once this ships, anyone who added a suppression to silence these false positives will see CS8032/similar "unrecognized diagnostic ID" warnings. This is unavoidable and is the correct behavior (suppressed dead rules should be cleaned up), but a brief release note or mention in the PR description would help users know what to do. Not a blocker.
Verdict
No issues with the implementation. This is a pure subtraction with no behavioral regressions — the rules were either never firing or firing incorrectly. Removing them is the right call. The 4 additions / 292 deletions ratio speaks for itself.
There was a problem hiding this comment.
Code Review
This is a clean, well-executed removal of three inaccurate Roslyn analyzer rules (TUnit0300, TUnit0301, TUnit0302). The PR is straightforward and the rationale is solid.
What's Done Well
Roslyn release-tracking conventions are correctly followed. Adding "Removed Rules" entries to AnalyzerReleases.Unshipped.md is required for NuGet analyzer packages so that tooling can warn consumers about suppressed rule IDs that no longer exist. This is the right approach.
Complete cleanup. All four artifact types are removed together: the analyzer class, its tests, the DiagnosticDescriptor fields in Rules.cs, and the Resources.resx strings. No dangling references remain.
The dead code identified in the issue is correctly characterized.
TUnit0301was guarded by#if !NET, making it a no-op on every modern .NET target — an analyzer that literally never fired.TUnit0302had resource strings but noDiagnosticDescriptorand no detection logic — a ghost rule.HasAotCompatibleAttributereferenced an attribute (AotCompatibleAttribute) that TUnit never defined, so it would always returnfalse.
Minor Observations (Non-blocking)
1. Check AnalyzerReleases.Shipped.md for prior entries.
If these rules were included in a prior NuGet release, they should also appear in Shipped.md marked as Removed. If they were only ever in pre-release builds that never shipped, the current approach is fine. Worth a quick sanity-check to confirm the release history is consistent.
2. Consumers with #pragma warning disable TUnit0300/0301/0302 will get a new warning.
Once this ships, anyone who added a suppression to silence these false positives will see "unrecognized diagnostic ID" warnings. This is correct behavior (dead suppressions should be cleaned up), but a brief release note or mention in the changelog would help users know what to do. Not a blocker.
Verdict
No issues with the implementation. This is a pure subtraction with no behavioral regressions — the rules were either never firing or firing incorrectly. Removing them is the right call. The 4 additions / 292 deletions ratio speaks for itself.
Up to standards ✅🟢 Issues
|
Closes #5722
Summary
TUnit0300,TUnit0301,TUnit0302— issue documents all three are inaccurate (LLM-hallucinated) and fire even when AOT is not enabled.AnalyzerReleases.Unshipped.mdwithRemoved Rulesentries per Roslyn release-tracking conventions.AotCompatibilityAnalyzerand its tests; removes corresponding strings fromResources.resxand descriptors fromRules.cs.Test plan