Roslyn: Cannot configure severity with editorconfig for analyzer diagnostics with Location.None

[mavasani]

Version Used: Latest 16.3 dogfood preview build

Steps to Reproduce:

1. Open attached ClassLibrary28.zip, which is a simple C# netstandard class library project with a ruleset with following contents to configure couple of CA diagnostics to errors:

<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="MyRules" Description="MyRules" ToolsVersion="10.0">
  <Rules AnalyzerId="Microsoft.CodeQuality.Analyzers" RuleNamespace="Microsoft.CodeQuality.Analyzers">
    <Rule Id="CA1014" Action="Error" /> <!-- Mark assemblies with CLSCompliant -->
    <Rule Id="CA1040" Action="Error" /> <!-- Avoid empty interfaces -->
  </Rules>
</RuleSet>

1. Build project and verify 2 errors as expected:
   
2. Now delete the ruleset from the project and replace it with the below equivalent editorconfig file (note can also use [*.cs] for the section header):

[*.*]

# CA1014: Mark assemblies with CLSCompliant
dotnet_diagnostic.CA1014.severity = error

# CA1040: Avoid empty interfaces 
dotnet_diagnostic.CA1040.severity = error

1. Rebuild project

Expected Behavior:
2 errors similar to behavior when ruleset was used

Actual Behavior:
Only one error for CA1040

CA1014 being a project level diagnostic with Location.None doesn't seem to be configurable from editorconfig file. This issue applies to all project level analyzer diagnostics, CA1014 is not special.

[mavasani]

Tagging @jasonmalinowski @sharwell @agocke @jinujoseph

I believe this could become a blocker in attempting to migrate from rulesets to editorconfig. I am not sure if there is an easy solution to this issue as the diagnostic does not apply to any specific source location/file/folder. One potential solution could be to apply the settings from the editorconfig file(s) marked with root = true for all no-location diagnostics.

[agocke]

This is by design and I'm strongly opposed to changing it. This functionality can be achieved by using warnaserror with the id in the project file. Editorconfig was never meant to replace all diagnostic functionality and we should not have that as a goal.

[mavasani]

I am going to re-open this issue so at least the consequences of Won't fixing this can be understood and discussed at IDE design meeting. Also changing it to Area-IDE.

1. We cannot write a reliable migration tool to convert any given ruleset file into an equivalent editorconfig file, which has been requested and discussed few times with @jinujoseph @vatsalyaagrawal @mikadumont. Note that a ruleset file is independent of project context and is generally shared across multiple projects in a solution, so one cannot go from a pointed ruleset file to each applicable project and update it to add a /nowarn.

2. The existing Analyzers node's "Set Rule Set Severity" functionality is being moved over to "Set severity" to instead update the editorconfig file with https://github.com/dotnet/roslyn/pull/37795. As the rule metadata does not indicate if it will produce no-location diagnostics, we cannot differentiate rules on when to add a configuration entry to editorconfig vs doing a /nowarn. Unfortunately, this would mean we cannot take that PR and would need to continue offering generating/editing ruleset files from Analyzers node for reliable configuration.

3. While attempting to manually update some existing project using ruleset to editorconfig, it took me considerable amount of debugging why some of the severity settings in editorconfig work and some don't - was my syntax wrong? was some other editorconfig in my directory structure overriding it? was some other MSBuild property causing it not to be respected for some analyzer diagnostics? Explaining all this to customers would be much harder.

[sharwell]

I'm with @agocke here. The .editorconfig format is known to not have full parity with ruleset files. The original design proposal included one possible provision to address this, but after discussion the provision was rejected. Outside of new evidence related to this (e.g. an intent to remove compiler support for ruleset files), I don't see a need to bring this to a design review again since a conclusion on the matter was already reached.

Diagnostics without a location, and diagnostics reported in files not contained in the directory tree under the solution root (including but not limited to files included as links into the shared NuGet packages directory and files in the temporary files directory) cannot be configured with .editorconfig files.

[mavasani]

Outside of new evidence related to this (e.g. an intent to remove compiler support for ruleset files), I don't see a need to bring this to a design review again since a conclusion on the matter was already reached.

I disagree strongly with the push back to bring this to IDE design meeting. We most certainly need to discuss this as the assertion here means we conclude we cannot offer an automated tooling or migration path for customers from rulesets to editorconfig, hence do not have a path forward to deprecate rulesets and ruleset editor. Additionally, we cannot claim that rulesets can be deprecated as existing IDE tooling, such as set ruleset severity command in analyzers node, cannot be moved to use editorconfig based configuration.

[mavasani]

Actually, I thought of a workaround from analyzer driver side that can resolve this issue. I am going to implement it and move this issue to Area-Analyzers, removing the Needs Design Review tag.

[ryzngard]

Design Review: For current release we will fix bugs but consider that editorconfig is not a direct replacement of ruleset. In next release we will evaluate gaps and coordinate with compiler team to find resolution and if editorconfig should be a full replacement for ruleset. We will continue to promote editorconfig as the solution for NEW projects.

[jasonmalinowski]

Reactivating this -- should we keep this open in the project for tracking the known issues when we discuss further?

[mavasani]

I believe I have a fix for this in the analyzer driver which should handle this specific case correctly: https://github.com/dotnet/roslyn/compare/master...mavasani:Issue37876?expand=1#diff-f8159931eab67875a9504f78827742b7R1515-R1544. I will submit a PR.

[CC84]

What is the state of this problem? I justed startet migrating rulesets to editorconfig as "highly recommended" here: https://docs.microsoft.com/en-us/visualstudio/code-quality/use-roslyn-analyzers?view=vs-2019
I just used the conversion by VS. This changes CodeAnalysisRuleSet to MinimumRecommendedRules.ruleset.

Now CA1014 is no longer a warning and i'm unable to change this bei editorconfig. Maintaining ruleset and editorconfig is very confusing as you never know, which file is used for which rule.

This can't be the right way....

I can't understand how you could highly recommend something, which is working so bad.

[mavasani]

@CC84 Configuring no location diagnostics with editorconfig should work with the the global analyzer config support that was recently added by @chsienki with https://github.com/dotnet/roslyn/pull/43889. This should be supported in VS2019 16.7 or later

1. You need to create a new analyzer config file with the following contents:

is_global = true

dotnet_diagnostic.CA1014.severity = warning

1. Then ensure this analyzer config file gets passed to the compiler in the one of the following ways:

   1. via MSBuild:

      <ItemGroup>
        <EditorConfigFiles Include="<%full_path_to_config_file%>" />
      </ItemGroup>
   

   1. via command line arguments to csc.exe:

   /analyzerconfig:<%full_path_to_config_file%>
   
   

[mavasani]

Actually, I spoke too soon. I _expected_ that global analyzer config would have resolved this issue, but it seems that it still does not. The following code in diagnostic filter logic that looks at editorconfig for effective severity only works for diagnostics with location (needs non-null syntax tree):
https://github.com/dotnet/roslyn/blob/ffbb9c7e8f7699c3eb24fa559e6bb381f3405605/src/Compilers/CSharp/Portable/Compilation/CSharpDiagnosticFilter.cs#L154-L159

@chsienki @agocke we need to update that logic to look at diagnostic severity settings from global analyzer config files for no location diagnostics.

[mavasani]

Maintaining ruleset and editorconfig is very confusing as you never know, which file is used for which rule.

@CC84 CA1014 is enabled as a warning by default in FxCop analyzers package. So, if you did not set any ruleset file for your project, the rule should execute fine with the default configuration. Seems like we recently disabled this rule by default in FxCop analyzers package: https://github.com/dotnet/roslyn-analyzers/blob/f15404b312295e7cee16fb40b4c8d3f91f10f087/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/MarkAssembliesWithAttributesDiagnosticAnalyzer.cs#L41

So you would need a ruleset or editorconfig entry to enable this rule, and given this issue, ruleset seems to be the only route to enable it as of now.

[chsienki]

@mavasani What's the issue here? That we're not checking the global config for the value?

I thought that it should make it to the tree by virtue of the analyzer config set. From that point on there shouldn't be any special global config handling, no?

[mavasani]

@chsienki - The core issue is for analyzer diagnostics without location or Location.None. Source tree for this diagnostic will be null: https://github.com/dotnet/roslyn/blob/ffbb9c7e8f7699c3eb24fa559e6bb381f3405605/src/Compilers/CSharp/Portable/Compilation/CSharpDiagnosticFilter.cs#L125

[agocke]

We should put the global options that apply to all syntax trees into the SpecificDiagnosticOptions attached to the compilation, not the per-syntax tree options. This should solve the problem.

[CC84]

Is there roadmap, when it will go on?

[agaace]

Is this in any way related to Visual Studio ignoring this editorconfig value?

dotnet_diagnostic.IDE0062.severity = silent

I have a Core project that builds both Core and legacy .NET 4.8 targets. This new project structure doesn't even include analyzers anymore, unless I manually modify the project's XML by hand. And I don't even know if analyzers even work in Core projects? I had the impression I'm supposed to use editorconfig for all new projects. Well, this is a new project but it cannot use C# 8 features due to .NET Framework backwards compatibility. So I keep getting all these suggestions about using C# 8 features that I cannot use, and I cannot suppress all this noise in editorconfig. How do I suppress, for example, IDE0062, please? Please don't tell me I need to maintain a suppression file now, because this is going backwards by a decade, compared to what my experience using rulesets had been :/

[webczat]

rulesets work well in dotnet core, same as editorconfig, although not sure what version of vs is needed. I am using vscode and this was aded only recently, but less so on vs.

[webczat]

and analyzers do work, I mean, the roslyn analyzers.

[mavasani]

[*.cs]
dotnet_diagnostic.IDE0062.severity = none

Above entry in your .editorconfig should suppress all IDE0062 for files and folders under that directory.

[vchirikov]

Faced the same problem, workaround with global .editorconfig doesn't work :(
A workaround is to use<NoWarn> :(

[mavasani]

@vchirikov I have verified that the issue is now resolved in internal 16.8 Preview3 dogood builds with recent global analyzer config changes from @chsienki. Attaching the modified class library from the issue description with a global analyzer config instead of a ruleset and it does the same job.
ClassLibrary28.zip

[beho]

Hi,

when using AllEnabledByDefault (opt-out) analysis mode and trying to disable CA1014: Mark assemblies as CLSCompliant (which is no-location diagnostic) I get following warning:

Multiple global analyzer config files set the same key 'dotnet_diagnostic.ca1014.severity' in section 'Global Section'.
It has been unset.
Key was set by the following files: 'C:\Program Files\dotnet\sdk\5.0.100\Sdks\Microsoft.NET.Sdk\analyzers\build\config\AnalysisLevel_5_AllEnabledByDefault.editorconfig, C:\Users\abc\Projects\solution\src\..path_to_project..\.globalconfig'

So it seems that SDK-level globalconfig clashes with project-level one.

Am I missing something or there is no way to disable no-location warning without getting another one in opt-out mode?

[ChrisTorng]

I'm using AllEnabledByDefault, and is still looking for way to disable CA1014. But I don't see any warning as @beho seem. If there is no way to disable, I can only adding [assembly: CLSCompliant(false)] to Program.cs. There is no default AssemblyInfo.cs in .NET Core project, and I'm not willing to add the file just for ignoring CA1014.

[mavasani]

@beho @ChrisTorng This is a known issue that will be addressed when @chsienki implements #48634 to allow user specified global config to override SDK/NuGet provided global config. Until that feature is implemented, you can workaround by disabling this rule in MSBuild props/project file:

<PropertyGroup>
  <NoWarn>$(NoWarn);CA1014</NoWarn>
</PropertyGroup>