Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix S2583 FP: Delegate unsubscription #8457

Merged
merged 3 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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))
Expand Down
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -3483,3 +3483,36 @@ public static void Foo()
}
}
}

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

this[42] += Callback;
this[42] -= Callback;
if (this[42] == null) // Compliant
{
Console.WriteLine();
}
}

private void Callback() { }
}