diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CompoundAssignment.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CompoundAssignment.cs index 3a871f50dbd..06f775539fd 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CompoundAssignment.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CompoundAssignment.cs @@ -29,6 +29,7 @@ protected override ICompoundAssignmentOperationWrapper Convert(IOperation operat protected override ProgramState Process(SymbolicContext context, ICompoundAssignmentOperationWrapper assignment) => ProcessNumericalCompoundAssignment(context.State, assignment) + ?? ProcessDelegateCompoundAssignment(context.State, assignment) ?? ProcessCompoundAssignment(context.State, assignment) ?? context.State; @@ -51,6 +52,28 @@ private static ProgramState ProcessNumericalCompoundAssignment(ProgramState stat } } + private static ProgramState ProcessDelegateCompoundAssignment(ProgramState state, ICompoundAssignmentOperationWrapper assignment) + { + // When the -= operator is used on a delegate instance (for unsubscribing), + // it can leave the invocation list associated with the delegate empty. In that case the delegate instance will become null. + // https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/subtraction-operator#delegate-removal + // For this reason, we remove the NotNull constraint from the delegate instance. + if (assignment.Target.Type.TypeKind == TypeKind.Delegate + && assignment.OperatorKind == BinaryOperatorKind.Subtract + && state.HasConstraint(assignment.Target, ObjectConstraint.NotNull)) + { + var value = (state[assignment.Target] ?? SymbolicValue.Empty).WithoutConstraint(ObjectConstraint.NotNull); + return state + .SetOperationValue(assignment, value) + .SetOperationValue(assignment.Target, value) + .SetSymbolValue(assignment.Target.TrackedSymbol(state), value); + } + else + { + return null; + } + } + private static ProgramState ProcessCompoundAssignment(ProgramState state, ICompoundAssignmentOperationWrapper assignment) { if ((state.HasConstraint(assignment.Target, ObjectConstraint.NotNull) && state.HasConstraint(assignment.Value, ObjectConstraint.NotNull)) 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 3ed45d8673b..6d5bb711a33 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.CSharp8.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.CSharp8.cs @@ -496,21 +496,6 @@ int SwitchExpression() } } -// https://github.com/SonarSource/sonar-dotnet/issues/8094 -class Repro8094 -{ - public void TestMethod() - { - Action? someDelegate = delegate { }; - someDelegate += Callback; - someDelegate -= Callback; - if (someDelegate == null) // Noncompliant {{Change this condition so that it does not always evaluate to 'False'.}} - { } - } - - private void Callback() { } -} - // https://github.com/SonarSource/sonar-dotnet/issues/8149 class Repro_8149 { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs index 916e8166433..c1a053690cb 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs @@ -3483,3 +3483,52 @@ public static void Foo() } } } + +// Reproducer for https://github.com/SonarSource/sonar-dotnet/issues/8094 +public class Repro_8094 +{ + public Action field; + public Action Prop { get; set; } + public Action this[int index] { get => null; set { _ = value; } } + + public void TestMethod() + { + Action someDelegate = delegate { }; + someDelegate += Callback; + someDelegate -= Callback; + + if (someDelegate == null) // Compliant + { + Console.WriteLine(); + } + + var delegateCopy = someDelegate -= Callback; + if (delegateCopy == null) // Compliant + { + Console.WriteLine(); + } + + field += Callback; + field -= Callback; + if (field == null) // Compliant + { + Console.WriteLine(); + } + + Prop += Callback; + Prop -= Callback; + if (Prop == null) // Compliant + { + Console.WriteLine(); + } + + this[42] += Callback; + this[42] -= Callback; + if (this[42] == null) // Compliant + { + Console.WriteLine(); + } + } + + private void Callback() { } +}