'X' is null on at least one execution path after null check throw

SonarQube Server / Community Build
1

SonarQube Community Edition Version 8.4.1 (build 35646)

The following code fails with ‘creditPlanEventMessage’ is null on at least one execution path, despite the null check above which will throw an exception if null so execution cannot go past there:

if (creditPlanEventMessage == null)
{
    LogAndThrow("Credit plan event message cannot be null.");
}

if (creditPlanEventMessage.EmailCredits < 0) // 'creditPlanEventMessage' is null on at least one execution path.

void LogAndThrow(string message)
{
    Logger.Error(message);
    throw new BadRequestException(message);
}

Logger is just a static Serilog instance.

EDIT: I ended up solving this by explicitly adding a return after the LogAndThrow call. It is a fair point in the solution post below that we shouldn’t depend on the LogAndThrow call just returning.

2

Add a [DoesNotReturn] attribute to the LogAndThrow method definition. The nullability analysis does not look into method bodies. This both keeps the CPU time used under control and helps avoid depending on implementation details that the programmer may not want to promise indefinitely.

References: Attributes interpreted by the compiler: Nullable static analysis - C# reference | Microsoft Learn , DoesNotReturnAttribute Class (System.Diagnostics.CodeAnalysis) | Microsoft Learn

1 Like
3

And just to add onto @jilles-sg’s answer – Only the latest version of SonarQube Community Build is considered active!

Your upgrade path is:

8.4.1 → 8.9.10 → 9.9.8 → 24.12 → 25.6

You may find these resources helpful:

5

I don’t think it’s a good idea to start writing code to take into account the possibility that a method named LogAndThrow returns. The author of the method would be happy to promise it does not return; otherwise, the method would not be named like that. [DoesNotReturn] should be used.

What I was thinking of is if you have something like

public static void Foo() => throw new NotImplementedException(nameof(Foo));

or

public static void Bar(string? s) {
    if (s is null) {
        throw new NotImplementedException();
    }
    // ...
}

no attributes should be added so the calling code cannot abuse the throws that happen to be there to avoid necessary null checks.

1 Like
6

That is a fair point - I didn’t like doing it just for the sake of keeping Sonar happy.