Skip to content

Fix MAS0004 stopping at the first unrelated CA1507 diagnostic - #1340

Merged
meziantou merged 3 commits into
mainfrom
feature/meziantou-analyzer-1337-5fa264
Aug 26, 2026
Merged

meziantou merged 3 commits into
mainfrom
feature/meziantou-analyzer-1337-5fa264

Conversation

@meziantou

Copy link
Copy Markdown
Owner

Fixes #1337

What changed

CA1507SerializationPropertyNameSuppressor.ReportSuppressions iterates context.ReportedDiagnostics, but the three "not applicable" guards used return instead of continue.

ReportedDiagnostics contains every CA1507 of the compilation, and the vast majority are not inside an attribute — so node.FirstAncestorOrSelf<AttributeSyntax>() was null and the method exited before ever reaching the [Newtonsoft.Json.JsonProperty] one. MAS0004 was effectively non-functional in any real project, and whether it worked at all depended on the ordering of ReportedDiagnostics, which is not guaranteed.

  • The three return statements are now continue.
  • GetBestTypeByMetadataName("Newtonsoft.Json.JsonPropertyAttribute") is hoisted out of the loop, with an early bail when Newtonsoft.Json is not referenced.

Test

CA1507_NewtonsoftJson_JsonPropertyName_AfterAnUnrelatedDiagnostic puts two CA1507 diagnostics in one file with the attribute one second: the first stays reported, the second must be suppressed. The three existing tests each contained exactly one CA1507, which is why the bug was invisible to the suite.

Notes for reviewers

  • The new test was confirmed to fail against the pre-fix code (CA1507 reported on "Bar") and to pass with the fix.
  • Suppressor tests pass on roslyn4.14, 5.0, 5.6 and 5.9 (14/14 each). roslyn4.8 runs zero — all three suppressor test files are guarded by #if ROSLYN_4_10_OR_GREATER, which is pre-existing.
  • dotnet run --project src/DocumentationGenerator exits 0 with no markdown changes.
  • The other two suppressors do not have this bug: CA1822DecoratedMethodSuppressor delegates the per-diagnostic work to a helper method, and IDE0058Suppressor already uses continue.

ReportSuppressions iterated context.ReportedDiagnostics but used return
instead of continue for the three "not applicable" cases. Since
ReportedDiagnostics contains every CA1507 of the compilation and most of
them are not inside an attribute, the loop exited before ever reaching
the [Newtonsoft.Json.JsonProperty] one, making the suppressor
effectively non-functional in any real project.

Also hoist the GetBestTypeByMetadataName lookup out of the loop and bail
early when Newtonsoft.Json is not referenced.

Add a test with two CA1507 diagnostics in one file, the attribute one
second, which reproduces the bug.

Fixes #1337
Only look up Newtonsoft.Json.JsonPropertyAttribute once a CA1507
diagnostic is actually located inside an attribute, so compilations
without such a diagnostic never load the symbol.
@meziantou
meziantou enabled auto-merge (squash) August 26, 2026 18:14
@meziantou
meziantou merged commit 240ac10 into main Aug 26, 2026
13 checks passed
@meziantou
meziantou deleted the feature/meziantou-analyzer-1337-5fa264 branch August 26, 2026 18:18
This was referenced Aug 26, 2026
This was referenced Sep 21, 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.

CA1507 suppressor (MAS0004) uses return instead of continue, so it stops at the first unrelated diagnostic

1 participant