Null Reference Check

I’m currently on SonarQube Enterprise edition 7.7 and was investigating some code that I believe should yield an error but does not. I’ve created a simple code example posted below for reference. I would have thought the “if” check on the length of Myfoo and foo would yield a SonarQube warning as they are obvious null reference exceptions waiting to happen, but SonarQube does not produce any warnings.

public class Foo
{
    public string Name { get; set; } = null; 
}

internal class SonarProgram
{
    string MyFoo { get; set; } = null; 

    internal void Main()
    {
        var foo = new Foo(); 
        if (MyFoo.Length > 0 || foo.Name.Length > 0)
            Console.WriteLine("Length"); 
    }
}

Hi, what is the version of the C# analyzer you are using?

Hello Brian,

Rule S2259 is based on Symbolic Execution. It walks through every path of program execution and looks for possible null dereferencing invocations. Analysis is done on method level only. Main concern here is to find good balance between “find all we can” and “do not report on too many false positives”.

Your code example can definitely lead to null reference exception and this case is not handled by S2259 and therefore not reported by SonarQube.

Property MyFoo can be updated to null/not null anytime by any method within the class or it’s inheritors (or generally any method outside the class or outside the assembly for public properties). So we cannot tell if MyFoo definitely is or is not null. For this reason we do not raise S2259 for properties to avoid too many false positives.

1 Like

Thanks for clarifying why no warnings were being produced. I guess I wish that there was a separate rule (2259b) was created that would have warned my developers and allow each team/company to decide whether they wanted to be made aware of the issues and incur the possible false positives or turn that off.

That would unfortunately produce a lot of false positives even in clear situations like

string MyFoo { get; set; } = "not null";

or

    public SonarProgram()
    {
        MyFoo = "always initialized";
    }

because in both cases

  • inheriting class could reset the value to null in theory and
  • analysis is done on method level only, so initialization is not visible in practice.

In code, it would require you to do null check for every field/property. Related to this, there exists rule S3900 (not part of SonarWay) that validates null checks for arguments of public methods.