Probably false-positive "variable is null on at least one execution path"

Hello,

I have a problem understanding on which execution path my varibale should be null.
A simplified Version of the Code looks like this:

static public void DoSomething()
{
    Object o = null;
    if (o == null)
    {
        ThrowError("String identifier for Loading", "some args");
    }
    o.ToString();
}

static private void ThrowError(string p, params object[] args)
{
    string msg = string.Format("Loaded String to Format", args);
    throw new ApplicationException(msg);
}

If i have the exception Thrown in the “DoSomething” method the Rule is not violated, so my question is, what could go wrong that no exception is thrown and the code is further executed? Or is this a false positive?

I Updated to the latest Sonar Scanner for MS Build 4.10 but our Server is still at Version 7.9.4.
Is this the right section for this question?

Hi @Benedikt,

Welcome to our community forum. Our null reference detection is based only on in-proc analysis. So it doesn’t know that ThrowError will always throw an exception. What it reads is:

o is known to be null
if o is null, method called ThrowError is invoked
o.ToString() is called, violating the rule

While it’s a False Positive in this case, I’d say that your method design is against good practices. Imagine rewriting your code a little to

static public string DoSomething(object o)
{
    if (o == null)
    {
        ThrowError("String identifier for Loading", "some args");
    }
    else
    {
        return o.ToString();
    }
}

This doesn’t compile, because compiler isn’t sure that ThrowError actually throws. It’s also not guaranteed that ThrowError will actually throw in case of some more complicated logic.

You should update your code tolike this:

static public void DoSomething()
{
    Object o = null;
    if (o == null)
    {
        throw CreateError("String identifier for Loading", "some args");
    }
    o.ToString();
}

static private Exception CreateError(string p, params object[] args)
{
    string msg = string.Format("Loaded String to Format", args);
    return new ApplicationException(msg);
}

That will be recognized by the compiler as well as our analyzer.

Hi Pavel,

thanks for the reply and your explanation.
I like your proposal and will try to adapt the code to look more like that.

Thank you very much.

Hi Pavel,

thanks for your explanation and proposal. I will try to adapt the code that.
Thanks very much.

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.