dotnet/roslyn-analyzers

Some rules can no longer be silenced with an .editorconfig file in .NET 7

billybraga opened this issue ยท 7 comments

Analyzer

Diagnostic ID: CA1848: Use the LoggerMessage delegates (among others)

Analyzer source

SDK: Built-in CA analyzers in .NET 7 SDK

Version: SDK 7.0.100

Describe the bug

Some rules' severity can no longer be set to none with a global .editorconfig file in .NET 7, but it could in .NET 6.

Steps To Reproduce

Build this solution with .NET 7.

Expected behavior

The Net7ProjectWithEditorConfigIgnore project builds successfully, like its equivalent in .NET 6.

Actual behavior

The project Net7ProjectWithEditorConfigIgnore fails to build because of CA1848 (CA1303 was successfully silenced, as we can see by the fact that Net7ProjectWithoutEditorConfigIgnore fails with CA1303).

Additional context

See the .editorconfig. It sets to none the severity of CA1303 and CA1848 in the Net7ProjectWithEditorConfigIgnore folder.

We're also running into this issue after udpating to Visual Studio 17.4 and .NET 7. While the projects configuration remained for "net6.0", the update in the tooling broke all our builds because we're using

<CodeAnalysisTreatWarningsAsErrors>true</CodeAnalysisTreatWarningsAsErrors>

and override errors within the .editorconfig instead of using other means of configuration. The official documentation at
https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-files#editorconfig
still shows the usage of overriding dotnet_diagnostic.CA____ severities.

Is this a regression problem or has the behavior of this feature changed with .NET 7?

This seems to have regressed with #5776 when CodeAnalysisTreatWarningsAsErrors is true and TreatWarningsAsErrors is not true. With that change, we now pass in /warnaserror+:CAxxxx for all CA rules and as per the severity precedence rules specified in https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-files#severity-options, /warnaserror+:ruleId overrides the editorconfig severity configurations dotnet_diagnostic.ruleId.severity = xxxx in the compiler.

I am not sure if it is possible to alter the compiler's precedence rules between /warnaserror+:ruleId and the editorconfig severity configurations, so we likely need a different mechanism to implement CodeAnalysisTreatWarningsAsErrors in the .NET SDK.

Meanwhile, as a workaround you can do one of the following:

  1. Set TreatWarningsAsErrors to true instead of CodeAnalysisTreatWarningsAsErrors
  2. Set <WarningsNotAsErrors>$(WarningsNotAsErrors);CAxxxx</WarningsNotAsErrors> for specific rules you don't want to bump to errors.

Solution 1 works, thanks!

@mavasani thank you for locating/isolating the cause.
Unfortunately we're in a migration phase for other code parts and have TreatWarningsAsErrors specifically set to false at the moment. Maybe we'll find a way to use it in combination with temporary editorconfig or WarningNotAsError overrides.

As mentioned the two options are only workarounds and not a "solution" as @billybraga described it. Since there seems to be an open issue with the handling between .editorconfig and warnaserror arguments, should this topic then preferrably be left open since the regression is still an issue that may be fixed at some point? Or should this be adressed in a separate topic in an SDK repo?

Since there seems to be an open issue with the handling between .editorconfig and warnaserror arguments

Can you please point out the issue?

As you pointed out:

I am not sure if it is possible to alter the compiler's precedence rules between /warnaserror+:ruleId and the editorconfig severity configurations, so we likely need a different mechanism to implement CodeAnalysisTreatWarningsAsErrors in the .NET SDK.

If I understand correctly, using compiler parameters is the necessary way to fix #5776 which implicitly changes the precedence of severity options and therefore .editorconfig is ignored. This only applies to the specific case where TreatWarningsAsErrors=false and CodeAnalysisTreatWarningsAsErrors=true.

I evaluated the behavior of TreatWarningsAsErrors and having TreatWarningsAsErrors=true it's still possible to override e.g. CS0067 via .editorconfig. Is the following an inconsistent behavior that would need an open issue?

  • TreatWarningsAsErrors=true - severity can be overwritten in .editorconfig
  • TreatWarningsAsErrors=false & CodeAnalysisTreatWarningsAsErrors=true - severity cannot be overwritten in .editorconfig

Notes about this fix:

  1. With #6427, we have re-implemented how CodeAnalysisTreatsWarningsAsErrors works, which should address the issues identified here. The fix is part of Microsoft.CodeAnalysis.NetAnalyzers NuGet package version 8.0.0-preview1.23063.1 which should be available on this public feed very soon: https://dev.azure.com/dnceng/public/_artifacts/feed/dotnet8/NuGet/Microsoft.CodeAnalysis.NetAnalyzers/versions

  2. To avail this fix one would need to do either of the following:

    1. Move to .NET 8 SDK preview1 or later (once released). You should be able to use CodeAnalysisTreatsWarningsAsErrors property once you are on an SDK version with this fix, regardless of whether you consume the analyzers directly from the SDK or add a NuGet package reference to Microsoft.CodeAnalysis.NetAnalyzers NuGet package with version greater than or equals 8.0.0-preview1.23063.1
    2. If you are unable to move to .NET 8 SDK preview1 or later, and still continue to be on .NET 7 SDK or earlier, you can avail this fix by just adding a PackageReference to Microsoft.CodeAnalysis.NetAnalyzers with version greater than or equals 8.0.0-preview1.23063.1 but setting the property EffectiveCodeAnalysisTreatsWarningsAsErrors instead of CodeAnalysisTreatsWarningsAsErrors. EffectiveCodeAnalysisTreatsWarningsAsErrors overrides the buggy implementation of CodeAnalysisTreatsWarningsAsErrors in already shipped .NET 7 SDK. In future, whenever you are able to move to .NET 8 SDK, you can revert back to using CodeAnalysisTreatsWarningsAsErrors property instead of EffectiveCodeAnalysisTreatsWarningsAsErrors