Only the compiler analyzer can use CS or BC prefix for diagnostics #23776
Conversation
jcouv
added
Area-IDE
PR For Personal Review Only
jcouv
force-pushed
the
nameof-bug
branch
2 times, most recently
from
e1a8d9d
to
bc5462f
Compare
jcouv
removed
the
PR For Personal Review Only
| private static bool IsCompilerReservedDiagnostic(string id) | ||
| { | ||
| // Only the compiler analyzer should produce diagnostics with CS or BC prefixes (followed by digit) | ||
| if (id.Length >= 3 && (id.StartsWith("CS", StringComparison.Ordinal) || id.StartsWith("BC", StringComparison.Ordinal))) |
There was a problem hiding this comment.
Would it be better to instead check for presence of custom tag WellKnownDiagnosticTags.Compiler on the diagnostic descriptor?
There was a problem hiding this comment.
It feels like this tag would only be set by the compiler, and so would not help catch regular analyzers misbehaving.
I can double-check though.
There was a problem hiding this comment.
Ah yes, that is correct. We should probably add to your current check and disallow that tag on any analyzer diagnostic. Only compiler diagnostics should use that tag.
There was a problem hiding this comment.
That doesn't need to happen in this PR though..
There was a problem hiding this comment.
Confirmed (by stepping through the tests I added for this scenario) that tags are not a good criteria.
|
test windows_release_unit64_prtest please |
|
@Pilchie for ask-mode approval for 15.6. Thanks |
This PR reverts that change and a similar diagnostic will be implemented as an analyzer with dotnet/roslyn-analyzers#1727. Fixes dotnet#25748
Revert compat break change made as part of #23776
Customer scenario
Some analyzer (such as the UnboundIdentifier analyzer in the linked issue) produce similar diagnostics (ID and message) to the compiler's.
That is extremely confusing and leads to wasting time in investigation.
In the screenshot below, the red squiggles on
nameofare actually produced by an analyzer, which makes a "CS0107" diagnostic...Bugs this fixes
Fixes part of #23667
Fixes part of #22615
Workarounds, if any
Not applicable.
Risk
Low. This PR modifies the analyzer infrastructure to catch when non-compiler analyzers produce a diagnostic starting with "CS" or "BC". A couple of such faulty analyzers are updated to produce distinct diagnostics instead (with correct "IDE" prefix).
Performance impact
Low. Just an additional validation on diagnostics.
Is this a regression from a previous update?
No.
How was the bug found?
Some customer reported issues caused me to hit this problem during investigation.