Fix S2583 FP: Issue is raised when there is comparison to a constant.

[mary-georgiou-sonarsource]

Repro steps

public class TestClass
{
    protected const int Limit = 10;

    protected void Foo()
    {
        for (int week = 0; week < Limit; week++)
        {
            var test = week > 2 ? 0 : 42; // week>2 raises S2583, considering this condition always false
        }
    }
}

When the Limit is declared locally, no issue is raised.
It seems that there's some issue with learning numerical constraints for constants.

The issue has been reported by a community post.

[Arthur-Robinson-Vincit]

It wasn't exclusively constants in my testing, it was any variable defined outside of the method.

[mary-georgiou-sonarsource]

Hi @Arthur-Robinson-Vincit

This is a symbolic execution rule.

In this context, the symbolic execution engine tracks const members and their values, so it should know that Limit == 10 always, and the issue should not have been raised.

In the case of nonconstant class members, their value is not tracked and the engine tries to learn number constraints from the context (just like they are parameters to the method, try to give Limit as an input parameter - you'll get the same FP). In this case, it's an FP for another reason and I will open another issue for it after investigating.

[Arthur-Robinson-Vincit]

Interesting, thanks for your explanation.

[steven-forrrest-vincit]

I am running into something very similar:

static bool Foo(string? someText)
{
    var parts = someText.Replace("F#3=", string.Empty).Replace("\r\n", string.Empty).Split(',');
    if (parts.Length != 5)
    {
        return false;
    }

    for (var i = 0; i < parts.Length; i++)
    {
        switch (i)
        {
            case 0:
                DoSomething();
                break;
            case 1:     // raises S2583 , considering this condition always true
                DoSomething();
                break;
            case 2:
                DoSomething();
                break;
            case 3:
                DoSomething();
                break;
        }
    }
    return true;
}

Is this the same issue as mentioned above?

[Tim-Pohlmann]

@steven-forrrest-vincit This is a different issue. It is caused by the Symbolic Execution engine exploring loops only for two iterations.

[steven-forrrest-vincit]

@steven-forrrest-vincit This is a different issue. It is caused by the Symbolic Execution engine exploring loops only for two iterations.

Okay, will this issue be fixed in a future update?

[Tim-Pohlmann]

The issue could be fixed in multiple ways.

1. Change how we handle loops. We might be able to find a better strategy on how to analyze them.
2. Track collection sizes instead of just whether or not a collection is empty.
3. Track field and property values of objects. This would have a similar effect as 2), we know the value of parts.Length which in turn helps with the loop analysis.
   I want to get 2) and 3) done at some point for sure, but we don't have a fixed roadmap. 1) might come at some point if we find a better approach.