Skip to content

Commit

Permalink
DisposeFieldsAnalyzer: Recognize (indirect) recursive methods.
Browse files Browse the repository at this point in the history
  • Loading branch information
manfred-brands committed Sep 27, 2023
1 parent 7130e0b commit b6d9d2b
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -771,5 +771,64 @@ public void Test()
// InstancePerTestCase mean test should use IDisposable
RoslynAssert.Valid(analyzer, testCode);
}

[Test]
public void ShouldDealWithDirectRecursiveMethods()
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
private Dictionary<string, bool> _values = new();
[Test]
public void MinimalRepro()
{
Recurse(""help"");
}
private void Recurse(string one, bool keepGoing = true)
{
_values[one] = keepGoing;
if (keepGoing)
{
Recurse(one, false);
}
}
", "using System.Collections.Generic;");

RoslynAssert.Valid(analyzer, testCode);
}

[Test]
public void ShouldDealWithIndirectRecursiveMethods()
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
private Dictionary<string, string> _values = new();
[Test]
public void Indirect()
{
A(""help"");
}
private void A(string one, bool keepGoing = true)
{
if (keepGoing)
{
_values[one] = nameof(A);
B(one, false);
}
}
private void B(string one, bool keepGoing)
{
if (keepGoing)
{
_values[one] = nameof(B);
A(one, false);
}
}
", "using System.Collections.Generic;");

RoslynAssert.Valid(analyzer, testCode);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ private static void AnalyzeAssignedButNotDisposed(

private static void AssignedIn(Parameters parameters, HashSet<string> assignments, IEnumerable<IMethodSymbol> methods)
{
parameters.ResetMethodCallVisits();
foreach (var method in methods)
{
AssignedIn(parameters, assignments, method);
Expand Down Expand Up @@ -263,7 +264,7 @@ memberAccessExpression.Expression is InvocationExpressionSyntax awaitedInvocatio
string? method = GetIdentifier(invocationExpression.Expression);
if (method is not null)
{
if (parameters.IsLocalMethodCall(invocationExpression, out IMethodSymbol? calledMethod))
if (parameters.IsLocalNotYetVisitedMethodCall(invocationExpression, out IMethodSymbol? calledMethod))
{
// We are calling a local method on our class, keep looking for assignments.
AssignedIn(parameters, assignments, calledMethod);
Expand Down Expand Up @@ -362,6 +363,7 @@ private static bool IsDisposableTypeNotRequiringToBeDisposed(ITypeSymbol typeSym

private static void DisposedIn(Parameters parameters, HashSet<string> disposals, IEnumerable<IMethodSymbol> methods)
{
parameters.ResetMethodCallVisits();
foreach (var method in methods)
{
DisposedIn(parameters, disposals, method);
Expand Down Expand Up @@ -408,7 +410,7 @@ awaitInvocationExpression.Expression is MemberAccessExpressionSyntax awaitMember
{
disposals.Add(disposedSymbol);
}
else if (parameters.IsLocalMethodCall(invocationExpression, out IMethodSymbol? calledMethod))
else if (parameters.IsLocalNotYetVisitedMethodCall(invocationExpression, out IMethodSymbol? calledMethod))
{
// We are calling a local method on our class, keep looking for disposals.
DisposedIn(parameters, disposals, calledMethod);
Expand Down Expand Up @@ -521,6 +523,7 @@ private sealed class Parameters
private readonly INamedTypeSymbol type;
private readonly ImmutableHashSet<string> disposeMethods;
private readonly HashSet<string> names;
private readonly HashSet<IMethodSymbol> visitedMethods = new(SymbolEqualityComparer.Default);

public Parameters(SemanticModel model, INamedTypeSymbol type, ImmutableHashSet<string> disposeMethods, HashSet<string> names)
{
Expand All @@ -541,6 +544,22 @@ public bool IsLocalMethodCall(
SymbolEqualityComparer.Default.Equals(calledMethod.ContainingType, this.type);
}

public void ResetMethodCallVisits() => this.visitedMethods.Clear();

public bool IsLocalNotYetVisitedMethodCall(
InvocationExpressionSyntax invocationExpression,
[NotNullWhen(true)] out IMethodSymbol? calledMethod)
{
if (this.IsLocalMethodCall(invocationExpression, out calledMethod) &&
!this.visitedMethods.Contains(calledMethod))
{
this.visitedMethods.Add(calledMethod);
return true;
}

return false;
}

public bool IsDisposalOf(
InvocationExpressionSyntax invocationExpression,
ExpressionSyntax? conditionalTarget,
Expand Down

0 comments on commit b6d9d2b

Please sign in to comment.