CommunityToolkit / Maui

The .NET MAUI Community Toolkit is a community-created library that contains .NET MAUI Extensions, Advanced UI/UX Controls, and Behaviors to help make your life as a .NET MAUI developer easier
https://learn.microsoft.com/dotnet/communitytoolkit/maui
MIT License
2.27k stars 397 forks source link

[Proposal] Add `MemoryAnalyzer` #1392

Open brminnick opened 1 year ago

brminnick commented 1 year ago

Feature name

Add MemoryAnalyzer

Link to discussion

https://github.com/CommunityToolkit/Maui/pull/1371

Progress tracker

Summary

This Proposal would add a new Roslyn Analyzer: MemoryAnalyzers, a set of Roslyn C# code analyzers for finding memory leaks in iOS and MacCatalyst applications, created by @jonathanpeppers on Microsoft's .NET MAUI team.

Motivation

The .NET MAUI team adopted MemoryAnalyzers, adding it to their code base in this PR: https://github.com/dotnet/maui/pull/16346/files

We should strive to avoid introducing new memory leaks and this analyzer will aid us in avoiding them.

In addition, we strive to mirror the .NET MAUI repo as closely as possible to make it easy for us to promote features from the .NET MAUI Community Toolkit into .NET MAUI. Implementing MemoryAnalzyers will ensure our code is production ready for .NET MAUI when the time comes to promote a feature.

Detailed Design

Directory.Build.props

We'll set <WarningsAsErrors> for every MemoryAnalyzer analyzer

    <!-- WarningsAsErrors
     CS0419: Ambiguous reference in cref attribute 
     CS1570: XML comment has badly formed XML 'Expected an end tag for element [parameter] 
     CS1571: XML comment on [construct] has a duplicate param tag for [parameter] 
     CS1572: XML comment has a param tag for '[parameter]', but there is no parameter by that name 
     CS1573: Parameter has no matching param tag in the XML comment 
     CS1574: XML comment has cref attribute that could not be resolved 
     CS1580: Invalid type for parameter 'parameter number' in XML comment cref attribute 
     CS1581: Invalid return type in XML comment cref attribute 
     CS1584: XML comment has syntactically incorrect cref attribute 
     CS1589: The syntax of a tag which referenced a file was incorrect 
     CS1590: Invalid XML include element Missing file attribute 
      CS1592: Badly formed XML in included comments file 
      CS1598: XML parser could not be loaded. The XML documentation file will not be generated. 
      CS1658: Identifier expected; 'true' is a keyword 
      CS1734: XML comment has a paramref tag, but there is no parameter by that name 
      MA0001: Don't define public events in NSObject subclasses
      MA0002: Don't declare members in NSObject subclasses unless they are WeakReference, WeakReference<T>, or Value types
      MA0003: Don't subscribe to events inside NSObject subclasses unless it's your event (via this.MyEvent) or inherited from a base type, or the method is static -->
      <WarningsAsErrors>nullable,CS0419,CS1570,CS1571,CS1572,CS1573,CS1574,CS1580,CS1581,CS1584,CS1589,CS1590,CS1592,CS1598,CS1658,CS1734,MA0001,MA0002,MA0003</WarningsAsErrors>

Every CSPROJ file

In every .csproj file, we will add a Private dependency to MemoryAnalyzers

   <PackageReference Include="MemoryAnalyzers" Version="0.1.0-beta.3">
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>

Device Unit Tests

We will write platform-specific unit tests for iOS, Android, Tizen, Windows and MacCatalyst to ensure that the changes proposed by MemoryAnalyzers does not break any existing behavior.

Usage Syntax

N/A

Drawbacks

We are not able to implement this feature until Device Tests are available. I.e. we are currently not able to write platform-specific unit tests for frameworks such as net7.0-ios.

Alternatives

None

Unresolved Questions

No response

brminnick commented 1 year ago

Adding the blocked label until Device Unit Tests are available

bijington commented 2 months ago

@brminnick thanks for assigning this over. I'll be honest I didn't know this existed.

One question - what do you think adding the Nuget reference into Directory.props? That way new projects won't have to include it

brminnick commented 2 months ago

The only hesitation I have is that it may affect our Unit Test projects, causing some extra headaches when writing tests. But, it likely won't, so it's probably fine to add it in Directory.Build.props