Some Analyzers Not Honoring All Rule Severity Using .NET 7 SDK

[RyanThomas73]

Analyzer

Multiple. Ones I've Witnessed So Far:

Diagnostic ID: CA1031
Diagnostic ID: CA1062
Diagnostic ID: CA1305
Diagnostic ID: CA5394

Analyzer source

SDK: Built-in CA analyzers in .NET 5 SDK or later

Version: SDK 7.0.201

Describe the bug

Some rules are not honoring severity when set for all analyzers or for an entire category.

Steps To Reproduce

Enable error diagnostic severity for all rules or for a category of rules that includes one of the above analyzers.
See also https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-options#scope

Test that analyzer against code which violates the analyzer rule.
For example:

// With dotnet_analyzer_diagnostic.severity = error
5.ToString(); // CA1305 Violation

Expected behavior

The analyzer flags the violation as an error.

Actual behavior

The analyzer flags the violation as a warning.

Additional context

Setting the severity by individual rule id does flag the violation at the correct severity.
Encountered while testing in a project targeting net6.0 but building using the .NET 7.0.201 SDK .

Re-tested in a project targeting net6.0 and building using the .NET 6.0.401 SDK and the severity for all analyzers works correctly.

[Youssef1313]

https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-options#scope

When you configure the severity level for multiple rules with a single entry, either for a category of rules or for all rules, the severity only applies to rules that are enabled by default. To enable rules that are disabled by default, you must either:

• Add an explicit dotnet_diagnostic.<rule ID>.severity = <severity> configuration entry for each rule.
• In .NET 6+, enable a category of rules by setting <AnalysisMode<Category>> to All.
• Enable all rules by setting <AnalysisMode> to All or by setting <AnalysisLevel> to latest-All.

Out of the 4 rules you mentioned, there are 3 that are not enabled by default:

roslyn-analyzers/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif

Lines 940 to 948 in b0fe62c

	"CA1031": {
	"id": "CA1031",
	"shortDescription": "Do not catch general exception types",
	"fullDescription": "A general exception such as System.Exception or System.SystemException or a disallowed exception type is caught in a catch statement, or a general catch clause is used. General and disallowed exceptions should not be caught.",
	"defaultLevel": "warning",
	"helpUri": "https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1031",
	"properties": {
	"category": "Design",
	"isEnabledByDefault": false,

roslyn-analyzers/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif

Lines 1357 to 1365 in b0fe62c

	"CA1062": {
	"id": "CA1062",
	"shortDescription": "Validate arguments of public methods",
	"fullDescription": "An externally visible method dereferences one of its reference arguments without verifying whether that argument is 'null' ('Nothing' in Visual Basic). All reference arguments that are passed to externally visible methods should be checked against 'null'. If appropriate, throw an 'ArgumentNullException' when the argument is 'null'. If the method is designed to be called only by known assemblies, you should make the method internal.",
	"defaultLevel": "warning",
	"helpUri": "https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1062",
	"properties": {
	"category": "Design",
	"isEnabledByDefault": false,

roslyn-analyzers/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif

Lines 5738 to 5746 in b0fe62c

	"CA5394": {
	"id": "CA5394",
	"shortDescription": "Do not use insecure randomness",
	"fullDescription": "Using a cryptographically weak pseudo-random number generator may allow an attacker to predict what security-sensitive value will be generated. Use a cryptographically strong random number generator if an unpredictable value is required, or ensure that weak pseudo-random numbers aren't used in a security-sensitive manner.",
	"defaultLevel": "warning",
	"helpUri": "https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca5394",
	"properties": {
	"category": "Security",
	"isEnabledByDefault": false,

So this behavior is expected for them as explained in the documentation.

For CA1305, it's enabled by default:

roslyn-analyzers/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif

Lines 1584 to 1592 in b0fe62c

	"CA1305": {
	"id": "CA1305",
	"shortDescription": "Specify IFormatProvider",
	"fullDescription": "A method or constructor calls one or more members that have overloads that accept a System.IFormatProvider parameter, and the method or constructor does not call the overload that takes the IFormatProvider parameter. When a System.Globalization.CultureInfo or IFormatProvider object is not supplied, the default value that is supplied by the overloaded member might not have the effect that you want in all locales. If the result will be based on the input from/output displayed to the user, specify 'CultureInfo.CurrentCulture' as the 'IFormatProvider'. Otherwise, if the result will be stored and accessed by software, such as when it is loaded from disk/database and when it is persisted to disk/database, specify 'CultureInfo.InvariantCulture'.",
	"defaultLevel": "hidden",
	"helpUri": "https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1305",
	"properties": {
	"category": "Globalization",
	"isEnabledByDefault": true,

So, I'd let @mavasani comment on that one.

[RyanThomas73]

@Youssef1313
Yes I was testing with analysis mode set to all and I had already included the link to that configuration in my steps to reproduce.

[mavasani]

This seems to be a duplicate of #5077 (comment). It's a known limitation that we do not support both MSBuild based bulk configuration (AnalysisMode and AnalysisLevel properties) and dotnet_analyzer_severity based bulk configuration in editorconfig/globalconfig.

However, note that your scenario might have a workaround due to a recent fix I made to Roslyn: If you use AnalysisMode or AnalysisLevel to bulk escalate more rules to warnings, and also set TreatWarningsAsErrors/TreatCodeAnalysisWarningsAsErrors in your MSBuild project file/targets/props, then enabled rules should get escalated to errors. You might need the latest VS/compiler for that to work though, the fix was checked in just a few months back...

[mavasani]

You might need the latest VS/compiler for that to work though, the fix was checked in just a few months back...

Actually, the fix was made in the analyzer package itself with #6427. So, this might work by referencing the latest 8.0 preview version of the analyzer package: https://dev.azure.com/dnceng/public/_artifacts/feed/dotnet8/NuGet/Microsoft.CodeAnalysis.NetAnalyzers/overview/8.0.0-preview.23322.1

[RyanThomas73]

@mavasani In light of this information - I believe it would be worth while to update the documentation in the Important call out under this section:
https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-options#scope

It should state that enabling analysis mode (categories or all) will invalidate any category or global severity settings specified via editorconfig, and that the first option using explicit dotnet_diagnostic entries would be needed in such scenarios.

Follow up question:
Is a list of just the disabled by default rules maintained and if so could the above documentation call out provide a link to it? Without a clear and efficient way of identifying which rules require explicit individual config entries, most such rules will remain effectively invisible in many scenarios.

[mavasani]

Thanks @RyanThomas73 - filed dotnet/docs#35984

[mavasani]

Is a list of just the disabled by default rules maintained and if so could the above documentation call out provide a link to it? Without a clear and efficient way of identifying which rules require explicit individual config entries, most such rules will remain effectively invisible in many scenarios.

Yes, a list is maintained at https://github.com/dotnet/roslyn-analyzers/blob/main/src/NetAnalyzers/Core/AnalyzerReleases.Shipped.md. This is enforced to be in sync with the actual severity/enabledByDefault values with each .NET SDK release via tooling.

[RyanThomas73]

Closing this issue as this is a known limitation and Manish has created the above documentation ticket to help reduce potential confusion.