search

dennisdoomen / CSharpGuidelines

A set of coding guidelines for C# 9.0, design principles and layout rules for improving the overall quality of your code development.
https://www.csharpcodingguidelines.com
Other
746 stars 271 forks source link

AV2210: Invalid warning level #234

Closed bkoelman closed 7 months ago

bkoelman commented 2 years ago

The rule at https://github.com/dennisdoomen/CSharpGuidelines/blob/master/_rules/2210.md states to use warning level 4, while the highest level is 5 at the moment.

But more importantly, since .NET 5, the warning level is driven by the AnalysisLevel MSBuild property: Microsoft.NET.Sdk.Analyzers.targets (which only applies to SDK-style projects) simply overwrites any explicit warning level with a value that depends on the target framework. For .NET 5, AnalysisLevel defaults to "5" (which maps to warning level 5) and for .NET 6 it defaults to "latest" (which maps to warning level 6, which does not actually exist, so csc.exe uses warning level 5 instead). Because of this, the recommendation to use warning level 9999 has no effect, except for .NET Framework projects that use the legacy .csproj format.

For more background info, see the links in the description of commit https://github.com/bkoelman/CSharpGuidelinesAnalyzer/commit/90174eafccf08514c856a1a0a1fa6adcfc15521a.

Since .NET 6, AnalysisLevel also affects which built-in analyzers (formerly FxCop) are activated (see https://docs.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#analysislevelcategory). We could advise on using (some of) those in AV2210, but they aren't widely used yet and are still quite buggy. Turning on all of them usually leads to many false positives and/or crashes.

So to keep it simple, I think the rule text should be changed from "use Warning Level 4 for the C# compiler" to "use the highest available warning Level for the C# compiler" (which is what you'll get by default).

bkoelman commented 2 years ago

@dennisdoomen Awaiting feedback before creating a PR for this.

dennisdoomen commented 2 years ago

I still have to wrap my head around the consequences of this. I probably should try myself on FluentAssertions and see what happens.

Udo-usis commented 1 year ago

Does anybody know why suppressing AV2210 with dotnet_diagnostic.AV2210.severity = none in .editorconfig is not working?

bkoelman commented 1 year ago

It needs to be in .globalconfig because it's a project-level analyzer. A .rulesets file works as well, although officially deprecated.

Udo-usis commented 1 year ago

@bkoelman : Thanks for the tip! That works fine.