Skip to content

Remove DisabledDiagnostics from the tests - #1370

Merged
meziantou merged 13 commits into
mainfrom
feature/disabled-diagnostics-cleanup-1d88c7
Sep 1, 2026
Merged

meziantou merged 13 commits into
mainfrom
feature/disabled-diagnostics-cleanup-1d88c7

Conversation

@meziantou

Copy link
Copy Markdown
Owner

Removes every DisabledDiagnostics usage from the tests (75 calls across 26 files, now 0). Most of them were leftovers from the migration to the Roslyn SDK harness: the original tests filtered the diagnostics to a single rule, so each rule of a multi-rule analyzer got its own test class that disabled all the others.

There is one commit per analyzer.

What changed

Merged the per-rule test classes (24 classes → 10). Each merged class now runs every rule its analyzer reports, so a snippet that triggers two rules has to say so:

Analyzer Was Now
OptimizeLinqUsageAnalyzer 9 classes OptimizeLinqUsageAnalyzerTests
EqualityShouldBeCorrectlyImplementedAnalyzer MA0077 / MA0094 / MA0095 / MA0097 EqualityShouldBeCorrectlyImplementedAnalyzerTests
DoNotUseBlockingCallInAsyncContextAnalyzer _AsyncContext / _NonAsyncContext DoNotUseBlockingCallInAsyncContextAnalyzerTests
MakeMethodStaticAnalyzer _Methods / _Properties MakeMethodStaticAnalyzerTests
AvoidClosureWhenUsingConcurrentDictionaryAnalyzer MA0105 / MA0106 ConcurrentDictionaryMustPreventClosureWhenAccessingTheKeyAnalyzerTests
OptionalParametersAttributeAnalyzer MA0087 / MA0088 OptionalParametersAttributeAnalyzerTests
UseStringComparisonAnalyzer MA0001 / MA0074 UseStringComparisonAnalyzerTests
DoNotUseDefaultEqualsOnValueTypeAnalyzer MA0065 / MA0066 DoNotUseDefaultEqualsOnValueTypeAnalyzerTests
DoNotImplicitlyConvertDateTimeToDateTimeOffsetAnalyzer MA0132 / MA0133 DoNotCompareDateTimeWithDateTimeOffsetAnalyzerTests
ArgumentExceptionShouldSpecifyArgumentNameAnalyzer MA0043 in its own class ArgumentExceptionShouldSpecifyArgumentNameAnalyzerTests

Colliding method names got a disambiguating prefix or suffix. Six tests that were byte-identical duplicates across a merged pair were dropped (one in the DateTimeOffset pair, five in the UseStringComparison pair) — no coverage is lost, the surviving copy asserts exactly the same thing.

Flagged the diagnostics the fix reveals, using the pattern already in the repo (FixedState.MarkupHandling = MarkupMode.Allow + CodeFixTestBehaviors.FixOne), so the test asserts a single fix application and the new diagnostic is visible in the expected code:

  • fixing MA0088 reveals MA0087
  • fixing MA0077 adds IEquatable<T>, which makes MA0095 report on the result (8 tests)
  • MA0156 and MA0157 contradict each other, so fixing one reveals the other

Flagged the other rules in the test code where a snippet reported more than the rule under test: UseLangwordInXmlComment (MA0154 / MA0218 / MA0219), UseStringComparison (the char overload tests now flag MA0001 and assert its fix), one DoNotUseBlockingCallInAsyncContext snippet (MA0045), and three AvoidClosureWhenUsingConcurrentDictionary snippets (MA0106).

Removed the disabling that was pure noise: ProcessStartAnalyzer disabled the two rules each test did not cover, and ArgumentExceptionShouldSpecifyArgumentName disabled MA0043, even though no snippet reported them.

Note for the reviewer

Three OptimizeLinqUsage order tests reported MA0159 only because their samples used an identity selector (OrderBy(x => x)). Flagging MA0159 there was not an option: with FixOne, the harness applied the MA0159 fix instead of the one under test, rewriting the sample to Order() and destroying what the test verified. Those samples now use a real selector (x => -x, x => x.Length) so the tests stay focused on MA0030 and MA0063.

Validation

  • dotnet build on the solution: 0 warnings, 0 errors
  • All Roslyn versions pass: roslyn4.8 (3742), roslyn4.14 (3769), roslyn5.0 (3797), roslyn5.6 (3821), roslyn5.9 (3854)
  • Every commit was checked out in turn and the test project compiles at each one
  • dotnet run --project src/DocumentationGenerator produced no markdown changes

… stop disabling MA0043

The MA0043 tests lived in their own class and the other class disabled MA0043,
even though no test snippet reported it. Both rules of the analyzer are now
verified by a single class.
MA0105 and MA0106 had one test class each, disabling the other rule. The
snippets that report both rules now flag MA0106 explicitly.
The MA0133 class disabled MA0132 and held a single test that the MA0132 class
already covered verbatim.
The async and non-async context classes disabled each other's rule. Only one
snippet actually reported the other rule, and it now flags MA0045.
MA0065 and MA0066 had one test class each, disabling the other rule, even
though no snippet reported both.
The MA0077 class disabled MA0095 because fixing MA0077 adds IEquatable<T>,
which makes MA0095 report on the result. Those tests now assert a single fix
application and flag MA0095 in the fixed code.
MA0038 and MA0041 had one test class each, disabling the other rule.
…ix tests

MA0156 and MA0157 contradict each other, so the fix of one reveals the other.
The two code fix tests now assert a single fix application and flag the
contradicting rule in the fixed code.
Nine classes covered one rule each and disabled all the others. The order
related tests used identity selectors, which made MA0159 report on them; they
now use a real selector so each test stays focused on the rule it covers.
MA0087 and MA0088 had one test class each, disabling the other rule. Fixing
MA0088 reveals MA0087, so that test now asserts a single fix application and
flags MA0087 in the fixed code.
Every test disabled the two rules it did not cover, even though no snippet
reported more than one of MA0161, MA0162 and MA0163.
The snippets that report MA0154, MA0218 or MA0219 in addition to the rule
under test now flag them.
MA0001 and MA0074 had one test class each, disabling the other rule, with five
tests duplicated between them. The char overload tests now flag MA0001 and
assert its fix.
@meziantou
meziantou enabled auto-merge September 1, 2026 03:57
@meziantou
meziantou merged commit ff29aa5 into main Sep 1, 2026
13 checks passed
@meziantou
meziantou deleted the feature/disabled-diagnostics-cleanup-1d88c7 branch September 1, 2026 03:59
This was referenced Sep 25, 2026
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