8.45.0.54064 many false positives with "xx is null on at least one execution path"

There appears to be a regression with the SonarAnalyzer.CSharp c# nuget update - after upgrading from 8.44.0.52574 to 8.45.0.54064, we received many false positive errors on rule S2259:

foo(46, 32): [S2259] ‘context’ is null on at least one execution path.

code example:

public ITrackable Context(Dictionary<string, string> context)
{
    if (context?.Any() == null) return this;

    foreach (var (k, v) in context)
    {
        Properties[$"c_{k}"] = v;
    }

    return this;
}

In all of the false positives, the c# code is checked with a conditional ? prior to assessing the value, so there shouldn’t be an additional null check required like the case above for the “context” variable in the foreach statement.

Make sure to read this post before raising a thread here:

Hey there.

Thanks for the report.

In this version of SonarAnalyzer.CSharp, S2259 was migrated to a new engine that was able to finally analyze methods that use newer syntax (from C# 9 and C# 10).

And now that this migration was done, there’s more work to do to kill new false-positives. If I understand correctly, null conditionals will be handled soon: S2259: FP for null conditional usage · Issue #3416 · SonarSource/sonar-dotnet · GitHub

understood - unfortunately it’s a regression in that we can’t upgrade the nuget until this is addressed - I think the issue you’ve linked to isn’t the same problem, I believe the regression here is not interpreting the “?” condition when evaluating null on the execution paths.

Hi,
to understand your scenario better, what’s the point of testing

 if (context?.Any() == null) return this;

instead of

 if (context == null) return this;

It seems that the nullability on bool confuses the rule (have have another issue for it in the backlog)

thx @Pavel_Mikula - you’re right that in that case we could just do if (context == null), but looks like nullability in general is having issues - here is a different example:

        if (statuses?.Count > 0)
        {
            // Sonar complains below about statuses is null on one execution path
            var filter = (statuses.Count == 1);
            // Other code....
        }

Yes, nullability and lifted operators have problems. As a workaround, we’ll suppress them in 8.46 release
that should come in roughly 2 weeks.

A proper fix should be part of MMF-2401 - work to migrate S3655 rule about nullable types.

ok thanks @Pavel_Mikula - let me know if you need any help testing/verifying before you release.

Thank you for your help. We should have enough information now.