Enable additional rules of source code analysis

[Flash0ver]

Description

Summary

• Consider changing implicit <AnalysisMode>Default</AnalysisMode> to explicit <AnalysisMode>All</AnalysisMode>
  • alternatively, we could also just enable a specific subset of rules
  • consider disabling specific rules that do not apply to our code base, or these that we want to review at a later point in time
    • C#: [assembly: SuppressMessage(..)]
    • MSBuild: <NoWarn>$(NoWarn);..</NoWarn>
    • editorconfig or a dedicated .globalconfig
• Pin/Lock ruleset to current SDK in use via <AnalysisLevel>9.0</AnalysisLevel> for deterministic builds, rather than having the implicit <AnalysisLevel>latest</AnalysisLevel> that is depending on the current .NET SDK version in use
• Update Analyzer-Packages to get the latest rules and diagnostics
  • Roslynator.Analyzers for all our .NET/C# code
  • Microsoft.CodeAnalysis.Analyzers for our Roslyn compiler plugins
  • could be separate issue or a parallel PR

Remarks

We currently use implicitly the Default code analysis mode:

<PropertyGroup>
  <AnalysisMode>Default</AnalysisMode>
</PropertyGroup>

which enables a subset of available diagnostics depending on the TFM.

These default rules do not contain diagnostics that could have prevented

• fix: InvalidOperationException sending attachments on Android with LLVM enabled #4276 (comment)

Therefore, we may want to consider explicitly enabling All rules:

<PropertyGroup>
  <AnalysisMode>All</AnalysisMode>
</PropertyGroup>

This ... as the time of creating this issue ... emits

• 3_494 (warnings as) errors
• 1_546 warnings (not as errors)

in our current code-base ... hence the new issue as this may take a day or so to go through all these diagnostics.

For deterministic builds, we may also want to lock the rules reporting diagnostics via

<PropertyGroup>
  <AnalysisLevel>9.0</AnalysisLevel>
</PropertyGroup>

to pin the ruleset to the current SDK in use, and also to review new diagnostics reported in newer .NET SDKs separately from upgrading the .NET SDK to not conflate too many concerns into one changeset to author and review.

This overrides the implicit

<PropertyGroup>
  <AnalysisLevel>latest</AnalysisLevel>
</PropertyGroup>

which uses a ruleset depending on the .NET SDK in use (see dotnet --version ).

Configuration

To configure our ruleset, we could consider

• in C# code, use the [SuppressMessage]Attribute
  • on a per-file level
  • on a per-project level via [assembly: SuppressMessage(..)]
• via MSBuild
  • <NoWarn>$(NoWarn);..</NoWarn>
• via .editorconfig
  • however, this conflates style choices and code analysis configuration
• via .globalconfig
  • I personally quite like configuring rules via .globalconfig, as it's a dedicated file for Code Analysis Rules, single-responsibility-style

Alternative

• We could control the Mode per category for a more fine-grained ruleset via AnalysisLevel<Category>
  • I'd advocate for enabling 9.0 for all categories though, and suppress specific diagnostics that are not relevant for use (at this time) with a potential review later on
• We could control the Mode per category for a more fine-grained ruleset via AnalysisMode<Category>
  • I'd advocate for enabling All for all categories though, and suppress specific diagnostics that are not relevant for use (at this time) with a potential review later on

Experiment

Start a branch and discover if enabling all or more Diagnostics per default is something that we want to do.

See also

• https://learn.microsoft.com/dotnet/fundamentals/code-analysis/overview
  • and further resources linked from that page
• https://learn.microsoft.com/dotnet/fundamentals/code-analysis/configuration-files

Related issues

• Investigate CA2007 checking #2308
• Add string.Format analyzer to IDiagnosticLogger.Log methods #3926