Skip to content

Cover the untested analyzer and fixer paths and remove the dead ones - #1436

Merged
meziantou merged 1 commit into
mainfrom
feature/analyzer-fixer-test-coverage-079540
Sep 7, 2026
Merged

meziantou merged 1 commit into
mainfrom
feature/analyzer-fixer-test-coverage-079540

Conversation

@meziantou

Copy link
Copy Markdown
Owner

Running the test suite with code coverage surfaced 16 rule files containing methods that no test ever executed. Each one turned out to be either a shipping behavior with no regression protection, or code that cannot be reached at all. This PR covers the former and deletes the latter.

Tests added

Rule Path that had no test
MA0168 The RegisterSymbolAction(SymbolKind.Parameter) handler. Every existing test declared a top-level local function, so in / ref readonly parameters on methods, constructors, indexers and delegates were never analyzed.
MA0182 An internal type used only as an array element type (AnalyzeArrayCreation never ran).
MA0062, MA0192, MA0099 All eight enum underlying types. Only int was covered, leaving the sbyte/short/ushort/uint/long/ulong arms of the bit-set and zero checks cold.
MA0040, MA0166 The nested member path (context.Request.RequestAborted).
MA0106 The AddOrUpdate code fix, which was registered but never applied by any test.
MA0105 All four AddOrUpdate and GetOrAdd parameter mappings.
MA0020, MA0031 LongCount() → LongLength, and Count(predicate) → Any(predicate).
MA0004, MA0008, MA0161 The second offered code action (ConfigureAwait(true), LayoutKind.Sequential, UseShellExecute = true).
MA0214 A lambda and an anonymous method inside the rewritten body, which the rewriter must not descend into.
MA0213 Flipping the relational operators (< → >=, …). Only == / != were covered.
MA0179 The length < 1 and length <= 0 comparisons.
MA0051 Property accessors, constructors and destructors — registered but never exercised.
MA0118 An explicit interface implementation, which correctly offers no code fix.

Bug fixed

UseRegexExplicitCaptureOptionsFixer matched NameEquals (a property or field initializer) against the constructor parameters, so an attribute written with named arguments had RegexOptions.ExplicitCapture appended to the wrong argument:

// before
[GeneratedRegex(options: RegexOptions.CultureInvariant, pattern: "([a-z]+)", matchTimeoutMilliseconds: -1)]
// fixed to (does not compile)
[GeneratedRegex(options: RegexOptions.CultureInvariant, pattern: "([a-z]+)" | RegexOptions.ExplicitCapture, matchTimeoutMilliseconds: -1)]

It now matches NameColon, with a regression test.

Unreachable code removed

  • EqualityShouldBeCorrectlyImplementedAnalyzer: HasMethodInHierarchy, IsEqualsMethod and IsCompareToMethod had no caller.
  • UseAnOverloadThatHasCancellationTokenAnalyzer: GetContainingType had no caller.
  • Both overload analyzers: the null-name handling of NameAndType, as the name always comes from ISymbol.Name. This also removed a per-diagnostic IsInStaticContext call that existed only to feed that dead check, and two usings that became unused.
  • OptimizeStringBuilderUsage: ReplaceToStringWithAppendFormat and its fix method — the analyzer has never reported that value.
  • MergeIsPatternChecksFixer: the negated and binary pattern cases plus TryGetPatternOperator. A not / and / or pattern always has a PatternSyntax, so the early return wins (confirmed by coverage on both Roslyn 4.8 and 5.9).
  • ValidateUnsafeAccessorAttributeUsageFixer: the method and fallback enumeration, as MA0146 is only reported for MethodKind.LocalFunction. The local function lookup also moved into RegisterCodeFixesAsync, per the validate-before-registering guidance in AGENTS.md.
  • JSInvokableMethodsMustBePublicFixer: the GetEnclosingSymbol lookup and the last fallback, along with the semantic model they needed.
  • DoNotUseZeroToInitializeAnEnumValueFixer: the four fallbacks of GetTargetEnumType, as the converted type resolves every reported position (the new tests cover each of those syntactic positions).

For the reviewer

Two things were deliberately left alone:

  1. RemoveUnnecessaryBracesInTypeDeclarationFixer.ContainsCommentOrDirectiveInBraces is the single remaining never-executed lambda. It duplicates the analyzer's own guard, which is what the fixer guidance asks for, so it is unreachable by design and was kept.
  2. UseAttributeIsDefined reversed comparisons look like an unfinished feature rather than dead code. IsValidLengthComparisonPattern has a fully written lengthIsOnLeft: false branch (0 == length, 1 > length, 0 >= length, …) in both the analyzer and the fixer, but the analyzer only ever passes lengthIsOnLeft: true and only inspects the left operand, so 0 == x.Length reports nothing today. Tests for those six shapes were written, observed to fail, and then removed rather than deleting the branches: whether to wire the feature up or drop it is a product decision.

Verification

  • dotnet build on the whole solution: succeeds with no warnings.
  • dotnet test --max-parallel-test-modules 2: 20,554 tests pass across Roslyn 4.8, 4.14, 5.0, 5.6 and 5.9 (the default project goes from 4,079 to 4,169 tests).
  • dotnet run --project src/DocumentationGenerator: exits 0, no markdown changes.
  • Coverage of Rules/: 90.6% → 91.8% lines, 78.1% → 79.7% branches; rule files with a never-executed method drop from 16 to 1.

Running the test suite with code coverage surfaced 16 rule files containing
methods that no test ever executed. Each one was either a behavior with no
regression protection or code that cannot be reached at all.

Add the missing tests:

- MA0168: the symbol action for parameters, as every test declared a top-level
  local function, leaving methods, constructors, indexers and delegates untested
- MA0182: an internal type used only as an array element type
- MA0062, MA0192 and MA0099: every enum underlying type, as only int was covered
- MA0040 and MA0166: the nested member path (context.Request.RequestAborted)
- MA0106: the AddOrUpdate code fix, and MA0105: the four AddOrUpdate and
  GetOrAdd parameter mappings, none of which was ever applied
- MA0020: LongCount() to LongLength, and MA0031: Count(predicate) to Any(predicate)
- MA0004, MA0008 and MA0161: the second offered code action
- MA0214: a lambda and an anonymous method in the rewritten body
- MA0213: flipping the relational operators
- MA0179: the length < 1 and length <= 0 comparisons
- MA0051: property accessors, constructors and destructors
- MA0118: an explicit interface implementation, which offers no code fix

Fix MA0023: the fixer matched NameEquals, which is a property initializer,
against the constructor parameters, so an attribute using named arguments had
RegexOptions.ExplicitCapture appended to the wrong argument, producing code that
does not compile.

Remove the code the coverage proved unreachable:

- EqualityShouldBeCorrectlyImplementedAnalyzer: three methods with no caller
- UseAnOverloadThatHasCancellationTokenAnalyzer: GetContainingType, with no caller
- Both overload analyzers: the null name handling of NameAndType, as the name
  always comes from ISymbol.Name, and with it the IsInStaticContext call that
  only fed that check
- OptimizeStringBuilderUsage: ReplaceToStringWithAppendFormat, which the analyzer
  has never reported
- MergeIsPatternChecksFixer: the negated and binary pattern cases, as those
  patterns always have a PatternSyntax and return earlier
- ValidateUnsafeAccessorAttributeUsageFixer: the method and fallback enumeration,
  as MA0146 only reports local functions
- JSInvokableMethodsMustBePublicFixer: the symbol lookup and the last fallback
- DoNotUseZeroToInitializeAnEnumValueFixer: the fallbacks of GetTargetEnumType,
  as the converted type resolves every reported position

Line coverage of the rules goes from 90.6% to 91.8% and branch coverage from
78.1% to 79.7%.
@meziantou
meziantou merged commit ce4d0d8 into main Sep 7, 2026
13 checks passed
@meziantou
meziantou deleted the feature/analyzer-fixer-test-coverage-079540 branch September 7, 2026 22:04
This was referenced Sep 7, 2026
This was referenced Sep 26, 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