Search code examples
c#.netsonarqube

Is this a SonarQube bug? Change this condition so that it does not always evaluate to 'false'; some subsequent code is never executed


SonarQube detects an issue "Change this condition so that it does not always evaluate to 'false'; some subsequent code is never executed" in the following code.

public void SonarPleaseWhy(IList<string> texts)
{
    var chain = new List<string>();

    bool firstItemAdded = false;
    bool otherItemAdded = false;

    foreach (var text in texts)
    {
        if (!firstItemAdded && text.Length == 1)
        {
            chain.Add(text);
            firstItemAdded = true;
        }
        else if (firstItemAdded && text.Length == 2)
        {
            chain.Add(text);
            otherItemAdded = true;
        }
        else if (otherItemAdded && text.Length == 3)
        {
            Console.WriteLine(string.Join(',', chain));
            chain.Clear();
            firstItemAdded = false;
            otherItemAdded = false;
        }
        else
        {
            chain.Clear();
            firstItemAdded = false;
            otherItemAdded = false;
        }
    }
}

Is this a bug in SonarQube, or some real issue in my code? If the latter, please help me, I don't understand.

SonarQube highlights the part text.Length == 3 in red and the body of the subsequent elseif block. For some reason, it does not have any problem with the first elseif block.

I've tried to simplify my code, that's how I reached this (almost) minimal reproducible example but still cannot see the reason.


Solution

  • The following code demonstrates that the body of the else if (otherItemAdded && text.Length == 3) can be entered, and also that the && text.Length == 3 condition is evaluated and necessary:

    using System;
    using System.Collections.Generic;
    
    namespace Console1;
    
    public static class Program
    {
        public static void Main()
        {
            string[] test = { "1", "22", "333", "1", "22", "4444" };
    
            SonarPleaseWhy(test);
        }
    
        public static void SonarPleaseWhy(IList<string> texts)
        {
            bool firstItemAdded = false;
            bool otherItemAdded = false;
    
            foreach (var text in texts)
            {
                if (!firstItemAdded && text.Length == 1)
                {
                    firstItemAdded = true;
                }
                else if (firstItemAdded && text.Length == 2)
                {
                    otherItemAdded = true;
                }
                else if (otherItemAdded && text.Length == 3)
                {
                    Console.WriteLine($"It entered the 'if' with text {text}");
                    firstItemAdded = false;
                    otherItemAdded = false;
                }
                else
                {
                    firstItemAdded = false;
                    otherItemAdded = false;
                }
            }
        }
    }
    

    (Note that I have elided the code manipulating chain from your original code because that plays no part in the logic in question.)

    If you run this, you will see the output:

    It entered the 'if' with text 333
    

    Now comment-out the && text.Length == 3 part of the condition and run it again. This time the output is:

    It entered the 'if' with text 333
    It entered the 'if' with text 4444
    

    This clearly demonstrates both of the points that I asserted above.

    Therefore if SonarQube really is warning you about either of those things, it's a false positive (at least, based only on the code you've shown us).