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.

Sign up for GitHub

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

Jump to bottom

Question: what's the suggested usage between NetAnalyzers and FxCopAnalyzers? #4100

Closed
huoyaoyuan opened this issue Aug 31, 2020 · 1 comment · Fixed by #4104
Closed

Question: what's the suggested usage between NetAnalyzers and FxCopAnalyzers? #4100

huoyaoyuan opened this issue Aug 31, 2020 · 1 comment · Fixed by #4104
Assignees
@mavasani
Labels
Area-Infrastructure Feature Request

Comments

@huoyaoyuan
Copy link
Member

After reading the post in devblog, I've got some questions:

  • Is it supported to use the two analyzer packages at the same time?
    image
    After upgrading to .NET 5 Preview 8, there is a warning icon in IDE for FxCopAnalyzers package. I'm afraid there's some conflict between the two packages.

  • What's the suggested way to aggressively opt-in more up-to-date analysis?
    FxCopAnalyzers and NetAnalyzers carries many analyzers, and some of them are old and not suitable for .NET 5. The default severity of FxCopAnalyzers looks too aggressive, where NetAnalyzers looks too loose. It's unclear that which rule set is suggested.
    I mentioned New rule: Extra type checking / type casting #4002 (comment) . But there are also other analyzers defaults to a different severity level like CA1305.
@mavasani
Copy link
Contributor

mavasani commented Aug 31, 2020 •
edited
Loading

@huoyaoyuan Thank you for reporting the issue.

Is it supported to use the two analyzer packages at the same time?

  1. The set of CA analyzers that ship in FxCop analyzers package and NetAnalyzers in the .NET SDK are currently identical. So you don't need both set. In future, it is likely that FxCop analyzers will be phased out in favor of NetAnalyzers, which will also be installable as a NuGet package for decoupling analyzers from the SDK.
  2. FxCopAnalayzers NuGet package explicitly turns off built-in NetAnalyzers in the SDK and ensures no duplicate analysis. The warning icon in the package seems unrelated, I am not sure why it is showing up for you.

aggressively opt-in more up-to-date analysis

This is a very good point, and I believe lot of customers would want a more aggressive opt-out mode. One simple trick that you can use is define dotnet_analyzer_diagnostic.severity = warning in your .editorconfig. This will default all enabled by default analyzers in your project to be warning by default. However, note that this will not turn on disabled analyzers, similar to the feature requested here: dotnet/roslyn#47046. We do have many disabled analyzers to keep the NetAnalyzers shipping in the box conservative, and lot of folks would want a single entry to turn on the applicable ones. I think it is reasonable to consider adding an MSBuild property similar to AnalysisLevel that enables switching analyzers from opt-in conservative mode to an opt-out aggressive mode. Let's track this feature request with this issue.

@mavasani mavasani self-assigned this Aug 31, 2020
mavasani added a commit to mavasani/roslyn-analyzers that referenced this issue Aug 31, 2020
@mavasani
Add support for AnalysisMode
9e8b60a 
Closes dotnet#4100

**Proposal**
This new property would allow users to switch between following 3 analysis modes:

1. **Default mode**: Analyzers with enabled by default and severity values that we ship today. It will continue to be the default analysis mode.
2. **OptIn or AllDisabled or Conservative mode**: All analyzers are disabled by default. Only the ones that user explicitly enables in their editorconfig/ruleset will be enabled with the explicitly configured severity.
3. **OptOut or AllEnabled or Aggressive mode**: Majority of analyzers get enabled as warnings, similar to the current FxCop analyzers package. We still want some security rules and no longer recommended rules to be disabled, but for all practical purposes, we can consider this to be the most aggressive analysis mode for the package.

The property should work amicably with AnalysisLevel, i.e. if user sets AnalysisLevel to 5.0, the aggressive, all-enabled mode only turns on rules shipped in or before 5.0 to warnings. Rules shipping after 5.0 will still be turned off by default.

**Targeted scenarios**

1. **User requests**: We already have a few user requests to enable aggressive analysis mode, similar to FxCop analyzers:
   1. dotnet#4100: This was requested from a customer from Jon’s blog.
   2. dotnet/roslyn#47046: This was requested from a customer trying out our bulk configuration editorconfig functionality to switch NetAnalyzers to be as aggressive as FxCopAnalyzers
This feature enables users who are super passionate about analyzers and code quality to easily switch to an opt-out, aggressive, analysis mode, which seems fantastic to me! At the same time, it helps users who like complete opt-in model with explicit control over each rule that is enabled in their configuration file.

2. **Path to deprecation for FxCopAnalyzers package**: Currently, we ship identical CA rules in FxCopAnalyzers and NetAnalyzers package/SDK. The only difference is default severity and enabled by default values for rules. Adding this AnalysisMode knob means existing FxCop analyzer customers can easily migrate to the .NET SDK based analyzers and get equivalent functionality and this allows us to deprecate this package soon. This will make the overall code quality analyzers story much cleaner from an end user perspective.
@mavasani mavasani mentioned this issue Aug 31, 2020
Merged
@jmarolf jmarolf added this to the .NET 5 RC1 milestone Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mavasani mavasani

Labels
Area-Infrastructure Feature Request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for AnalysisMode mavasani/roslyn-analyzers
3 participants
@huoyaoyuan @jmarolf @mavasani