Skip to content

Commit

Permalink
Better heuristics for MA0003 (#2)
Browse files Browse the repository at this point in the history
.NET MAUI is getting some warnings like:

    src\Core\src\Platform\iOS\MauiSearchBar.cs(73,31):
    error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

But this is a pattern that is OK:

    picker.ValueChanged += _proxy.OnValueChanged;
    //...
    class UIDatePickerProxy
    {
        public void OnValueChanged(object? sender, EventArgs e) { }
    }

Added some logic to allow this pattern.

I continued to emit MA0003 if `UIDatePickerProxy` is an
`NSObject`-subclass.
  • Loading branch information
jonathanpeppers committed Aug 10, 2023
1 parent fbb22a3 commit 3699191
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 2 deletions.
66 changes: 66 additions & 0 deletions MemoryAnalyzers/MemoryAnalyzers.Test/MemoryAnalyzersUnitTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -506,5 +506,71 @@ public MauiDatePicker()
var expected = VerifyCS.Diagnostic("MA0003").WithLocation(0).WithArguments("OnValueChanged");
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

/// <summary>
/// This pattern is used commonly in .NET MAUI, and should be OK
/// </summary>
[TestMethod]
public async Task SubscriptionForProxyClass_OK()
{
var test = """
[Register(Name = "UITextField", IsWrapper = true)]
class UITextField
{
[UnconditionalSuppressMessage("Memory", "MA0001")]
public event EventHandler EditingDidBegin;
}

class MyView : UIView
{
readonly UITextFieldProxy _proxy = new();

public MyView()
{
new UITextField().EditingDidBegin += _proxy.OnEditingDidBegin;
}

class UITextFieldProxy
{
public void OnEditingDidBegin(object sender, EventArgs e) { }
}
}
""";

await VerifyCS.VerifyAnalyzerAsync(test);
}

[TestMethod]
public async Task SubscriptionForProxyClass_NotOK()
{
var test = """
[Register(Name = "UITextField", IsWrapper = true)]
class UITextField
{
[UnconditionalSuppressMessage("Memory", "MA0001")]
public event EventHandler EditingDidBegin;
}

class MyView : UIView
{
[UnconditionalSuppressMessage("Memory", "MA0002")]
readonly UITextFieldProxy _proxy = new();

public MyView()
{
new UITextField().EditingDidBegin += _proxy.{|#0:OnEditingDidBegin|};;
}

// This should warn, because it is NSObject
class UITextFieldProxy : NSObject
{
public void OnEditingDidBegin(object sender, EventArgs e) { }
}
}
""";

var expected = VerifyCS.Diagnostic("MA0003").WithLocation(0).WithArguments("OnEditingDidBegin");
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}
}
}
13 changes: 11 additions & 2 deletions MemoryAnalyzers/MemoryAnalyzers/MemoryAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,23 @@ static void AnalyzeSubscription(SyntaxNodeAnalysisContext context)
var rightInfo = context.SemanticModel.GetSymbolInfo(assignment.Right);
if (rightInfo.Symbol is not IMethodSymbol methodSymbol || methodSymbol.IsStatic)
return; // static methods are fine
if (!IsNSObjectSubclass(methodSymbol.ContainingType))
return; // If the method is on a non-NSObject subclass, it's fine

// Subscribing to events you declare are fine
if (assignment.Left is IdentifierNameSyntax)
return;
if (assignment.Left is MemberAccessExpressionSyntax m && m.Expression is ThisExpressionSyntax)
if (assignment.Left is MemberAccessExpressionSyntax left && left.Expression is ThisExpressionSyntax)
return;

context.ReportDiagnostic(Diagnostic.Create(MA0003Rule, assignment.Right.GetLocation(), methodSymbol.Name));
// We have to calculate the proper location for something like `+= _proxy.OnValueChanged`
Location? location;
if (assignment.Right is MemberAccessExpressionSyntax right)
location = right.Name.GetLocation();
else
location = assignment.Right.GetLocation();

context.ReportDiagnostic(Diagnostic.Create(MA0003Rule, location, methodSymbol.Name));
}

static bool HasUnconditionalSuppressMessage(ISymbol symbol, string expectedCode)
Expand Down

0 comments on commit 3699191

Please sign in to comment.