From 2e656264f7f52bc00ce984e984a9e2179846adcf Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 21 Aug 2023 10:56:43 -0500 Subject: [PATCH] [ios] test MemoryAnalyzers package (#16346) 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` * 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: https://github.com/dotnet/maui/pull/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. --- src/Core/src/Core.csproj | 1 + src/Core/src/Platform/iOS/ContainerViewController.cs | 3 +++ src/Core/src/Platform/iOS/MauiCheckBox.cs | 1 - src/Core/src/Platform/iOS/MauiRefreshView.cs | 6 +++--- src/Core/src/Platform/iOS/MauiSearchBar.cs | 6 +++++- src/Core/src/Platform/iOS/MauiSwipeView.cs | 8 ++++---- src/Core/src/Platform/iOS/MauiTextField.cs | 4 ++-- src/Core/src/Platform/iOS/MauiTextView.cs | 4 ++-- src/Core/src/Platform/iOS/MauiTimePicker.cs | 1 + src/Core/src/Platform/iOS/MauiUIApplicationDelegate.cs | 2 ++ src/Core/src/Platform/iOS/MauiWKWebView.cs | 2 ++ src/Core/src/Platform/iOS/PlatformTouchGraphicsView.cs | 2 ++ .../iOS/ResignFirstResponderTouchGestureRecognizer.cs | 3 +++ src/Core/src/Platform/iOS/WrapperView.cs | 2 ++ 14 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/Core/src/Core.csproj b/src/Core/src/Core.csproj index b37dfc2aee93..ed8a447689ce 100644 --- a/src/Core/src/Core.csproj +++ b/src/Core/src/Core.csproj @@ -15,6 +15,7 @@ + diff --git a/src/Core/src/Platform/iOS/ContainerViewController.cs b/src/Core/src/Platform/iOS/ContainerViewController.cs index d7fc21feec7f..e7cad69b780e 100644 --- a/src/Core/src/Platform/iOS/ContainerViewController.cs +++ b/src/Core/src/Platform/iOS/ContainerViewController.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using Microsoft.Maui.HotReload; using ObjCRuntime; using UIKit; @@ -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 diff --git a/src/Core/src/Platform/iOS/MauiCheckBox.cs b/src/Core/src/Platform/iOS/MauiCheckBox.cs index f68afad84251..29bbabfd20cb 100644 --- a/src/Core/src/Platform/iOS/MauiCheckBox.cs +++ b/src/Core/src/Platform/iOS/MauiCheckBox.cs @@ -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); diff --git a/src/Core/src/Platform/iOS/MauiRefreshView.cs b/src/Core/src/Platform/iOS/MauiRefreshView.cs index 11513f6d9b5a..19a13092d4f6 100644 --- a/src/Core/src/Platform/iOS/MauiRefreshView.cs +++ b/src/Core/src/Platform/iOS/MauiRefreshView.cs @@ -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; diff --git a/src/Core/src/Platform/iOS/MauiSearchBar.cs b/src/Core/src/Platform/iOS/MauiSearchBar.cs index 8909d4207d50..2438bd4ac0d5 100644 --- a/src/Core/src/Platform/iOS/MauiSearchBar.cs +++ b/src/Core/src/Platform/iOS/MauiSearchBar.cs @@ -1,5 +1,5 @@ using System; -using System.ComponentModel.Design; +using System.Diagnostics.CodeAnalysis; using System.Drawing; using CoreGraphics; using Foundation; @@ -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? TextSetOrChanged; public override string? Text @@ -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) @@ -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); diff --git a/src/Core/src/Platform/iOS/MauiSwipeView.cs b/src/Core/src/Platform/iOS/MauiSwipeView.cs index c39532f3a465..4bce0ac51877 100644 --- a/src/Core/src/Platform/iOS/MauiSwipeView.cs +++ b/src/Core/src/Platform/iOS/MauiSwipeView.cs @@ -17,13 +17,13 @@ public class MauiSwipeView : ContentView readonly SwipeRecognizerProxy _proxy; readonly Dictionary _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; diff --git a/src/Core/src/Platform/iOS/MauiTextField.cs b/src/Core/src/Platform/iOS/MauiTextField.cs index ec6cc2df5d44..4b3402e69a0e 100644 --- a/src/Core/src/Platform/iOS/MauiTextField.cs +++ b/src/Core/src/Platform/iOS/MauiTextField.cs @@ -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; } } \ No newline at end of file diff --git a/src/Core/src/Platform/iOS/MauiTextView.cs b/src/Core/src/Platform/iOS/MauiTextView.cs index 9819b7c45d2b..6f6640d225be 100644 --- a/src/Core/src/Platform/iOS/MauiTextView.cs +++ b/src/Core/src/Platform/iOS/MauiTextView.cs @@ -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; @@ -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 diff --git a/src/Core/src/Platform/iOS/MauiTimePicker.cs b/src/Core/src/Platform/iOS/MauiTimePicker.cs index d569b6f5f26f..977f4810c9ae 100644 --- a/src/Core/src/Platform/iOS/MauiTimePicker.cs +++ b/src/Core/src/Platform/iOS/MauiTimePicker.cs @@ -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 diff --git a/src/Core/src/Platform/iOS/MauiUIApplicationDelegate.cs b/src/Core/src/Platform/iOS/MauiUIApplicationDelegate.cs index d951a60b4214..0055eec07afd 100644 --- a/src/Core/src/Platform/iOS/MauiUIApplicationDelegate.cs +++ b/src/Core/src/Platform/iOS/MauiUIApplicationDelegate.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using Foundation; using Microsoft.Extensions.DependencyInjection; using Microsoft.Maui.Hosting; @@ -158,6 +159,7 @@ public virtual void PerformFetch(UIApplication application, Action(del => del(application, completionHandler)); } + [UnconditionalSuppressMessage("Memory", "MA0002", Justification = "There can only be one MauiUIApplicationDelegate.")] public static MauiUIApplicationDelegate Current { get; private set; } = null!; [Export("window")] diff --git a/src/Core/src/Platform/iOS/MauiWKWebView.cs b/src/Core/src/Platform/iOS/MauiWKWebView.cs index ba94dd84f760..6894414191e9 100644 --- a/src/Core/src/Platform/iOS/MauiWKWebView.cs +++ b/src/Core/src/Platform/iOS/MauiWKWebView.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using System.Drawing; using System.IO; using System.Threading.Tasks; @@ -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; diff --git a/src/Core/src/Platform/iOS/PlatformTouchGraphicsView.cs b/src/Core/src/Platform/iOS/PlatformTouchGraphicsView.cs index 7c4ef5676bfc..62b175be3d13 100644 --- a/src/Core/src/Platform/iOS/PlatformTouchGraphicsView.cs +++ b/src/Core/src/Platform/iOS/PlatformTouchGraphicsView.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using Foundation; using Microsoft.Maui.Graphics; using Microsoft.Maui.Graphics.Platform; @@ -10,6 +11,7 @@ public class PlatformTouchGraphicsView : PlatformGraphicsView { readonly UIHoverGestureRecognizerProxy _proxy; WeakReference? _graphicsView; + [UnconditionalSuppressMessage("Memory", "MA0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")] UIHoverGestureRecognizer? _hoverGesture; RectF _rect; bool _pressedContained = false; diff --git a/src/Core/src/Platform/iOS/ResignFirstResponderTouchGestureRecognizer.cs b/src/Core/src/Platform/iOS/ResignFirstResponderTouchGestureRecognizer.cs index 59a510f7c4ca..6b950c781a18 100644 --- a/src/Core/src/Platform/iOS/ResignFirstResponderTouchGestureRecognizer.cs +++ b/src/Core/src/Platform/iOS/ResignFirstResponderTouchGestureRecognizer.cs @@ -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) : diff --git a/src/Core/src/Platform/iOS/WrapperView.cs b/src/Core/src/Platform/iOS/WrapperView.cs index f857557f48c7..a1e308605fe9 100644 --- a/src/Core/src/Platform/iOS/WrapperView.cs +++ b/src/Core/src/Platform/iOS/WrapperView.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using CoreAnimation; using CoreGraphics; using Microsoft.Maui.Graphics; @@ -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()