Fix S2589 FN: Support recursive pattern redundant null check

[jnyrup]

Description

When using a property pattern is { } as a null check S2589 does not recognize it as redundant. Is that a bug or a deliberate design decision?

Repro steps

public class Class1
{
    public void A(string s)
    {
        _ = s.GetHashCode();
        if (s is not null) { /* Ignore */ } // TP
    }

    public void B(string s)
    {
        _ = s.GetHashCode();
        if (s != null) { /* Ignore */ } // TP
    }

    public void C(string s)
    {
        _ = s.GetHashCode();
        if (!ReferenceEquals(s, null)) { /* Ignore */ } // TP
    }

    public void E(string s)
    {
        _ = s.GetHashCode();
        if (s is { }) { /* Ignore */ } // FN: null check is not considered redundant
    }
}

Expected behavior

E() should report S2589

Actual behavior

E() does not report S2589

Known workarounds

Use is not null instead of is {} to trigger S2589

Related information

• C#/VB.NET Plugins version
• Visual Studio version: 17.10.0 Preview 5.0
• MSBuild / dotnet version: 8.0
• SonarLint for Visual Studio 2022 7.8.0.88494
• Operating System: Windows 10

[gregory-paidis-sonarsource]

Hey,

That is a good find. As you might know, recursive pattern can be tricky, as it can appear in many, many forms. I will document it with a reproducer in our unit tests, and it will be taken care of at some point in the future, when we tackle Symbolic Execution FP/FNs.

Thanks for raising the issue.

PS: FluentAssertions is awesome!

[jnyrup]

Thanks for looking into this and for the kind words regarding Fluent Assertions

As you might know, recursive pattern can be tricky, as it can appear in many, many forms.

Yeah, I did think of the many forms recursive patterns can take. Without having the faintest insights into your analyzers, I wondered if perhaps is { } could somehow be special cased to be equivalent to a null check and then (for now) ignore the more complex patterns.