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

configure rule severity in .EditorConfig is not working for category level #5077

Closed
Ankhimes opened this issue Apr 29, 2021 · 16 comments
Closed

Comments

@Ankhimes
Copy link

Analyzer

Diagnostic ID: CA2013: Do not use ReferenceEquals with value types

Analyzer source

SDK: Built-in CA analyzers in .NET 5 SDK or later

Version: SDK 5.0.100

OR

NuGet Package: Microsoft.CodeAnalysis.NetAnalyzers

Version: 5.0.3 (Latest)

Describe the bug

Steps To Reproduce

Expected behavior

Actual behavior

Additional context

@Ankhimes
Copy link
Author

[*.cs]

dotnet_analyzer_diagnostic.category-design.severity = error

is not working

@Youssef1313
Copy link
Member

@Ankhimes Have you tried "Design" instead of "design"?
I think the category may be case-sensitive.

@Ankhimes
Copy link
Author

yes i have tried..didnt worked

@Ankhimes
Copy link
Author

categoryRuleapplied

As clear in the screenshot...Indiviadual rule ID rule works but Category Rule is not working

@mavasani
Copy link
Contributor

@Ankhimes Your issue here is that CA1031 is disabled by default, and dotnet_analyzer_diagnostic.category-XXX.severity entries only apply to analyzers which are enabled by default. Please see the Important note section in https://docs.microsoft.com/dotnet/fundamentals/code-analysis/configuration-options#scope for details and how to configure this correctly.

@Ankhimes
Copy link
Author

@Ankhimes Your issue here is that CA1031 is disabled by default, and dotnet_analyzer_diagnostic.category-XXX.severity entries only apply to analyzers which are enabled by default. Please see the Important note section in https://docs.microsoft.com/dotnet/fundamentals/code-analysis/configuration-options#scope for details and how to configure this correctly.

Thanks for updates. But I did tried that option by updating the csproj file and its not working. Is there any other setting I need to try with.
image

@mavasani
Copy link
Contributor

Can you share your project as a zip or at least the screenshot of entire project file?

@Ankhimes
Copy link
Author

Can you share your project as a zip or at least the screenshot of entire project file?

image

@mavasani
Copy link
Contributor

Ah, I get the core issue now. AnalysisMode property works by adding a globalconfig to the project that explicitly sets severity for every rule by rule ID as a warning. This means that the category based bulk configuration setting has lower precedence and never works. In short, AnalysisMode and dotnet_analyzer_diagnostic.category-<rule category>.severity = <severity value> can never work together in the current implementation of auto-generated global config files for AnalysisMode.

I know how we can fix this, I'll have a PR out for this sometime today/tomorrow. Meanwhile, I suggest you explicitly escalate each rule by rule ID instead of doing it by category.

@Youssef1313
Copy link
Member

Youssef1313 commented Apr 30, 2021

@mavasani Isn't this the same issue as dotnet/roslyn#51225 which got fixed?
I also wonder if it's a precedence issue, then at least the rule will have be a warning instead of the expected error right? But the screenshot shows no violation at all.

@mavasani
Copy link
Contributor

@Youssef1313 This is a different issue - the compiler is working as expected, the global config from AnalysisMode has ruleID based entry which will always override all category based entries. The solution is to change the config file generation for AnalysisMode to use category based entries.

@Youssef1313
Copy link
Member

Youssef1313 commented Apr 30, 2021

Got it thanks.

Switching to the globalconfig generation to category based entries can affect other analyzers if they use the same category though, will that be intended?

Wondering if rule-based configuration should override category-based only if it has a higher global_level?

// Append 'global_level' to ensure conflicts are properly resolved between different global configs:
// 1. Lowest precedence (-2): Category-agnostic config generated by us.
// 2. Higher precedence (-1): Category-specific config generated by us.
// 3. Highest predence (non-negative integer): User provided config.
// See https://github.com/dotnet/roslyn/issues/48634 for further details.
var globalLevel = category != null ? -1 : -2;
result.AppendLine($@"global_level = {globalLevel}");
result.AppendLine();

@mavasani
Copy link
Contributor

So, I put more thought into this and I believe this scenario cannot be supported based on the current compiler and AnalsisMode design.

  1. Only way to enable a disabled by default rule is with a ruleID based dotnet_diagnostic entry.
  2. Even if you specify <AnalysisMode>AllEnabledByDefault</AnalysisMode> in the the project file, the auto-generated global config has to specify a ruleID based entry, category based entry will not enable a disabled by default rule.
  3. Now, as a ruleID based entry has been specified, there is no way to use a category based bulk configuration entry as it can never override ruleID based entry.

The only possible solution here could be if we introduce parallel group of auto-generated editorconfigs for enabling rules as warnings vs enabling them as errors. This is a much bigger work item, and we probably need more customer feedback before attempting any such changes. @Ankhimes - do you think just using <CodeAnalysisTreatWarningsAsErrors>true</CodeAnalysisTreatWarningsAsErrors> to bump all CA warnings to errors will suffice your scenario?

@Ankhimes
Copy link
Author

@mavasani Thanks for your support. Unfortunately we don't have requirement to convert all warnings as error.
We only need to configure severity based on the category

@mavasani
Copy link
Contributor

mavasani commented Apr 30, 2021

@Ankhimes In that case you will need to do the following:

  1. Use the category based entry to escalate all the enabled by default rules in categories to errors. Do not specify AnalysisMode as that will nullify all category based entries.
  2. Use rule ID based entries to escalate individual disabled by default rule to error.
  3. Wait for Bulk severity configuration in .editorconfig should be applied to "disabled by default" analyzers roslyn#47046 (comment) so we can use category based entries to apply on disabled by default rules.

@mavasani
Copy link
Contributor

mavasani commented May 6, 2021

Closing this out as there is not much we can do here. You need to follow the suggestion above.

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

No branches or pull requests

3 participants