- Notifications
You must be signed in to change notification settings - Fork481
Write CA1824 as non-compilation-end#6873
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
7fd5ab1 to2073e40CompareCodecov Report
Additional details and impacted files@@ Coverage Diff @@## main #6873 +/- ##======================================= Coverage 96.39% 96.39% ======================================= Files 1403 1403 Lines 330977 331006 +29 Branches 10890 10892 +2 =======================================+ Hits 319055 319085 +30+ Misses 9188 9187 -1 Partials 2734 2734 |
...ers/Core/Microsoft.NetCore.Analyzers/Resources/MarkAssembliesWithNeutralResourcesLanguage.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| data.ApplicationSyntaxReference?.GetSyntax(context.CancellationToken)is{}attributeSyntax) | ||
| { | ||
| // we have the attribute but its doing it wrong. | ||
| context.ReportDiagnostic(attributeSyntax.CreateDiagnostic(Rule)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
❗ This diagnostic (the one with a specific location) shouldalways be reported.It can set it should not return without reporting ifalreadyReportedDiagnostic to true, butalreadyReportedDiagnostic was already true.
Edit: It cannot setalreadyReportedDiagnostic to true because it would destabilize the no-location reporting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Diagnostic with location will always be reported if we find a source assembly attribute here, otherwise a no-location diagnostic will always be reported - I believe the logic here is deterministic becausedata is computed at CompilationStart and does not alter during subsequent callbacks.
mavasani commentedOct 3, 2023
@sharwell I can merge once you are happy with the PR, thanks! |
mavasani commentedOct 11, 2023
Merging this PR - feel free to create a follow-up PR for any follow-up changes. |
The intent of this change is two things:
@mavasani@sharwell Do you think it's worth changing this analyzer to not be non-compilation-end (which will make it available in IDE live diagnostics)?
Is there a better than what I did here?