Skip to content

Commit

Permalink
[ios] test MemoryAnalyzers package (#16346)
Browse files Browse the repository at this point in the history
Context: https://github.com/jonathanpeppers/memory-analyzers
Context: https://www.nuget.org/packages/MemoryAnalyzers/0.1.0-beta.3

This adds a new Roslyn analyzer that warns about the following cases.

 ## MA0001

Don't define `public` events in `NSObject` subclasses:

```csharp
public class MyView : UIView
{
    // NOPE!
    public event EventHandler MyEvent;
}
```

 ## MA0002

Don't declare members in `NSObject` subclasses unless they are:

* `WeakReference` or `WeakReference<T>`
* Value types

```csharp
class MyView : UIView
{
    // NOPE!
    public UIView? Parent { get; set; }

    public void Add(MyView subview)
    {
        subview.Parent = this;
        AddSubview(subview);
    }
}
```

 ## MA0003

Don't subscribe to events inside `NSObject` subclasses unless:

* It's your event via `this.MyEvent` or from a base type.
* The method is `static`.

```csharp
class MyView : UIView
{
    public MyView()
    {
        var picker = new UIDatePicker();
        AddSubview(picker);
        picker.ValueChanged += OnValueChanged;
    }

    void OnValueChanged(object sender, EventArgs e) { }

    // Use this instead and it doesn't leak!
    //static void OnValueChanged(object sender, EventArgs e) { }
}
```

This is also on NuGet, but I just commited the package until we can get
it added to the `dotnet-public` feed.

Places with PRs in flight are marked with:

    [UnconditionalSuppressMessage("Memory", "MA0002", Justification = "FIXME: #16530")]

A few are marked as "not an issue" with an explanation. Others mention a
test with a proof they are OK.

A few places I could actually *remove* `UnconditionalSuppressMessage`
where I could improve the analyzer to ignore that case.
  • Loading branch information
jonathanpeppers authored Aug 21, 2023
1 parent 9fea1f5 commit 2e65626
Show file tree
Hide file tree
Showing 14 changed files with 32 additions and 13 deletions.
1 change: 1 addition & 0 deletions src/Core/src/Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<Import Project="$(MauiSrcDirectory)MultiTargeting.targets" />

<ItemGroup>
<PackageReference Include="MemoryAnalyzers" Version="0.1.0-beta.3" PrivateAssets="All" />
<PackageReference Include="Microsoft.Extensions.Configuration" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" />
<PackageReference Include="Microsoft.Extensions.Logging" />
Expand Down
3 changes: 3 additions & 0 deletions src/Core/src/Platform/iOS/ContainerViewController.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics.CodeAnalysis;
using Microsoft.Maui.HotReload;
using ObjCRuntime;
using UIKit;
Expand All @@ -8,11 +9,13 @@ namespace Microsoft.Maui.Platform
public class ContainerViewController : UIViewController, IReloadHandler
{
IElement? _view;
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: NavigationPageTests.DoesNotLeak")]
UIView? currentPlatformView;

// The handler needs this view before LoadView is called on the controller
// So this is used to create the first view that the handler will use
// without forcing the VC to call LoadView
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: NavigationPageTests.DoesNotLeak")]
UIView? _pendingLoadedView;

public IElement? CurrentView
Expand Down
1 change: 0 additions & 1 deletion src/Core/src/Platform/iOS/MauiCheckBox.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public class MauiCheckBox : UIButton

readonly WeakEventManager _weakEventManager = new WeakEventManager();

[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
public event EventHandler? CheckedChanged
{
add => _weakEventManager.AddEventHandler(value);
Expand Down
6 changes: 3 additions & 3 deletions src/Core/src/Platform/iOS/MauiRefreshView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ public class MauiRefreshView : UIView
bool _isRefreshing;
nfloat _originalY;
nfloat _refreshControlHeight;
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
UIView _refreshControlParent;
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
UIView? _contentView;
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
UIRefreshControl _refreshControl;
public UIRefreshControl RefreshControl => _refreshControl;

Expand Down
6 changes: 5 additions & 1 deletion src/Core/src/Platform/iOS/MauiSearchBar.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System;
using System.ComponentModel.Design;
using System.Diagnostics.CodeAnalysis;
using System.Drawing;
using CoreGraphics;
using Foundation;
Expand Down Expand Up @@ -33,6 +33,7 @@ protected internal MauiSearchBar(NativeHandle handle) : base(handle)
// Native Changed doesn't fire when the Text Property is set in code
// We use this event as a way to fire changes whenever the Text changes
// via code or user interaction.
[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "FIXME: https://github.com/dotnet/maui/pull/16383")]
public event EventHandler<UISearchBarTextChangedEventArgs>? TextSetOrChanged;

public override string? Text
Expand All @@ -51,7 +52,9 @@ public override string? Text
}
}

[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "FIXME: https://github.com/dotnet/maui/pull/16383")]
internal event EventHandler? OnMovedToWindow;
[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "FIXME: https://github.com/dotnet/maui/pull/16383")]
internal event EventHandler? EditingChanged;

public override void WillMoveToWindow(UIWindow? window)
Expand All @@ -74,6 +77,7 @@ public override void WillMoveToWindow(UIWindow? window)
OnMovedToWindow?.Invoke(this, EventArgs.Empty);
}

[UnconditionalSuppressMessage("Memory", "MA0003", Justification = "FIXME: https://github.com/dotnet/maui/pull/16383")]
void OnEditingChanged(object? sender, EventArgs e)
{
EditingChanged?.Invoke(this, EventArgs.Empty);
Expand Down
8 changes: 4 additions & 4 deletions src/Core/src/Platform/iOS/MauiSwipeView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ public class MauiSwipeView : ContentView

readonly SwipeRecognizerProxy _proxy;
readonly Dictionary<ISwipeItem, object> _swipeItems;
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
readonly UITapGestureRecognizer _tapGestureRecognizer;
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
readonly UIPanGestureRecognizer _panGestureRecognizer;
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
UIView _contentView;
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
UIStackView _actionView;
SwipeTransitionMode _swipeTransitionMode;
SwipeDirection? _swipeDirection;
Expand Down
4 changes: 2 additions & 2 deletions src/Core/src/Platform/iOS/MauiTextField.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ public override UITextRange? SelectedTextRange
}
}

[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
public event EventHandler? TextPropertySet;
[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
internal event EventHandler? SelectionChanged;
}
}
4 changes: 2 additions & 2 deletions src/Core/src/Platform/iOS/MauiTextView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.Maui.Platform
{
public class MauiTextView : UITextView
{
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
readonly UILabel _placeholderLabel;
nfloat? _defaultPlaceholderSize;

Expand Down Expand Up @@ -38,7 +38,7 @@ public override void WillMoveToWindow(UIWindow? window)
// Native Changed doesn't fire when the Text Property is set in code
// We use this event as a way to fire changes whenever the Text changes
// via code or user interaction.
[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "Proven safe in test: MemoryTests.DoesNotLeak")]
[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
public event EventHandler? TextSetOrChanged;

public string? PlaceholderText
Expand Down
1 change: 1 addition & 0 deletions src/Core/src/Platform/iOS/MauiTimePicker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ namespace Microsoft.Maui.Platform
{
public class MauiTimePicker : NoCaretField
{
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
readonly UIDatePicker _picker;

#if !MACCATALYST
Expand Down
2 changes: 2 additions & 0 deletions src/Core/src/Platform/iOS/MauiUIApplicationDelegate.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics.CodeAnalysis;
using Foundation;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Maui.Hosting;
Expand Down Expand Up @@ -158,6 +159,7 @@ public virtual void PerformFetch(UIApplication application, Action<UIBackgroundF
Services?.InvokeLifecycleEvents<iOSLifecycle.PerformFetch>(del => del(application, completionHandler));
}

[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "There can only be one MauiUIApplicationDelegate.")]
public static MauiUIApplicationDelegate Current { get; private set; } = null!;

[Export("window")]
Expand Down
2 changes: 2 additions & 0 deletions src/Core/src/Platform/iOS/MauiWKWebView.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics.CodeAnalysis;
using System.Drawing;
using System.IO;
using System.Threading.Tasks;
Expand All @@ -12,6 +13,7 @@ namespace Microsoft.Maui.Platform
{
public class MauiWKWebView : WKWebView, IWebViewDelegate
{
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Used to persist cookies across WebView instances. Not a leak.")]
static WKProcessPool? SharedPool;

string? _pendingUrl;
Expand Down
2 changes: 2 additions & 0 deletions src/Core/src/Platform/iOS/PlatformTouchGraphicsView.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics.CodeAnalysis;
using Foundation;
using Microsoft.Maui.Graphics;
using Microsoft.Maui.Graphics.Platform;
Expand All @@ -10,6 +11,7 @@ public class PlatformTouchGraphicsView : PlatformGraphicsView
{
readonly UIHoverGestureRecognizerProxy _proxy;
WeakReference<IGraphicsView>? _graphicsView;
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
UIHoverGestureRecognizer? _hoverGesture;
RectF _rect;
bool _pressedContained = false;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using UIKit;

namespace Microsoft.Maui.Platform
{
internal class ResignFirstResponderTouchGestureRecognizer : UITapGestureRecognizer
{
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "FIXME: https://github.com/dotnet/maui/pull/16530")]
UIView? _targetView;
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "FIXME: https://github.com/dotnet/maui/pull/16530")]
Token? _token;

public ResignFirstResponderTouchGestureRecognizer(UIView targetView) :
Expand Down
2 changes: 2 additions & 0 deletions src/Core/src/Platform/iOS/WrapperView.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics.CodeAnalysis;
using CoreAnimation;
using CoreGraphics;
using Microsoft.Maui.Graphics;
Expand All @@ -12,6 +13,7 @@ public partial class WrapperView : UIView, IDisposable
CAShapeLayer? _maskLayer;
CAShapeLayer? _backgroundMaskLayer;
CAShapeLayer? _shadowLayer;
[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "_borderView is a SubView")]
UIView? _borderView;

public WrapperView()
Expand Down

0 comments on commit 2e65626

Please sign in to comment.