Fix S2589 FN: Support recursive pattern redundant null check
Opened this issue · 2 comments
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
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!
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.