Fix S2259 FP: Raising issue in unreachable code when using declaratio…

…n pattern on unknown value (#8488)

analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs

@@ -93,24 +93,36 @@ internal static IArrayElementReferenceOperationWrapper ToArrayElementReference(t
     internal static IBinaryOperationWrapper ToBinary(this IOperation operation) =>
         IBinaryOperationWrapper.FromOperation(operation);
 
+    internal static IBinaryPatternOperationWrapper ToBinaryPattern(this IOperation operation) =>
+        IBinaryPatternOperationWrapper.FromOperation(operation);
+
     internal static ICompoundAssignmentOperationWrapper ToCompoundAssignment(this IOperation operation) =>
         ICompoundAssignmentOperationWrapper.FromOperation(operation);
 
+    internal static IConstantPatternOperationWrapper ToConstantPattern(this IOperation operation) =>
+        IConstantPatternOperationWrapper.FromOperation(operation);
+
     internal static IConversionOperationWrapper ToConversion(this IOperation operation) =>
         IConversionOperationWrapper.FromOperation(operation);
 
-    internal static IIncrementOrDecrementOperationWrapper ToIncrementOrDecrement(this IOperation operation) =>
-        IIncrementOrDecrementOperationWrapper.FromOperation(operation);
+    internal static IDeclarationPatternOperationWrapper ToDeclarationPattern(this IOperation operation) =>
+        IDeclarationPatternOperationWrapper.FromOperation(operation);
 
-    internal static IInvocationOperationWrapper ToInvocation(this IOperation operation) =>
-        IInvocationOperationWrapper.FromOperation(operation);
+    internal static IEventReferenceOperationWrapper ToEventReference(this IOperation operation) =>
+        IEventReferenceOperationWrapper.FromOperation(operation);
 
     internal static IFieldReferenceOperationWrapper ToFieldReference(this IOperation operation) =>
         IFieldReferenceOperationWrapper.FromOperation(operation);
 
     internal static IFlowCaptureReferenceOperationWrapper ToFlowCaptureReference(this IOperation operation) =>
         IFlowCaptureReferenceOperationWrapper.FromOperation(operation);
 
+    internal static IIncrementOrDecrementOperationWrapper ToIncrementOrDecrement(this IOperation operation) =>
+        IIncrementOrDecrementOperationWrapper.FromOperation(operation);
+
+    internal static IInvocationOperationWrapper ToInvocation(this IOperation operation) =>
+        IInvocationOperationWrapper.FromOperation(operation);
+
     internal static ILocalReferenceOperationWrapper ToLocalReference(this IOperation operation) =>
         ILocalReferenceOperationWrapper.FromOperation(operation);
 
@@ -120,17 +132,23 @@ internal static IMemberReferenceOperationWrapper ToMemberReference(this IOperati
     internal static IMethodReferenceOperationWrapper ToMethodReference(this IOperation operation) =>
         IMethodReferenceOperationWrapper.FromOperation(operation);
 
+    internal static INegatedPatternOperationWrapper ToNegatedPattern(this IOperation operation) =>
+        INegatedPatternOperationWrapper.FromOperation(operation);
+
     internal static IObjectCreationOperationWrapper ToObjectCreation(this IOperation operation) =>
         IObjectCreationOperationWrapper.FromOperation(operation);
 
+    internal static IParameterReferenceOperationWrapper ToParameterReference(this IOperation operation) =>
+        IParameterReferenceOperationWrapper.FromOperation(operation);
+
     internal static IPropertyReferenceOperationWrapper ToPropertyReference(this IOperation operation) =>
         IPropertyReferenceOperationWrapper.FromOperation(operation);
 
-    internal static IParameterReferenceOperationWrapper ToParameterReference(this IOperation operation) =>
-        IParameterReferenceOperationWrapper.FromOperation(operation);
+    internal static IRecursivePatternOperationWrapper ToRecursivePattern(this IOperation operation) =>
+        IRecursivePatternOperationWrapper.FromOperation(operation);
 
-    internal static IEventReferenceOperationWrapper ToEventReference(this IOperation operation) =>
-        IEventReferenceOperationWrapper.FromOperation(operation);
+    internal static ITypePatternOperationWrapper ToTypePattern(this IOperation operation) =>
+        ITypePatternOperationWrapper.FromOperation(operation);
 
     internal static ITupleOperationWrapper ToTuple(this IOperation operation) =>
         ITupleOperationWrapper.FromOperation(operation);

analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/IsPattern.cs

@@ -28,7 +28,7 @@ protected override IIsPatternOperationWrapper Convert(IOperation operation) =>
         IIsPatternOperationWrapper.FromOperation(operation);
 
     protected override SymbolicConstraint BoolConstraintFromOperation(ProgramState state, IIsPatternOperationWrapper operation, bool isLoopCondition, int visitCount) =>
-        BoolConstraintFromConstant(state, operation) ?? BoolConstraintFromPattern(state, operation);
+        BoolConstraintFromConstant(state, operation) ?? BoolConstraintFromPattern(state, state.Constraint<ObjectConstraint>(operation.Value), operation.Pattern);
 
     protected override ProgramState LearnBranchingConstraint(ProgramState state, IIsPatternOperationWrapper operation, bool isLoopCondition, int visitCount, bool falseBranch) =>
         operation.Value.TrackedSymbol(state) is { } testedSymbol
@@ -134,38 +134,38 @@ private static BoolConstraint BoolConstraintFromConstant(ProgramState state, IIs
         return null; // We cannot take conclusive decision
     }
 
-    private static SymbolicConstraint BoolConstraintFromPattern(ProgramState state, IIsPatternOperationWrapper isPattern) =>
-        state[isPattern.Value] is { } value
-        && value.Constraint<ObjectConstraint>() is { } valueConstraint
-        && BoolConstraintFromPattern(state, valueConstraint, isPattern.Pattern) is { } newConstraint
-            ? newConstraint
-            : null;
-
     private static SymbolicConstraint BoolConstraintFromPattern(ProgramState state, ObjectConstraint valueConstraint, IPatternOperationWrapper pattern)
     {
-        return pattern.WrappedOperation.Kind switch
+        var patternOperation = pattern.WrappedOperation;
+        if (patternOperation.Kind is OperationKindEx.DiscardPattern
+            || patternOperation.AsDeclarationPattern() is { MatchesNull: true })
         {
-            OperationKindEx.ConstantPattern when state[As(IConstantPatternOperationWrapper.FromOperation).Value]?.HasConstraint(ObjectConstraint.Null) is true =>
-                BoolConstraint.From(valueConstraint == ObjectConstraint.Null),
-            OperationKindEx.RecursivePattern => BoolConstraintFromRecursivePattern(valueConstraint, As(IRecursivePatternOperationWrapper.FromOperation)),
-            OperationKindEx.DeclarationPattern => BoolConstraintFromDeclarationPattern(valueConstraint, As(IDeclarationPatternOperationWrapper.FromOperation)),
-            OperationKindEx.TypePattern when
-                As(ITypePatternOperationWrapper.FromOperation) is var type
-                && type.InputType.DerivesOrImplements(type.NarrowedType) => BoolConstraint.From(valueConstraint == ObjectConstraint.NotNull),
-            OperationKindEx.NegatedPattern => BoolConstraintFromPattern(state, valueConstraint, As(INegatedPatternOperationWrapper.FromOperation).Pattern)?.Opposite,
-            OperationKindEx.DiscardPattern => BoolConstraint.True,
-            OperationKindEx.BinaryPattern => BoolConstraintFromBinaryPattern(state, valueConstraint, As(IBinaryPatternOperationWrapper.FromOperation)),
-            _ => null,
-        };
-
-        T As<T>(Func<IOperation, T> fromOperation) =>
-             fromOperation(pattern.WrappedOperation);
+            return BoolConstraint.True;
+        }
+        else if (valueConstraint is not null)
+        {
+            return patternOperation.Kind switch
+            {
+                OperationKindEx.ConstantPattern when state[patternOperation.ToConstantPattern().Value]?.HasConstraint(ObjectConstraint.Null) is true =>
+                    BoolConstraint.From(valueConstraint == ObjectConstraint.Null),
+                OperationKindEx.RecursivePattern => BoolConstraintFromRecursivePattern(valueConstraint, patternOperation.ToRecursivePattern()),
+                OperationKindEx.DeclarationPattern => BoolConstraintFromDeclarationPattern(valueConstraint, patternOperation.ToDeclarationPattern()),
+                OperationKindEx.TypePattern when patternOperation.ToTypePattern() is var type && type.InputType.DerivesOrImplements(type.NarrowedType) =>
+                    BoolConstraint.From(valueConstraint == ObjectConstraint.NotNull),
+                OperationKindEx.NegatedPattern => BoolConstraintFromPattern(state, valueConstraint, patternOperation.ToNegatedPattern().Pattern)?.Opposite,
+                OperationKindEx.BinaryPattern => BoolConstraintFromBinaryPattern(state, valueConstraint, patternOperation.ToBinaryPattern()),
+                _ => null,
+            };
+        }
+        else
+        {
+            return null;
+        }
     }
 
     private static BoolConstraint BoolConstraintFromDeclarationPattern(ObjectConstraint valueConstraint, IDeclarationPatternOperationWrapper declaration) =>
         declaration switch
         {
-            { MatchesNull: true } => BoolConstraint.True,
             _ when valueConstraint == ObjectConstraint.Null => BoolConstraint.False,
             _ when declaration.InputType.DerivesOrImplements(declaration.NarrowedType) => BoolConstraint.From(valueConstraint == ObjectConstraint.NotNull),
             _ => null,

analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.LearnConstraint.cs

@@ -803,14 +803,21 @@ public void Branching_LearnsObjectConstraint_DeclarationPattern_ElseIsNull(strin
             .And.ContainSingle(x => x.HasConstraint(ObjectConstraint.NotNull));
     }
 
-    [DataTestMethod]
-    [DataRow("arg is var o")]
-    [DataRow("arg is object o", "T")]
-    public void Branching_LearnsObjectConstraint_DeclarationPattern_NoConstraints(string expression, string argType = "object")
+    [TestMethod]
+    public void Branching_LearnsObjectConstraint_DeclarationPattern_NoConstraints()
     {
-        var validator = CreateIfElseEndValidatorCS(expression, OperationKind.DeclarationPattern, argType);
+        var validator = CreateIfElseEndValidatorCS("arg is object o", OperationKind.DeclarationPattern, "T");
         validator.TagValue("If").Should().BeNull();
-        validator.TagValue("Else").Should().BeNull();
+        validator.TagValue("Else").Should().BeNull();   // Should().HaveOnlyConstraint(ObjectConstraint.Null)
+        validator.TagValue("End").Should().BeNull();
+    }
+
+    [TestMethod]
+    public void Branching_LearnsObjectConstraint_DeclarationPattern_Var_NoConstraints()
+    {
+        var validator = CreateIfElseEndValidatorCS("arg is var o", OperationKind.DeclarationPattern, "object");
+        validator.TagValue("If").Should().BeNull();
+        validator.TagValues("Else").Should().BeEmpty(); // unreachable
         validator.TagValue("End").Should().BeNull();
     }
 

analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.PatternMatching.cs

@@ -252,19 +252,20 @@ public void DeclarationPattern_Discard_DoesNotFail()
     [TestMethod]
     public void DeclarationPattern_Var_PreservePreviousConstraint_DoesNotSetNotNullConstraint()
     {
-        const string code = @"
-if (arg is var value)
-{
-    Tag(""Value"", value);
-    Tag(""Arg"", arg);
-}
-Tag(""End"", arg);";
+        const string code = """
+            if (arg is var value)
+            {
+                Tag("Value", value);
+                Tag("Arg", arg);
+            }
+            Tag("End", arg);
+            """;
         var setter = new PreProcessTestCheck(OperationKind.ParameterReference, x => x.SetSymbolConstraint(x.Operation.Instance.TrackedSymbol(x.State), TestConstraint.First));
         var validator = SETestContext.CreateCS(code, "object arg", setter).Validator;
         validator.ValidateContainsOperation(OperationKind.DeclarationPattern);
         validator.TagValue("Value").Should().HaveOnlyConstraint(TestConstraint.First, "'var' only propagates existing constraints and ObjectConstraint is missing");
         validator.TagValue("Arg").Should().HaveOnlyConstraint(TestConstraint.First, "'var' only propagates existing constraints and ObjectConstraint is missing");
-        validator.TagValues("End").Should().HaveCount(2).And.OnlyContain(x => x != null && x.HasConstraint(TestConstraint.First));     // 2x because value has different states
+        validator.TagValue("End").Should().HaveOnlyConstraint(TestConstraint.First, "'var' only propagates existing constraints and ObjectConstraint is missing");
     }
 
     [TestMethod]
@@ -381,7 +382,7 @@ public void RecursivePatternDeconstructionSubpatternSetBoolConstraint_TwoStates(
     [DataTestMethod]
     [DataRow("objectNull is var a", true)]
     [DataRow("objectNotNull is var a", true)]
-    [DataRow("objectUnknown is var a", null)] // Should be "true". Some patterns always match.
+    [DataRow("objectUnknown is var a", true)]
     [DataRow("objectNull is object o", false)]
     [DataRow("objectNotNull is object o", true)]
     [DataRow("objectNull is int i", false)]

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/NullPointerDereference.cs

@@ -2105,6 +2105,6 @@ void Foo(object o, int i)
         if (o is var x2)
             Console.WriteLine();
         else
-            Console.WriteLine(n.ToString());    // Noncompliant FP: unreachable
+            Console.WriteLine(n.ToString());    // Compliant
     }
 }