Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to override severity of single rules when dotnet_analyzer_diagnostic.severity != unset|default #61777

Open
leoniDEV opened this issue Jun 8, 2022 · 4 comments

Comments

@leoniDEV
Copy link

leoniDEV commented Jun 8, 2022

Version Used:
VisualStudio.17.Preview/17.3.0-pre.1.1+32519.111
.net 7.0.100-preview.4.22252.9

Steps to Reproduce:

  1. in a project folder make a new .globalconfig
  2. add a third-party analyzer like StyleCop
  3. add the key dotnet_analyzer_diagnostic.severity = error
  4. add a key dotnet_diagnostic.<StyleCop rule ID>.severity = warning

Expected Behavior:
The severity of <StyleCop rule ID> is warning

Actual Behavior:
The severity of the still error

Additional Notes
Whatever is the severity in dotnet_analyzer_diagnostic.severity I'm unable to override the severity of a single rule but I'm still able to override the severity of an entire category with the key dotnet_analyzer_diagnostic.category-<rule category>.severity
This behavior occurs with analyzers installed from Nuget, at least with StyleCop, sonar-dotnet and Roslynator (which are the ones I tested).
With the built-in NetAnalyzer all works as expected (I'm still able to override the severity of a single rule), but if I disable the built-in analyzer using the property <EnableNETAnalyzers>false</EnableNETAnalyzers> and install the analyzer from Nuget also the NetAnalyzer follow the same behavior of the other third-party analyzer

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 8, 2022
@mavasani
Copy link
Member

@leoniDEV I am unable to reproduce this on the latest 17.3 preview build. Can you please re-confirm if you are still seeing this? If so, can you please share a repro project?

image

@mavasani mavasani added the Resolution-Not Reproducible The described behavior could not be reproduced by developers label Jun 29, 2022
@leoniDEV
Copy link
Author

After further investigation it seems a problem with the UI.

If you expand the Analyzers node in the Solution explorer the icons don't reflect the severity setting.
But it seems that the severity is effectively applied correctly.

image

image

Anyway here a repro project https://github.com/leoniDEV/Repro-proj-61777

@mavasani
Copy link
Member

mavasani commented Jun 30, 2022

Adding @chsienki and @jasonmalinowski.

Thanks @leoniDEV. I investigated this further and it seems the issue here is that diagnostic severity entries in global config files (basically dotnet_diagnostic.<%RuleId%>.severity entries) are slipping through the cracks in some of the compiler APIs and IDE handling of analyzer config options.

  1. AnalyzerConfigSet.GetOptionsForSourcePath API does not include the diagnostic severity entries from the global section of the global config files in the returned AnalyzerConfigOptionsResult. They are separately exposed via AnalyzerConfigSet.GlobalOptions property, these are not aggregated anywhere in the IDE.
  2. ProjectState.AnalyzerConfigOptionsCache type in the IDE holds both these per-path AnalyzerConfigData and the global AnalyzerConfigData separately, but these never seem to get aggregated. I don't think we want IDE to be in the business of aggregating options from two different option sources/files, compiler already has code do this merging.
  3. The IDE code to compute effective severity of a diagnostic ID at bunch of places only uses the per-path AnalyzerConfigData, either for a specific source file or root directory of the project, and ends up showing incorrect effective severity. The Analyzers node issue in the Solution explorer you mentioned above is one of these cases, but I think there are bunch more places where this would be the case.

Personally, I feel AnalyzerConfigOptionsResult.TreeOptions returned from AnalyzerConfigSet.GetOptionsForSourcePath API should include diagnostic severity entries from the global section of the global config files in the analyzer config set. If we do not want to change the semantics of an existing public API, maybe we should consider exposing another public API that returns aggregated AnalyzerConfigOptionsResult?

Moving this issue to compilers area path and assigning to @chsienki to comment.

@mavasani mavasani added Bug New Feature - Editor Config and removed Resolution-Not Reproducible The described behavior could not be reproduced by developers untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 30, 2022
@mavasani mavasani assigned chsienki and unassigned mavasani Jun 30, 2022
@danielchalmers
Copy link
Contributor

@mavasani Do you think this issue I'm having is related? #72726

# C# files
[*.cs]

# CA1041: Provide ObsoleteAttribute message
dotnet_diagnostic.CA1041.severity = warning

# Field preferences
dotnet_style_readonly_field = true:warning

# Unit Test Overrides
# Rules don't have to be as strict.
[**UnitTests**/*.cs]

# CA1041: Provide ObsoleteAttribute message
dotnet_diagnostic.CA1041.severity = suggestion

# Field preferences
dotnet_style_readonly_field = true:suggestion

The second CA1041 severity is not being respected in src/MudBlazor.UnitTests and is still a warning, but the dotnet_style_readonly_field is correctly being overridden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants