Analyzer: prohibit redundant null-guards on non-nullable references in nullable-enabled code (FFS0055)

[credfeto]

Original request

any code with #nullable enable or the project setting to enable nullable should NOT use ArgumentNullException.ThrowIfNull(document); or if(document == null) throw new ArgumentNullException(...) or similar so we need an analyzer to prohibit, unless there's a path through the code e.g bool fn(bool nullable, Doc? document) { if(nullable) { return false; } ArgumentNullException.ThrowIfNull(document); return document.IsUpToDate; }

Problem

In code where nullable reference types are enabled (#nullable enable or the project-level <Nullable>enable</Nullable>), a value declared as a non-nullable reference type (Doc document) is already guaranteed non-null by the compiler/annotation contract. A runtime null-guard such as ArgumentNullException.ThrowIfNull(document); or if (document == null) throw new ArgumentNullException(...) on such a value is redundant noise.

The guard is legitimate, and must not be flagged, when the value's declared type is nullable (Doc? document) - there genuinely is a null case to guard, e.g.:

bool Fn(bool nullable, Doc? document)
{
    if (nullable)
    {
        return false;
    }

    ArgumentNullException.ThrowIfNull(document); // OK: document is Doc? (nullable)
    return document.IsUpToDate;
}

Scope / decisions

• Applies to: any expression whose static type is a non-nullable reference type in an enabled nullable annotation context - parameter, local, field, or property (not just parameters).
• Guard forms to detect:
  1. ArgumentNullException.ThrowIfNull(x) (and the ThrowIfNull(x, "name") overload).
  2. if (x == null) throw new ArgumentNullException(...) and equivalents (x is null, null == x, ReferenceEquals(x, null)) whose body throws ArgumentNullException.
• Not flagged: guards on values whose declared type is nullable (Doc?), value types / Nullable<T>, unconstrained generic type parameters, or when nullable annotations are disabled at that location.

Rule definition

• ID: FFS0055 (next free id - Helpers/Rules.cs currently ends at FFS0054).
• Category: new NullChecking constant in Helpers/Categories.cs (no existing category fits).
• Severity: error (house default via RuleHelpers.CreateRule).

Analyzer design

New analyzer src/FunFair.CodeAnalysis/RedundantNullCheckDiagnosticsAnalyzer.cs (auto-discovered by Roslyn via [DiagnosticAnalyzer(LanguageNames.CSharp)] - no central registration list in this repo). Modelled on ArgumentExceptionAnalysisDiagnosticsAnalyzer and NullableDirectiveDiagnosticsAnalyzer.

1. Register syntax-node actions for InvocationExpression (catches ThrowIfNull) and IfStatement (catches if (x == null) throw).
2. Nullable context gate: only proceed when semanticModel.GetNullableContext(node.SpanStart) has the Annotations flag set at that location; otherwise skip (guard is expected/legitimate there).
3. Resolve the guarded expression's type via the semantic model. Report only when the type is a reference type with NullableAnnotation == NotAnnotated.
   • Skip value types / Nullable<T>.
   • Skip NullableAnnotation == Annotated (declared Doc?) - this is the legitimate case from the example above.
   • Skip unconstrained generic type parameters (NullableAnnotation == None) to avoid false positives.
4. For ThrowIfNull: confirm the invoked symbol is System.ArgumentNullException.ThrowIfNull (semantic check on the method's receiver type, mirroring ArgumentExceptionAnalysisDiagnosticsAnalyzer) before inspecting the single value argument.
5. For the if-throw form: confirm the condition is a null comparison against a single identifier and the if body throws ArgumentNullException, then inspect that identifier per step 3.

Files to change

• src/FunFair.CodeAnalysis/RedundantNullCheckDiagnosticsAnalyzer.cs - new analyzer.
• src/FunFair.CodeAnalysis/Helpers/Rules.cs - add RuleXxx = "FFS0055".
• src/FunFair.CodeAnalysis/Helpers/Categories.cs - add NullChecking category constant.
• README.md - add | FFS0055 | ... | row to the rule table (after FFS0054).
• .globalconfig - add dotnet_diagnostic.FFS0055.severity = error # ... (after FFS0054).
• src/FunFair.CodeAnalysis.Tests/RedundantNullCheckDiagnosticsAnalyzerTests.cs - new tests using DiagnosticAnalyzerVerifier<RedundantNullCheckDiagnosticsAnalyzer>.
• CHANGELOG.md - ### Added entry (via the changelog tool, not by hand).
• No AnalyzerReleases.*.md / .editorconfig changes needed - those mechanisms aren't used in this repo.

Test strategy

Using DiagnosticAnalyzerVerifier<T> (Result(id, message, severity, line, column) + VerifyCSharpDiagnosticAsync):

• Flagged: #nullable enable, non-nullable ref param guarded by ArgumentNullException.ThrowIfNull(x).
• Flagged: non-nullable ref param guarded by if (x == null) throw new ArgumentNullException(nameof(x));.
• Flagged: non-nullable local/field/property guarded (scope covers all of these, not just parameters).
• Not flagged: nullable param Doc? guarded by ThrowIfNull (the motivating example, including the early-return-then-guard shape).
• Not flagged: same guards when nullable context is disabled at that location.
• Not flagged: value-type / Nullable<T> guards; unconstrained generic type parameter.

Verification

• dotnet test on src/FunFair.CodeAnalysis.Tests - new tests pass.
• dotnet build / buildcheck clean - no new analyzer warnings on this repo's own source.