Fix incorrect version check order in MVVMToolkitRemoveDuplicateAnalyzers target

Merged
SE
8mo ago
+1-1

Bug

The MVVMToolkitRemoveDuplicateAnalyzersWhenRoslynComponentVersioningIsNotSupported target in CommunityToolkit.Mvvm.SourceGenerators.targets selects the wrong Roslyn analyzer directory when the compiler version is >= 4.12 (e.g. Roslyn 5.0).

In the PropertyGroup that determines MVVMToolkitSelectedRoslynAnalyzerDirectoryName, MSBuild properties are evaluated top-to-bottom, with later assignments overwriting earlier ones. The >= 4.12 check was placed before the >= 4.3 check:

<!-- Before (wrong): -->
<MVVMToolkitSelectedRoslynAnalyzerDirectoryName Condition="$([MSBuild]::VersionGreaterThanOrEquals(..., 4.12))">roslyn4.12</...>
<MVVMToolkitSelectedRoslynAnalyzerDirectoryName Condition="$([MSBuild]::VersionGreaterThanOrEquals(..., 4.3))">roslyn4.3</...>

For any Roslyn version >= 4.12 (including 5.0), both conditions are true, so the correct roslyn4.12 result is immediately overwritten by roslyn4.3. This causes the roslyn4.3 analyzers to be passed to the compiler instead of the intended roslyn4.12 ones.

Fix

Swap the order so the lower version threshold (4.3) is checked first and the higher one (4.12) is checked last. This way, for Roslyn >= 4.12 the final (winning) assignment is roslyn4.12:

<!-- After (correct): -->
<MVVMToolkitSelectedRoslynAnalyzerDirectoryName Condition="$([MSBuild]::VersionGreaterThanOrEquals(..., 4.3))">roslyn4.3</...>
<MVVMToolkitSelectedRoslynAnalyzerDirectoryName Condition="$([MSBuild]::VersionGreaterThanOrEquals(..., 4.12))">roslyn4.12</...>

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • Tested code with current supported SDKs
  • Contains NO breaking changes

Other information

This was discovered by inspecting a binlog from a WinStore client app build using Roslyn 5.0. The RemoveItem Analyzer step was incorrectly removing the roslyn4.0 and roslyn4.12 analyzers, leaving only roslyn4.3 to be passed to the XAML compiler.