From 6dda3588936be386e575f9874f111899c6ccf122 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay <121798625+zsolt-kolbay-sonarsource@users.noreply.github.com> Date: Wed, 20 Dec 2023 13:11:46 +0100 Subject: [PATCH] Fix S2589 FP: Discard pattern in switch statement (#8485) --- .../Roslyn/Extensions/IOperationExtensions.cs | 3 ++ .../ConditionEvaluatesToConstantBase.cs | 10 ++-- .../ConditionEvaluatesToConstant.CSharp9.cs | 49 ++++++++++++++----- 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs index 0e9119f27ac..688773207cb 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs @@ -72,6 +72,9 @@ OperationKindEx.PropertyReference when operation.ToPropertyReference() is { Prop internal static IPropertyReferenceOperationWrapper? AsPropertyReference(this IOperation operation) => operation.As(OperationKindEx.PropertyReference, IPropertyReferenceOperationWrapper.FromOperation); + internal static IRecursivePatternOperationWrapper? AsRecursivePattern(this IOperation operation) => + operation.As(OperationKindEx.RecursivePattern, IRecursivePatternOperationWrapper.FromOperation); + internal static ITupleOperationWrapper? AsTuple(this IOperation operation) => operation.As(OperationKindEx.Tuple, ITupleOperationWrapper.FromOperation); diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/ConditionEvaluatesToConstantBase.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/ConditionEvaluatesToConstantBase.cs index 8281b2ef6d0..4d66fd98c7e 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/ConditionEvaluatesToConstantBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/ConditionEvaluatesToConstantBase.cs @@ -75,16 +75,18 @@ public override ProgramState ConditionEvaluated(SymbolicContext context) private bool IsIgnored(ProgramState state, IOperation operation) => operation.Kind is OperationKindEx.Literal || IsVarPattern(operation) - || IsDiscardPattern(operation) + || IsDiscardPattern(state, operation) || operation.TrackedSymbol(state) is IFieldSymbol { IsConst: true } || operation.Syntax.Ancestors().Any(x => IsInsideUsingDeclaration(x) || IsLockStatement(x)); private static bool IsVarPattern(IOperation operation) => operation.AsIsPattern()?.Pattern.WrappedOperation.AsDeclarationPattern()?.MatchesNull is true; - private static bool IsDiscardPattern(IOperation operation) => - operation.AsIsPattern() is { } pattern - && pattern.Pattern.WrappedOperation.Kind is OperationKindEx.DiscardPattern; + private static bool IsDiscardPattern(ProgramState state, IOperation operation) => + operation.Kind is OperationKindEx.DiscardPattern + || (operation.AsIsPattern() is { } isPattern + && (isPattern.Pattern.WrappedOperation.Kind is OperationKindEx.DiscardPattern + || (isPattern.Pattern.WrappedOperation.AsRecursivePattern() is {} recursivePattern && recursivePattern.DeconstructionSubpatterns.All(x => IsDiscardPattern(state, x))))); public override void ExecutionCompleted() { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.CSharp9.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.CSharp9.cs index 2b33b3831d0..a851f0bee43 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.CSharp9.cs @@ -269,22 +269,49 @@ var value when value.HasFlag(Flags.Bar) => 2, // Compliant } // https://github.com/SonarSource/sonar-dotnet/issues/8008 -namespace Repro_8008 +public class Repro_8008 { - class Person + public int Compare(string name1, string name2) => (name1, name2) switch { - public int? Age { get; set; } - } + (null, null) => 0, + (null, _) => 1, + (_, null) => -1, + (_, _) => Comparer.Default.Compare(name1, name2) + }; + + public int NestedTuples(string name1, string name2, string name3) => ((name1, name2), name3) switch + { + ((null, null), null) => 42, + ((null, null), _) => 43, + ((_, _), null) => 44, + ((_, _), _) => 45 + }; + + public int DeeperNestedTuples(string name1, string name2, string name3, string name4) => (((name1, name2), name3), name4) switch + { + (((null, null), null), null) => 42, + (((_, _), null), null) => 43, + (((_, _), _), null) => 44, + (((_, _), _), _) => 45, + }; - class Double_Wildcard_FP + public int TupleWithKnownNullConstraint() { - public int Compare(Person x, Person y) => (x.Age, y.Age) switch + string name1 = null; + string name2 = null; + return (name1, name2) switch { - (null, null) => 0, - (null, _) => 1, - (_, null) => -1, - (_, _) => Comparer.Default.Compare(x.Age.Value, y.Age.Value) // Noncompliant {{Change this condition so that it does not always evaluate to 'True'.}} - //^^^^^^ + (null, null) => 0, // FN + (null, _) => 1, // FN + (_, null) => -1, // FN + (_, _) => Comparer.Default.Compare(name1, name2) }; } + + public int TupleWithDeclaration(string name1, string name2) => + (name1, name2) switch + { + ("A", "B") => 0, + (var x, _) => -1, // Noncompliant - FP + }; }