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

Analysers don't respect <IncludeAll Action="Warning" /> in .ruleset file #25409

Open
Gav-Brown opened this issue Mar 11, 2018 · 6 comments
Open
Labels
Milestone

Comments

@Gav-Brown
Copy link

Version Used:
Visual Studio 2017 v15.6.1
Microsoft.CodeAnalysis.FxCopAnalyzers v2.6.0 (+ other analysers)

Steps to Reproduce:

  1. Create new .ruleset file
  2. Add <IncludeAll Action="Warning" /> element to it
  3. View rules in Visual Studio rule editor UI
  4. Run analysis on build
<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="Code Rules" Description="Should include all rules" ToolsVersion="15.0">
  <IncludeAll Action="Warning" />
</RuleSet>

Expected Behaviour:
All rules, from all analysers, are enabled (see UI) and produce warnings when rules are broken (run build).

Actual Behaviour:
Only the "default" rules are enabled and at their "default" severity.

This setting works for the original VS Code Analysis (managed binary) rules.
It would be great if analysers could respect this flag as this is useful for two reasons.

  1. The .ruleset file only contains rules you have disabled, and over time you can fix them and thus remove them from this file.

  2. When you upgrade the analysers and new rules have been added, you get these for free. You can then decide to fix or disable them as required. Otherwise you never know about the new rules.

I've tested on FxCopAnalyzers and other third party analysers. Seems to affect them all. Except perhaps StyleCop.Analyzers but I would have to double check that.

Please see dotnet/roslyn-analyzers#1621 for list of FxCopAnalyzer rules that are disabled even when this flag is on.

@jcouv jcouv added the Area-IDE label Mar 12, 2018
@Gav-Brown
Copy link
Author

Gav-Brown commented Mar 24, 2018

@jcouv , just a note to say this affects the build as well as viewing the rules in the IDE. So the label Area-IDE isn't quite accurate.

It's also particularly handy to support this for legacy (but active) projects where many rules may be disabled or customised in the .ruleset file. Mixing this with other rules that needed to be turned on makes it harder to maintain.

@jinujoseph jinujoseph added this to the Unknown milestone Apr 20, 2018
@Gav-Brown
Copy link
Author

To workaround this issue I have created an "All rules enabled" ruleset. Then I inherit from that and disable the rules we need to. Each time we upgrade any analyser, we add new rules to the "All rules enabled" ruleset and fix/disable any in inherited ruleset as required.

If this feature is corrected we wouldn't need to maintain the "All rules enabled" ruleset, or manually update it.

@steve-haar
Copy link

@Gav-Brown can you share your "All rules enabled" ruleset? Where do you look to maintain that?

@Gav-Brown
Copy link
Author

@steve-haar I can share but it is dependant on what analysers you use. The easiest way to create it for your setup, is to open the ruleset editor in Visual Studio, enable all the rules as error, except for old 'Managed Binary Analysis' and 'IDE' rules, and then save it to a .ruleset file. I've attached mine as an example.

CodeAnalysis.AllRulesEnabled.ruleset.txt

Then you can create your actual ruleset file by including and overriding this one, disabling any rules you want to turn off.

<Include Path="CodeAnalysis.AllRulesEnabled.ruleset" Action="Error" />

@nahk-ivanov
Copy link

So is there a way to enable all analyzer rules and then selectively disable those that we don't want to use within our team? Ideally I'd like to have all of them enabled by default, unless explicitly disabled. If new rule is added in the new version of analyzer package - we want it to be enabled as well, so that we can discuss it within the team and decide whether it makes sense for us or not.

I'm wondering if with new .editorconfig configuration file we can do that?

@nahk-ivanov
Copy link

nahk-ivanov commented Jul 24, 2020

Seems like in .editorconfig we can now do the following for all analyzers' rules:

# Enable all
dotnet_diagnostic.severity = warning

# FxCop

## Avoid uninstantiated internal classes.
## Justification: Incompatible with dependency injection - internal classes registered as services with container or exported by attribute
dotnet_diagnostic.CA1812.severity = none

# StyleCop
dotnet_diagnostic.SA0001.severity = none

Now the "problem" is that you can't enable/disable all rules for a particular analyzer package. There is an article about FxCop rules configuration through .editorconfig, which says it should be dotnet_code_quality, but it didn't work. dotnet_diagnostic worked, but it also includes all IDE rules. This is not a big deal for me, as I'd prefer to enable all and explicitly disable some rules with justification.

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

No branches or pull requests

5 participants