Maui icon indicating copy to clipboard operation
Maui copied to clipboard

Fix: .UseMauiCommunityToolkit() Analyzer when wrapped in preprocessor directives

Open IeuanWalker opened this issue 8 months ago • 1 comments

Description of Change

Linked Issues

  • Fixes #2621

PR Checklist

  • [ ] Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • [ ] Has tests (if omitted, state reason in description)
  • [ ] Has samples (if omitted, state reason in description)
  • [ ] Rebased on top of main at time of PR
  • [ ] Changes adhere to coding standard
  • [ ] Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

Additional information

IeuanWalker avatar Jun 25 '25 20:06 IeuanWalker

Wasn't able to test it via the toolkit .sln directly as it wouldn't build (build was running for 20m+ and not ending). The unit tests also weren't showing up in the test explorer, so I have no idea if the test will pass or fail.

Was able to test it by building the nuget via powershell -

$version = "99.0.0-preview2"
$outputDir = "$PSScriptRoot\nuget"

# Ensure the output directory exists
if (-not (Test-Path $outputDir)) {
    New-Item -ItemType Directory -Path $outputDir | Out-Null
}

# Build solution
dotnet build "D:/WS/Github-Fork/MauiToolkit/src/CommunityToolkit.Maui.sln" -c Release

# Pack nuget
dotnet pack D:/WS/Github-Fork/MauiToolkit/src/CommunityToolkit.Maui/CommunityToolkit.Maui.csproj -c Release --no-restore -p:PackageVersion=$version -o $outputDir
dotnet pack D:/WS/Github-Fork/MauiToolkit/src/CommunityToolkit.Maui.Core/CommunityToolkit.Maui.Core.csproj -c Release --no-restore -p:PackageVersion=$version -o $outputDir

# Show the generated NuGet package(s)
Write-Host "Generated NuGet package(s) in ${outputDir}:"
Get-ChildItem -Path $outputDir -Filter "*.nupkg"

# Keep the PowerShell window open
Read-Host -Prompt "Press Enter to exit"

Have tested that generated nuget against my current solution, and does fix the linked issue

IeuanWalker avatar Jun 25 '25 20:06 IeuanWalker

@TheCodeTraveler thanks, let me know how u get on the copilot.

Atm, the solution isn't building correctly in VS for me to debug so I can see the types, etc.

So if Copilot doesn't work, I'll see if I can spin up a VM in Azure to see if that works.

IeuanWalker avatar Jun 26 '25 17:06 IeuanWalker

let me know how u get on the copilot.

You can use any LLM. Claude.ai is my preference. Just copy paste the analyzer code into Claude and tell it to optimize the code using Roslyn syntax.

You can lean on the existing Unit Tests and the new Unit Test you've added to this PR to test the updated analyzer code without needing a debugger.

For now, I cannot merge this PR until you update it.

TheCodeTraveler avatar Jun 26 '25 18:06 TheCodeTraveler

@TheCodeTraveler Claude is awesome! been playing with Copilot in VS and been impressed, Claude seems to be on another level.

Took a few iterations to get something to work, but all seems ok now 🤞

From Claude itself -

This version is ultra-optimized:

Zero string allocations - No ToString() calls at all
Direct character access - Uses sourceText[i] indexer
Manual search loop - Avoids any internal string operations
Early termination - Stops as soon as we find a match

Performance improvements:

⚡ No memory allocations for strings
⚡ Direct character comparison
⚡ Cache-friendly sequential access pattern
⚡ Minimal overhead - Just simple loops

This should be the fastest possible version while still being robust enough to handle preprocessor directives!

IeuanWalker avatar Jun 26 '25 18:06 IeuanWalker

Ran the unit tests via cmd line and all pass - image

IeuanWalker avatar Jun 26 '25 19:06 IeuanWalker

@TheCodeTraveler hopefully this is better, im guessing there is still going to be an increase in memory but id think it be marginal. The updated code still calls .ToString(), but only as a last resort and only on a subset of the syntax tree

IeuanWalker avatar Jun 26 '25 21:06 IeuanWalker

Excellent work! I'll copy this implementation across our other analyzers and merge the PR shortly.

Analyzer Benchmark (this PR)

Method Mean Error StdDev Median Gen0 Gen1 Allocated
VerifyNoErrorsWhenUseMauiCommunityToolkit 7.610 ms 0.3397 ms 0.9357 ms 7.300 ms 93.7500 31.2500 1.65 MB
VerifyNoErrorsWhenUseMauiCommunityToolkitHasAdditionalWhitespace 7.538 ms 0.2848 ms 0.7699 ms 7.346 ms 93.7500 31.2500 1.65 MB
VerifyErrorsWhenMissingUseMauiCommunityToolkit 12.294 ms 0.7193 ms 2.0169 ms 11.378 ms 133.3333 66.6667 2.36 MB

TheCodeTraveler avatar Jun 26 '25 23:06 TheCodeTraveler