Fix S2583 FP: Property pattern match in else-if condition

[a10r]

We found some false positive S2583 warnings in our code. ("Change this condition so that it does not always evaluate to 'false'; some subsequent code is never executed.")

Consider the following example:

public class A { public string Inner { get; set; } }
public class B { public string Inner { get; set; } }
public class C { public string Inner { get; set; } }
public class D { public string Inner { get; set; } }

static class Program
{
	static void Main(string[] args)
	{
		var b = new B { Inner = "Test" };
		Demo(b);
	}

	static void Demo(object demo)
	{
		if (demo is A { Inner: var innerA }) // [1]
		{
			Console.WriteLine($"A {innerA}");
		}
		else if (demo is B { Inner: var innerB }) // <-- S2583; [2]
		{
			Console.WriteLine($"B {innerB}");
		}
		else if (demo is C { Inner: string innerC }) // <-- S2583; [3]
		{
			Console.WriteLine($"C {innerC}");
		}
		else if (demo is D { Inner: "Test" }) // <-- S2583; [4]
		{
			Console.WriteLine($"D Test");
		}
	}
}

S2583 is generated for the conditions in the lines marked [2], [3] and [4].

S2583 is not generated for the first condition in the if-else chain (line [1]).

It appears that using a property pattern causes all following property patterns in the same if-else chain to be evaluated incorrectly. For example, changing the condition in line [2] to demo is B b and then accessing the property via b.Inner does not generate a warning for [2], even though it is functionally the same. Also, changing the condition in [1] to demo is A a makes the warning in line [2] disappear, but not the warnings in lines [3] and [4].

The type of property patterns does not seem to matter, as shown in the example. The conditions [2] and [3] are slightly different: [2] only unwraps the property, [3] also does a null-check.

Expected behavior

S2583 should not be raised since all conditions in the example can evaluate to true given a matching object.

Known workarounds

No known workarounds.

Related information

• C#/VB.NET Plugins version: SonarLint for Visual Studio 2019 (4.38.0.36876)
• Visual Studio version: Visual Studio 2019
• MSBuild / dotnet version: .NET Core 3.1
• SonarScanner for .NET version (if used): no
• Operating System: Windows 10

[pavel-mikula-sonarsource]

Hi @a10r,

Thanks for reporting these. I confirm them as False Positive.

[markusschaber]

Just to make sure this will also be covered: It also triggers when the patterns are not about type matching, but just comparing a member value, like in the following fragment (where the else if branch triggers the FP):

        if (poffset is { OffsetValueGetter: 4 })
        {
            // do something
        }
        else if (poffset is { OffsetValueSetter: 8 })
        {
            // do something different
        }
        else // ...

[rjgotten]

It appears that using a property pattern causes all following property patterns in the same if-else chain to be evaluated incorrectly.

It also hits the false positive if the patterns are not connected in an if-else if-else chain.
The only thing required is that you test two disjoint patterns on the same local variable in order.

E.g. the following is enough:

public class Test
{
  public int First { get; set; }
  public int Second { get; set; }

  public static int ShowBug(Test testee)
  {
    if (testee is { First : 0 })
      return 1;

    if (testee is { Second : 0 }) // <-- raises S2583
      return 2;

    return 0;
  }
}

This bug is live in the recent release of SonarLint for Visual Studio and in case of my own projects, it's making a thorough mess of it in some methods heavy on pattern matching.

If a fix can't be made on short term, then a rollback to whatever the previous implementation of this rule was, would be nice instead. Don't really want to roll back SonarLint since the recent version also fixes problems connecting to older instances of SonarQube.

[mary-georgiou-sonarsource]

Fixed by #7750