C#: False positive variable is null on at least one execution path

I have a piece of code that looks like this:

public int Compare(... left, ... right)
{
  string leftName = ...;
  string rightName = ...;
  if (string.Equals(leftName, rightName))
    return 0;

  if (leftName == null)
  {
    ERROR: 'rightName' is null on at least one execution path.
    if (rightName.EndsWith("module.js"))

To me we are in one of two cases here:

  1. SonarQube simply doesn’t know, but then it shouldn’t report it with the wording it does, instead it should be a warning with “rightName may be null”…
  2. SonarQube think it knows that rightName can be null but actually gets that completely wrong.

IF leftName is null in this block, rightName cannot be null, otherwise the “string.Equals” would have returned from the method, hence we KNOW that rightName holds a value in this context.

(This is even verified with a Unit test)

hi @jeme, welcome to our community!

I tried to reproduce with the latest SonarCsharp analyzer and could not. This is my reproducer attempt:

    class TestCommunityFP
    {
        public int Compare(string leftName, string rightName)
        {
            if (string.Equals(leftName, rightName))
                return 0;

            if (leftName == null)
            {
                if (rightName.EndsWith("module.js")) // ok
                {
                    return 1;
                }
            }
            return 0;
        }
    }

If you ignored such a case previously, can you dig it out again?, otherwise I am not sure I can get to verify it in our code base any time soon :(.

Will try though if there is no easier option.

I tried to reproduce it with SonarCSharp 7.15 and 8.0 and I could not. If you’re on a really old analyzer version, please update.

What version of SonarQube are you on?
What version of the SonarCSharp plugin are you using?

I am using the scanner from here: https://docs.sonarqube.org/latest/analysis/scan/sonarscanner-for-msbuild/

After your response here I updated from:

  • sonar-scanner-msbuild-4.6.0.1930-net46.zip
    to
  • sonar-scanner-msbuild-4.7.1.2311-net46.zip
    (The latest one I could find under that link)

But As i stated, I did ignore it previously, so wouldn’t I have to somehow “Unignore” it for it to pop up again (if it does)?

The scanner downloads the plugin (which is used to analyze the code) from your SonarQube instance. The plugin version can be checked either in the scanning logs for the Begin step, or in your SonarQube instance.

What do you mean you ignored it? Did you mark it as FP or Won't fix in SQ, and now it appears again as a fresh problem?

Sorry that I have taken a long time to get back to you on this.
We have migrated from a local - per project - deployment of SonarQube (pilot) to running on a company enterprise server.

Because of that the any previous marking of Won’t fix or False positive is obviously gone. (We just started over on the new server)

This problem still appears for us with SonarCSharp v8.6.1, SQ 7.9.2 (LTS). We are not using SonarLint at this point.

Is there any “Cloud hosted” instances of SonarQube for Open Source, then perhaps I could try and see if I could reproduce it in such an environment, which I could then hand directly of to you (environment, repo and all).

I will just add the full code here, with some comments about the logical process, because maybe simplifying it down is why Sonar doesn’t stumble:

    public int Compare(BundleFile left, BundleFile right)
    {
        string leftName = left?.VirtualFile.VirtualPath;
        string rightName = right?.VirtualFile.VirtualPath;

        if (string.Equals(leftName, rightName))
            return 0;

        if (leftName == null)
        {
            // FACT: rightName is not null, otherwise it would have returned in the first if block.
            // ERROR: 'rightName' is null on at least one execution path.
            if (rightName.EndsWith("module.js"))
                return 1;
            if (rightName.Contains("config."))
                return -1;
            return 0;
        }

        if (rightName == null)
        {
            // FACT: leftName is not null, otherwise it would have returned in the first if block.
            if (leftName.EndsWith("module.js"))
                return -1;
            if (leftName.Contains("config."))
                return 1;
            return 0;
        }

        // FACT: rightName is not null, leftName is not null, otherwise it would have returned in one of the if blocks above.
        if (leftName.EndsWith("module.js") && !rightName.EndsWith("module.js"))
            return -1;
        if (leftName.Contains("config.") && !rightName.Contains("config."))
            return 1;
        if (rightName.EndsWith("module.js") && !leftName.EndsWith("module.js"))
            return 1;
        if (rightName.Contains("config.") && !leftName.Contains("config."))
            return -1;

        return 0;
    }

We have another case of the same “Error”, they are not immediately similar, the only thing that did come to mind was that both cases had a use of the Null-conditional operator, I don’t know if that may throw SonarQube off:

    public override bool Matches(JToken token, IJsonValidationContext context)
    {
        switch (token?.Type)
        {
            case JTokenType.Array:
                // ERROR: 'token' is null on at least one execution path.
                // FACT: token is never null here, IF the token was null, token?.Type would return null which would lead to the last case above the default case.
                return !token.HasValues;
            case JTokenType.Integer:
            case JTokenType.Float:
                return 0 == (int) token;
            case JTokenType.String:
                return bool.FalseString.Equals((string) token, StringComparison.OrdinalIgnoreCase);
            case JTokenType.Boolean:
                return !(bool)token;
            case JTokenType.Null:
            case JTokenType.Undefined:
            case null:
                return true;
            default:
                return false;
        }
    }

Hi @jeme

Yes, you can use sonarcloud.io for free with open source projects. It will use the latest release of analyzers.

Thanks a lot for coming back with this example, I opened Fix S2259 FP: object.Equals method recognizes null arguments · Issue #3416 · SonarSource/sonar-dotnet · GitHub .

I could not come with a reproducer. This is what I tried:

    public enum MyEnum
    {
        ONE,
        TWO,
        THREE,
        FOUR,
        FIVE
    }

    public class ValueHolder
    {
        public string Value { get; }
        public MyEnum MyEnum { get; }
    }

    // https://github.com/SonarSource/sonar-dotnet/issues/3416
    public class ReproCommunityIssue3416
    {
        public int Compare(ValueHolder left, ValueHolder right)
        {
            string leftName = left?.Value;
            string rightName = right?.Value;

            if (string.Equals(leftName, rightName))
                // this will return if both are NULL or if they have equal non-null values
                return 0;

            // at this point, leftName can be NULL if rightName is not NULL
            if (leftName == null)
            {
                // rightName is not null
                if (rightName.EndsWith("foo")) // Noncompliant FP
                    return 1;
                return 0;
            }

            return 0;
        }

        public string Foo(ValueHolder valueHolder)
        {
            switch (valueHolder?.MyEnum)
            {
                case MyEnum.ONE:
                    return valueHolder.Value;
                case MyEnum.TWO:
                case MyEnum.THREE:
                    return valueHolder.Value;
                case MyEnum.FOUR:
                    return valueHolder.Value;
                case MyEnum.FIVE:
                case null:
                    return valueHolder.Value; // Noncompliant Ok
                default:
                    return string.Empty;
            }
        }
    }


A post was split to a new topic: SingleOrDefault generating S2259 FP