Skip to content

BRO1147: Remove a redundant null-forgiving operator

A null-forgiving ! on a value the compiler already knows isn't null hides nothing: removing it changes no warning. Not a StyleCop rule: from Sonar's S8969; the SDK's IDE0370 is the same idea (see below). Off by default, also in the preset.

Example

root = true

[*.cs]
dotnet_diagnostic.BRO1147.severity = warning

Before

public static class Greeting
{
    public static string For(string? name)
    {
        if (name is null)
        {
            return "Hello!";
        }

        return "Hello, " + name!.Trim() + "!";
    }
}

After

public static class Greeting
{
    public static string For(string? name)
    {
        if (name is null)
        {
            return "Hello!";
        }

        return "Hello, " + name.Trim() + "!";
    }
}

Why

A ! says "I know better than the compiler". Where the compiler already agrees, it's noise, and worse: when the code above changes so that the value can be null again, the stale ! silences the warning that would have pointed at it.

How the fix works

It removes the !. A ! is reported when its operand's nullable flow state, without the !, is not-null: the compiler analyzes the enclosing statement (or initializer, or expression body) again with the ! removed, from the state the original had there.

Not reported

  • null! and default! (default(T)! too): they say "this is set later" on purpose.
  • Operands whose type has type arguments (List<string?>, (string, int)), arrays and type parameters: the ! also hides nested mismatches (List<string?> passed as List<string>) and flow states that depend on the type argument, which the operand's own state doesn't show. Type arguments that are plain value types are fine (int?, (int, int)).
  • ref and out arguments (N(ref s!)): their declared type must match the parameter's, whatever the state.
  • Nullable analysis off, or #nullable changing the project's setting where the ! is (a #nullable enable file in a project without <Nullable>enable</Nullable>, a #nullable disable annotations region).
  • Projects with several target frameworks. Nullable annotations of the base library differ per framework: after string.IsNullOrEmpty(s), s is not null on .NET Core 3.0 and later but unknown on .NET Framework and netstandard2.0, so the same ! is redundant in one compilation and needed in the other, and dotnet format writes a fix into the file every framework shares. The package's build targets pass the frameworks as build_property.StyleBroTargetFrameworks (see modernizing.md); a project that references StyleBro's DLLs directly, without the package, doesn't get the guard.
  • A comment or line break between the operand and the !.
  • Constructor initializers and attribute arguments (no statement around them to analyze again).

Configuration

[*.cs]
# true: a '!' on a value declared without nullable annotations stays, like Sonar's S8969 (default false)
stylebro_allow_null_forgiving_on_unannotated = false

A property, field or method declared under #nullable disable or in a library without nullable annotations has no annotation, and the compiler treats its value as not null without knowing it: legacy.Name! is redundant to the compiler, but the ! documents that it was checked. With true it stays, also on a var local initialized from such a value (var name = legacy.Name; ... name!); explicitly typed locals and parameters are still checked, as in S8969. stylebro-migrate writes it for a repository that enforces S8969.

Compared with the SDK's IDE0370

IDE0370 (SDK 10) removes the !s too. It reports in a build and dotnet format fixes it, but only when its severity is set explicitly (dotnet_diagnostic.IDE0370.severity = warning; it is EnforceOnBuild.WhenExplicitlyEnabled, which is why a first probe saw nothing). It decides differently: it removes every ! in a member, compiles the file again and reports them all when the member has no new warning, or none of them. So it also removes a ! on a maybe-null value that goes where null is allowed (N(t!) with a string? parameter), keeps every ! of a member where one is needed, and needs SDK 10. In a multi-targeted project its fix leaves the ! (it checks every framework's copy) but the build keeps reporting it. BRO1147 decides per ! from the flow state, runs with older SDKs and stays quiet in multi-targeted projects. Turning both on is harmless: they remove the same !.

Compared with StyleCop

StyleCop has no such rule. stylebro-migrate leaves it off.

Compared with Sonar's S8969

With stylebro_allow_null_forgiving_on_unannotated = true (what stylebro-migrate writes for S8969), BRO1147 removes no ! that S8969 accepts. Sonar also reports a ! that hides a nested nullability mismatch, which BRO1147 keeps (its type has type arguments): Buildalyzer passes GetEffectiveEnvironmentVariables(env)!, a Dictionary<string, string>, to a Dictionary<string, string?> parameter; without the ! the compiler warns (CS8620), so Sonar's S8969 there stays for a person (that project also targets two frameworks, which BRO1147 skips anyway).