SonarSource / sonar-dotnet

Code analyzer for C# and VB.NET projects
https://redirect.sonarsource.com/plugins/csharp.html
GNU Lesser General Public License v3.0
790 stars 227 forks source link

Fix S2589 FP: Break Inside Loop #8434

Closed groogiam closed 11 months ago

groogiam commented 11 months ago

Description

Please provide a succinct description of your issue. Also please provide the rule ID (e.g. S1234)

Repro steps

The following example results in a false positive.

async Task Main()
{
    MyResult result = null;

    for (var i = 0; i < 300; i++)
    {
        await Task.Delay(10000);

        result = IsOpFinished();

        if (result.Completed != null)
        {
            break;
        }
    }

    if (result?.Completed == null) //s2580 flagged here when it should not be
    {
        throw new TimeoutException();
    }
}

// You can define other methods, fields, classes and namespaces here\
public class MyResult
{
    public DateTime? Completed { get; set; }
}

public static MyResult IsOpFinished()
{
        //actual method implementation would eventually return a completed status
    return new MyResult();
}

Expected behavior

Should not flag s2589

Actual behavior

Flags s2589

Known workarounds

Please provide a description of any known workarounds.

Related information

zsolt-kolbay-sonarsource commented 11 months ago

Thank you for submitting this issue. Confirmed as False Positive.

zsolt-kolbay-sonarsource commented 11 months ago

The issue is a True Positive. The analyzer only complains about the conditional access: Changing result?.Completed == null to result.Completed == null eliminates the warning.