diff --git a/analyzers/its/expected/Net5/Net5--net5.0-S2583.json b/analyzers/its/expected/Net5/Net5--net5.0-S2583.json index aa9576b547d..32af9d3a856 100644 --- a/analyzers/its/expected/Net5/Net5--net5.0-S2583.json +++ b/analyzers/its/expected/Net5/Net5--net5.0-S2583.json @@ -149,6 +149,30 @@ "message": "Change this condition so that it does not always evaluate to 'False'. Some code paths are unreachable.", "location": [ { +"uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Net5/Net5/S3440.cs#L10", +"region": { +"startLine": 10, +"startColumn": 17, +"endLine": 10, +"endColumn": 18 +} +}, +{ +"uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Net5/Net5/S3440.cs#L10", +"region": { +"startLine": 10, +"startColumn": 22, +"endLine": 10, +"endColumn": 23 +} +} +] +}, +{ +"id": "S2583", +"message": "Change this condition so that it does not always evaluate to 'False'. Some code paths are unreachable.", +"location": [ +{ "uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Net5/Net5/S4201.cs#L37", "region": { "startLine": 37, diff --git a/analyzers/its/expected/Net5/Net5--net5.0-S2589.json b/analyzers/its/expected/Net5/Net5--net5.0-S2589.json index 38b1fb234e2..49176110200 100644 --- a/analyzers/its/expected/Net5/Net5--net5.0-S2589.json +++ b/analyzers/its/expected/Net5/Net5--net5.0-S2589.json @@ -51,6 +51,19 @@ "endColumn": 26 } } +}, +{ +"id": "S2589", +"message": "Change this condition so that it does not always evaluate to 'True'.", +"location": { +"uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Net5/Net5/S3440.cs#L11", +"region": { +"startLine": 11, +"startColumn": 17, +"endLine": 11, +"endColumn": 18 +} +} } ] } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/NumberConstraint.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/NumberConstraint.cs index 770b6823a56..55724ee5599 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/NumberConstraint.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/NumberConstraint.cs @@ -87,6 +87,11 @@ public static NumberConstraint From(BigInteger? min, BigInteger? max) public bool CanContain(BigInteger value) => !(value < Min || Max < value); + public bool Overlaps(NumberConstraint other) => + other is null // NumberConstraint.From(null, null) returns null and should be considered an unlimited range that overlaps with every other range + || ((Min is null || other.Max is null || Min <= other.Max) + && (Max is null || other.Min is null || Max >= other.Min)); + public override bool Equals(object obj) => obj is NumberConstraint other && other.Min == Min && other.Max == Max; diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/IsPattern.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/IsPattern.cs index a075c42726d..598043cac01 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/IsPattern.cs +++ b/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) => - BoolContraintFromConstant(state, operation) ?? BoolConstraintFromPattern(state, operation); + BoolConstraintFromConstant(state, operation) ?? BoolConstraintFromPattern(state, operation); protected override ProgramState LearnBranchingConstraint(ProgramState state, IIsPatternOperationWrapper operation, bool isLoopCondition, int visitCount, bool falseBranch) => operation.Value.TrackedSymbol(state) is { } testedSymbol @@ -100,19 +100,28 @@ private static SymbolicConstraint ConstraintFromDeclarationPattern(IDeclarationP } } - private static BoolConstraint BoolContraintFromConstant(ProgramState state, IIsPatternOperationWrapper isPattern) + private static BoolConstraint BoolConstraintFromConstant(ProgramState state, IIsPatternOperationWrapper isPattern) { if (state[isPattern.Value] is { } value && isPattern.Pattern.WrappedOperation.Kind == OperationKindEx.ConstantPattern - && IConstantPatternOperationWrapper.FromOperation(isPattern.Pattern.WrappedOperation).Value.ConstantValue.Value is bool boolConstant) + && IConstantPatternOperationWrapper.FromOperation(isPattern.Pattern.WrappedOperation).Value is var constantPattern + && constantPattern.ConstantValue.Value is { } constant) { - if (value.Constraint() is { } valueConstraint) + if (constant is bool boolConstantConstraint) { - return BoolConstraint.From(valueConstraint == BoolConstraint.From(boolConstant)); + if (value.Constraint() is { } valueBoolConstraint) + { + return BoolConstraint.From(valueBoolConstraint == BoolConstraint.From(boolConstantConstraint)); + } + else if (value.HasConstraint(ObjectConstraint.Null)) + { + return BoolConstraint.False; + } } - else if (value.HasConstraint(ObjectConstraint.Null)) + else if (state.Constraint(constantPattern) is { } constantNumberConstraint + && value.Constraint() is { } valueNumberConstraint) { - return BoolConstraint.False; + return BoolConstraint.From(valueNumberConstraint.Overlaps(constantNumberConstraint)); } } return null; // We cannot take conclusive decision diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Constraints/NumberConstraintTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Constraints/NumberConstraintTest.cs index 9d6435d3cc0..73e1a351649 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Constraints/NumberConstraintTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Constraints/NumberConstraintTest.cs @@ -223,4 +223,42 @@ public void CanContain_True(int? min, int? max, int value) => [DataRow(42, null, 41)] public void CanContain_False(int? min, int? max, int value) => NumberConstraint.From(min, max).CanContain(value).Should().BeFalse(); + + [DataTestMethod] + [DataRow(null, 42, null, 42)] + [DataRow(null, 42, null, 0)] + [DataRow(null, 42, 0, 0)] + [DataRow(null, 42, 0, 100)] + [DataRow(null, 42, 0, null)] + [DataRow(null, 42, null, null)] + [DataRow(0, null, 0, null)] + [DataRow(0, null, 0, 42)] + [DataRow(0, null, 10, 42)] + [DataRow(0, null, -10, 42)] + [DataRow(0, null, null, 42)] + [DataRow(0, null, null, null)] + [DataRow(0, 42, 0, 42)] + [DataRow(0, 42, -10, 42)] + [DataRow(0, 42, null, 42)] + [DataRow(0, 42, -10, 10)] + [DataRow(0, 42, 10, 20)] + [DataRow(0, 42, 10, 100)] + [DataRow(0, 42, 10, null)] + [DataRow(0, 42, 0, 100)] + [DataRow(0, 42, 0, null)] + [DataRow(0, 42, null, null)] + public void Overlaps_True(int? min, int? max, int? otherMin, int? otherMax) => + NumberConstraint.From(min, max).Overlaps(NumberConstraint.From(otherMin, otherMax)).Should().BeTrue(); + + [DataTestMethod] + [DataRow(42, null, null, 0)] + [DataRow(42, null, -10, 0)] + [DataRow(42, 100, null, 0)] + [DataRow(42, 100, -10, 0)] + [DataRow(null, 0, 42, null)] + [DataRow(null, 0, 42, 100)] + [DataRow(-10, 0, 42, null)] + [DataRow(-10, 0, 42, 100)] + public void Overlaps_False(int? min, int? max, int? otherMin, int? otherMax) => + NumberConstraint.From(min, max).Overlaps(NumberConstraint.From(otherMin, otherMax)).Should().BeFalse(); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.PatternMatching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.PatternMatching.cs index 1a30fb5459e..30317d1ad3d 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.PatternMatching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.PatternMatching.cs @@ -325,6 +325,25 @@ public void ConstantPatternSetBoolConstraint_SingleState(string isPattern, bool? public void ConstantPatternSetBoolConstraint_TwoStates(string testedSymbol, string isPattern, ConstraintKind[] expectedForTrue, ConstraintKind[] expectedForFalse) => ValidateSetBoolConstraint_TwoStates(testedSymbol, isPattern, OperationKindEx.ConstantPattern, expectedForTrue, expectedForFalse); + [DataTestMethod] + [DataRow("i == 42", 42, true)] + [DataRow("i == 16", 42, false)] + [DataRow("i >= 42", 42, true)] + [DataRow("i <= 42", 42, true)] + [DataRow("i > 42", 42, false)] + [DataRow("i < 42", 42, false)] + public void ConstantNumberPatternSetBoolConstraint(string rangeCondition, int constant, bool expectedBoolConstraint) + { + var code = $$""" + if ({{rangeCondition}}) + { + var result = i is {{constant}}; + Tag("Result", result); + } + """; + SETestContext.CreateCS(code, "int i").Validator.TagValue("Result").Should().HaveOnlyConstraints(BoolConstraint.From(expectedBoolConstraint), ObjectConstraint.NotNull); + } + [DataTestMethod] [DataRow("objectNotNull is { }", true)] [DataRow("objectNotNull is object { }", true)] diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.CSharp8.cs index 7ef0f3d2aa1..0c779520c17 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.CSharp8.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.CSharp8.cs @@ -49,43 +49,44 @@ int SwitchExpression_Results() a = false switch { _ => false }; - if (a) // Noncompliant + if (a) // Noncompliant { - return 42; // Secondary + return 42; // Secondary } a = false switch { - false => false, // Noncompliant - _ => false // Secondary + false => false, // Noncompliant + _ => false // Secondary }; - if (a) // Noncompliant + if (a) // Noncompliant { - return 42; // Secondary + return 42; // Secondary } a = false switch { - true => true, // Noncompliant - // Secondary@-1 + true => true, // Noncompliant + // Secondary@-1 _ => false }; - if (a) // Noncompliant + if (a) // Noncompliant { - return 42; // Secondary + return 42; // Secondary } a = 0 switch { - 1 => true, // FN - _ => false // Compliant, we don't raise in default statement + 1 => true, // Noncompliant + // Secondary@-1 + _ => false // Compliant, we don't raise in default statement }; - if (a) + if (a) // Noncompliant { - return 42; + return 42; // Secondary } return 0; }