Skip to content

Commit

Permalink
Fix FP for delegate unsubscription
Browse files Browse the repository at this point in the history
  • Loading branch information
zsolt-kolbay-sonarsource committed Dec 11, 2023
1 parent b36349a commit a96061b
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 15 deletions.
Expand Up @@ -29,6 +29,7 @@ internal sealed class CompoundAssignment : SimpleProcessor<ICompoundAssignmentOp

protected override ProgramState Process(SymbolicContext context, ICompoundAssignmentOperationWrapper assignment) =>
ProcessNumericalCompoundAssignment(context.State, assignment)
?? ProcessDelegateCompoundAssignment(context.State, assignment)
?? ProcessCompoundAssignment(context.State, assignment)
?? context.State;

Expand All @@ -51,6 +52,29 @@ 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);
state = state
.SetOperationValue(assignment, value)
.SetOperationValue(assignment.Target, value);
state = state.SetSymbolValue(assignment.Target.TrackedSymbol(state), value);
return state;
}
else
{
return null;
}
}

private static ProgramState ProcessCompoundAssignment(ProgramState state, ICompoundAssignmentOperationWrapper assignment)
{
if ((state.HasConstraint(assignment.Target, ObjectConstraint.NotNull) && state.HasConstraint(assignment.Value, ObjectConstraint.NotNull))
Expand Down
Expand Up @@ -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
{
Expand Down
Expand Up @@ -3483,3 +3483,27 @@ public static void Foo()
}
}
}

// Reproducer for https://github.com/SonarSource/sonar-dotnet/issues/8094
public class Repro_8094
{
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();
}
}

private void Callback() { }
}

0 comments on commit a96061b

Please sign in to comment.