Add support to detect unnecessary inline SuppressMessageAttribute suppressions #45765
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…clared top level and method level members from a tree
…ce is split into two parts, similar to `ISyntaxFacts` and `ISyntaxFactsService`: 1. `ISemanticFacts`, which is purely semantic query API, is now accessible from shared analyzer and code fixes layer. 2. `ISemanticFactsService`, which is semantic query + code transformation API as well as an `ILanguageService`, is only accessible from shared code fixes layer. It stays as being accessible via GetLanguageService API.
…ressions Fixes dotnet#44178 Builds on top of dotnet#44848, which added support to detect unnecessary pragma suppressions.
mavasani
commented
Jul 7, 2020
Comment on lines
+177
to
+212
| // Compute all the reported compiler and analyzer diagnostics for diagnostic IDs corresponding to pragmas in the tree. | ||
| var (diagnostics, unhandledIds) = await GetReportedDiagnosticsForIdsAsync( | ||
| idsToAnalyze, root, semanticModel, compilationWithAnalyzers, | ||
| getSupportedDiagnostics, getIsCompilationEndAnalyzer, compilerDiagnosticIds, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
||
| // Iterate through reported diagnostics which are suppressed in source through pragmas and mark the corresponding pragmas as used. | ||
| await ProcessReportedDiagnosticsAsync(diagnostics, tree, compilationWithAnalyzers, idToPragmasMap, | ||
| pragmasToIsUsedMap, idToSuppressMessageAttributesMap, suppressMessageAttributesToIsUsedMap, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
||
| // Remove entries for unhandled diagnostic ids. | ||
| foreach (var id in unhandledIds) | ||
| { | ||
| foreach (var (pragma, _) in idToPragmasMap[id]) | ||
| { | ||
| pragmasToIsUsedMap.Remove(pragma); | ||
| } | ||
|
|
||
| if (idToSuppressMessageAttributesMap.TryGetValue(id, out var attributeNodes)) | ||
| { | ||
| foreach (var attributeNode in attributeNodes) | ||
| { | ||
| suppressMessageAttributesToIsUsedMap.Remove(attributeNode); | ||
| } | ||
|
|
||
| idToSuppressMessageAttributesMap.Remove(id); | ||
| } | ||
| } | ||
|
|
||
| // Finally, report the unnecessary suppressions. | ||
| var effectiveSeverity = severity.ToDiagnosticSeverity() ?? s_removeUnnecessarySuppressionDescriptor.DefaultSeverity; | ||
| ReportUnnecessarySuppressions(pragmasToIsUsedMap, sortedPragmasWithIds, | ||
| suppressMessageAttributesToIsUsedMap, reportDiagnostic, effectiveSeverity, compilationWithAnalyzers.Compilation); |
There was a problem hiding this comment.
All of this is just existing code, which now shows up as new code as bunch of existing code was refactored into ProcessPragmaDirectives.
|
Ping for reviews... |
|
Thanks @jmarolf! |
ghost
modified the milestones:
mavasani
added a commit
to mavasani/roslyn
that referenced
this pull request
Jul 16, 2020
…osticAnalyzer Fixes dotnet#46047 dotnet#45765 enhance this analyzer to support detecting unnecessary inline SuppressMessageAttribute suppressions. This led to a regression when processing idToPragmasMap for unhandled IDs, which is fixed by this change.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Strongly recommended to review commit by commit
Fixes #44178
Builds on top of #44848, which added support to detect unnecessary pragma suppressions.
ISyntaxFacts.GetTopLevelAndMethodLevelMembersto get all the declared top level and method level members from a treeISyntaxFactsandISyntaxFactsService:ISemanticFacts, which is purely semantic query API, is now accessible from shared analyzer and code fixes layer.ISemanticFactsService, which is semantic query + code transformation API as well as anILanguageService, is only accessible from shared code fixes layer. It stays as being accessible via GetLanguageService API.