Skip to content

Fix incorrect version check order in MVVMToolkitRemoveDuplicateAnalyzers target - #1174

Merged
Sergio0694 merged 1 commit into
mainfrom
fix/analyzer-dedup-version-order
Mar 24, 2026
Merged

Fix incorrect version check order in MVVMToolkitRemoveDuplicateAnalyzers target#1174
Sergio0694 merged 1 commit into
mainfrom
fix/analyzer-dedup-version-order

Conversation

@Sergio0694

Copy link
Copy Markdown
Member

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.

…ers target

MSBuild PropertyGroup elements are evaluated top-to-bottom, with later
assignments overwriting earlier ones. The >= 4.12 check was before the
>= 4.3 check, so for any Roslyn version >= 4.12 (including 5.0), the
4.12 result would be immediately overwritten by the 4.3 match. This
caused the wrong analyzer (roslyn4.3) to be selected instead of the
correct one (roslyn4.12).

Swap the order so the lower version (4.3) is checked first and the
higher version (4.12) is checked last, ensuring the highest compatible
analyzer wins.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Sergio0694 Sergio0694 added bug 🐛 An unexpected issue that highlights incorrect behavior build 🔥 Some changes or issues related to build infrastructure mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit bugfix 🔧 PRs fixing a discovered bug labels Mar 24, 2026
@Sergio0694
Sergio0694 merged commit f207070 into main Mar 24, 2026
5 checks passed
@Sergio0694
Sergio0694 deleted the fix/analyzer-dedup-version-order branch March 24, 2026 22:03
This was referenced Mar 25, 2026
evgenygunko pushed a commit to evgenygunko/CopyWordsDA that referenced this pull request Mar 26, 2026
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [CommunityToolkit.Mvvm](https://github.com/CommunityToolkit/dotnet) | nuget | patch | `8.4.1` -> `8.4.2` |

---

### Release Notes

<details>
<summary>CommunityToolkit/dotnet (CommunityToolkit.Mvvm)</summary>

### [`v8.4.2`](https://github.com/CommunityToolkit/dotnet/releases/tag/v8.4.2): 8.4.2

#### What's Changed

-   Fix incorrect version check order in MVVMToolkitRemoveDuplicateAnalyzers target by [@&#8203;Sergio0694](https://github.com/Sergio0694) in CommunityToolkit/dotnet#1174

**Full Changelog**: CommunityToolkit/dotnet@v8.4.1...v8.4.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or PR is renamed to start with "rebase!".

👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
RootMeanSqr pushed a commit to RootMeanSqr/xsd-editor that referenced this pull request Sep 4, 2026
Updated
[CommunityToolkit.Mvvm](https://github.com/CommunityToolkit/dotnet) from
8.4.0 to 8.4.2.

<details>
<summary>Release notes</summary>

_Sourced from [CommunityToolkit.Mvvm's
releases](https://github.com/CommunityToolkit/dotnet/releases)._

## 8.4.2

## What's Changed
* Fix incorrect version check order in
MVVMToolkitRemoveDuplicateAnalyzers target by @​Sergio0694 in
CommunityToolkit/dotnet#1174


**Full Changelog**:
CommunityToolkit/dotnet@v8.4.1...v8.4.2

## 8.4.1

This release of the .NET Community Toolkit updates the MVVM Toolkit
analyzers and source generator to Roslyn 5.0, so they can work with C#
14 out of the box. This means that `[ObservableProperty]` on partial
properties no longer needs `preview` language version 🎉

## What's Changed 🆕
* Migrate workflow to GitHub Actions by @​Sergio0694 in
CommunityToolkit/dotnet#1105
* Remove extra dash in `--no-build` argument in dotnet test invokation
by @​echoix in CommunityToolkit/dotnet#1132
* Bump unit tests from .NET 7 to .NET 9 by @​Sergio0694 in
CommunityToolkit/dotnet#1085
* Migrate solution to '.slnx' format by @​Sergio0694 in
CommunityToolkit/dotnet#1133
* Remove .NET 7 TFM by @​Sergio0694 in
CommunityToolkit/dotnet#1134
* fix object.Equals for EquatableArray by @​JamesYFC in
CommunityToolkit/dotnet#1120
* Handle 'CanExecute' with method overrides by @​Sergio0694 in
CommunityToolkit/dotnet#1081
* Migrate to new MSTest APIs to prepare for 4.0.x by @​Sergio0694 in
CommunityToolkit/dotnet#1136
* Update MSTest to latest by @​Sergio0694 in
CommunityToolkit/dotnet#1137
* Update NuGet packages and Windows SDK props by @​Sergio0694 in
CommunityToolkit/dotnet#1047
* Update to .NET 10 SDK, add .NET 10 TFM to test projects by
@​Sergio0694 in CommunityToolkit/dotnet#1144
* Fix C# language version checks for `[ObservableProperty]` by
@​Sergio0694 in CommunityToolkit/dotnet#1140
* Eliminate this = default shortcut when height * width = 0 by
@​marcin-krystianc in
CommunityToolkit/dotnet#1130
* Support nested blocks in semi-auto property analyzer, add tests by
@​Youssef1313 in CommunityToolkit/dotnet#1025
* Update .NET SDK and NuGet dependencies by @​Sergio0694 in
CommunityToolkit/dotnet#1155
* Update Roslyn 5.0 analyzer and code fixer paths by @​Sergio0694 in
CommunityToolkit/dotnet#1156
* Update analyzers for C# 14.0 language version checks by @​Sergio0694
in CommunityToolkit/dotnet#1159
* Fix deterministic builds, improve GitHub links by @​Sergio0694 in
CommunityToolkit/dotnet#1170
* Add MVVM Toolkit code fixer for Roslyn 5.0 by @​Sergio0694 in
CommunityToolkit/dotnet#1171

## New Contributors
* @​echoix made their first contribution in
CommunityToolkit/dotnet#1132
* @​JamesYFC made their first contribution in
CommunityToolkit/dotnet#1120
* @​marcin-krystianc made their first contribution in
CommunityToolkit/dotnet#1130

**Full Changelog**:
CommunityToolkit/dotnet@v8.4.0...v8.4.1

Commits viewable in [compare
view](CommunityToolkit/dotnet@v8.4.0...v8.4.2).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=CommunityToolkit.Mvvm&package-manager=nuget&previous-version=8.4.0&new-version=8.4.2)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🐛 An unexpected issue that highlights incorrect behavior bugfix 🔧 PRs fixing a discovered bug build 🔥 Some changes or issues related to build infrastructure mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant