[msbuild-quality] MSBuild File Quality Report — 2026-06-25

[Evangelink]

🔧 MSBuild File Quality Report — 2026-06-25

Files reviewed: 97 (70 NuGet build extensions · 7 MSTest.Sdk files · 20 repo-infrastructure files · 3 eng/common/* reviewed but excluded from findings per policy)
Findings: 🔴 0 errors · 🟡 2 warnings · 🔵 1 suggestion (affecting 14+ files)

---

🟡 Warnings

src/Adapter/MSTest.TestAdapter/buildTransitive/common/MSTest.TestAdapter.targets

• Rule A-2 — GetMSTestV2CultureHierarchy is named with the Get prefix convention but uses item-group side effects instead of the Returns attribute; the data flow is implicit rather than declared.
  • Lines: 49–82 (target declaration)
  • Current:

    <Target Name="GetMSTestV2CultureHierarchy">
      <ItemGroup>
        <CurrentUICultureHierarchy Include="$([System.Globalization.CultureInfo]::CurrentUICulture.Name)" />
        <!-- ... 4 more culture levels ... -->
      </ItemGroup>
      <ItemGroup>
        <MSTestV2Files Include="$(MSBuildThisFileDirectory)..\_localization\%(CurrentUICultureHierarchy.Identity)\*.dll">
          <UICulture>%(CurrentUICultureHierarchy.Identity)</UICulture>
        </MSTestV2Files>
      </ItemGroup>
    </Target>

  • Suggested: Add Returns="@(MSTestV2Files)" (and expose @(CurrentUICultureHierarchy) too if callers need it), so the output contract is explicit and callers can consume results via CallTarget Output or normal DependsOnTargets:

    <Target Name="GetMSTestV2CultureHierarchy" Returns="@(MSTestV2Files)">

  • Note: Same issue exists in src/Adapter/MSTest.TestAdapter/buildTransitive/uwp/MSTest.TestAdapter.targets (line 16).

---

Directory.Packages.props

• Rule A-5 — _ValidateBundledSdkFeatureVersions is a <Target> defined inside a .props file. Targets should live in .targets files, not .props files; .props is evaluated early during property/item collection and targets defined there can be confusing to maintain.
  • Lines: last ~18 lines of the file
  • Current (in Directory.Packages.props):

    <Target Name="_ValidateBundledSdkFeatureVersions"
            BeforeTargets="CollectPackageReferences;GenerateNuspec;Build;Pack"
            Condition=" '$(MSBuildProjectName)' == 'MSTest.Sdk' ">
      <PropertyGroup>
        <_AspireHostingTestingPackageVersion>
          @(PackageVersion->WithMetadataValue('Identity','Aspire.Hosting.Testing')->'%(Version)')
        </_AspireHostingTestingPackageVersion>
        <_MicrosoftPlaywrightPackageVersion>
          @(PackageVersion->WithMetadataValue('Identity','Microsoft.Playwright.MSTest.v4')->'%(Version)')
        </_MicrosoftPlaywrightPackageVersion>
      </PropertyGroup>
      <Error Condition="..." Text="..." />
      <Error Condition="..." Text="..." />
    </Target>

  • Suggested: Move the target to Directory.Build.targets (which is already present at the repo root) or to a dedicated eng/Validation.targets that is imported from Directory.Build.targets. The PackageVersion items it references are populated before target execution regardless of which file the target is authored in.

---

🔵 Suggestions

All MTP extension buildTransitive/*.props files (12 packages)

• Rule E-1 — buildTransitive/*.props for all MTP extension packages forward directly to buildMultiTargeting/ instead of forwarding through the corresponding build/ file. The standard NuGet convention is buildTransitive/ → build/ → (shared location), which establishes a clear ownership chain and makes it obvious that transitive consumers receive a subset of what direct consumers see.
  • Affected files (one example pattern shown; identical in all 12):
    • src/Platform/Microsoft.Testing.Extensions.*/buildTransitive/Microsoft.Testing.Extensions.*.props
    • Same pattern in src/Platform/Microsoft.Testing.Platform/buildTransitive/Microsoft.Testing.Platform.props
  • Current (e.g. buildTransitive/Microsoft.Testing.Extensions.TrxReport.props):

    <Project>
      <Import Project="$(MSBuildThisFileDirectory)..\..\buildMultiTargeting\Microsoft.Testing.Extensions.TrxReport.props" />
    </Project>

  • Suggested — forward to build/ which in turn already forwards to buildMultiTargeting/:

    <Project>
      <Import Project="$(MSBuildThisFileDirectory)..\..\build\Microsoft.Testing.Extensions.TrxReport.props" />
    </Project>

  • Packages affected: AzureDevOpsReport, AzureFoundry, CrashDump, CtrfReport, HangDump, HotReload, HtmlReport, JUnitReport, Retry, Telemetry, TrxReport, Microsoft.Testing.Platform (base).

---

Files reviewed without findings (83)

NuGet build extensions — clean

• src/Adapter/MSTest.TestAdapter/build/net{462,8.0,9.0}/MSTest.TestAdapter.{props,targets} (6 files)
• src/Adapter/MSTest.TestAdapter/build/uap10.0/MSTest.TestAdapter.{props,targets} (2 files)
• src/Adapter/MSTest.TestAdapter/buildTransitive/Parallelize.targets
• src/Adapter/MSTest.TestAdapter/buildTransitive/uwp/MSTest.TestAdapter.props
• src/Analyzers/MSTest.Analyzers.Package/buildTransitive/MSTest.Analyzers.{props,targets} (2 files)
• src/Platform/Microsoft.Testing.Extensions.{AzureDevOpsReport,AzureFoundry,CrashDump,CtrfReport,HangDump,HotReload,HtmlReport,JUnitReport,Retry,Telemetry,TrxReport}/build/*.props (11 files)
• src/Platform/Microsoft.Testing.Extensions.{AzureDevOpsReport,AzureFoundry,CrashDump,CtrfReport,HangDump,HotReload,HtmlReport,JUnitReport,Retry,Telemetry,TrxReport}/buildMultiTargeting/*.props (11 files)
• src/Platform/Microsoft.Testing.Platform.Internal.DotnetTest/build/Microsoft.Testing.Platform.Internal.DotnetTest.props
• src/Platform/Microsoft.Testing.Platform.MSBuild/build/Microsoft.Testing.Platform.MSBuild.{props,targets} (2 files)
• src/Platform/Microsoft.Testing.Platform.MSBuild/buildMultiTargeting/Microsoft.Testing.Platform.MSBuild.{CustomTestTarget,VSTest,props,targets}.targets (4 files)
• src/Platform/Microsoft.Testing.Platform.MSBuild/buildTransitive/Microsoft.Testing.Platform.MSBuild.{props,targets} (2 files)
• src/Platform/Microsoft.Testing.Platform/build/Microsoft.Testing.Platform.{props,targets} (2 files)
• src/Platform/Microsoft.Testing.Platform/buildMultiTargeting/Microsoft.Testing.Platform.{props,targets} (2 files)
• src/Platform/Microsoft.Testing.Platform/buildTransitive/Microsoft.Testing.Platform.{props,targets} (2 files)
• src/TestFramework/TestFramework.Extensions/build/net{462,8.0,9.0,standard2.0}/MSTest.TestFramework.targets (4 files)
• src/TestFramework/TestFramework.Extensions/build/uap10.0/MSTest.TestFramework.targets
• src/TestFramework/TestFramework.Extensions/buildTransitive/{net8.0AndLater,others}/MSTest.TestFramework.targets (2 files)

MSTest.Sdk — clean

• src/Package/MSTest.Sdk/Sdk/Features/Aspire.targets
• src/Package/MSTest.Sdk/Sdk/Features/Playwright.targets
• src/Package/MSTest.Sdk/Sdk/Runner/ClassicEngine.targets
• src/Package/MSTest.Sdk/Sdk/Runner/Common.targets
• src/Package/MSTest.Sdk/Sdk/Runner/NativeAOT.targets
• src/Package/MSTest.Sdk/Sdk/Sdk.targets
• src/Package/MSTest.Sdk/Sdk/VSTest/VSTest.targets

Repository infrastructure — clean

• Directory.Build.props
• Directory.Build.targets
• eng/AfterSolutionBuild.targets
• eng/Analyzers.props
• eng/Build.props
• eng/Publishing.props
• eng/Tools.props
• eng/Versions.props
• eng/common/internal/Directory.Build.props (eng/common — reviewed, not in scope for findings)
• eng/common/native/LocateNativeCompiler.targets (eng/common — reviewed, not in scope for findings)
• eng/common/native/NativeAotSupported.props (eng/common — reviewed, not in scope for findings)
• samples/public/Directory.Build.{props,targets} (2 files)
• samples/public/Directory.Packages.props
• src/Directory.Build.props
• src/Platform/Directory.Build.props
• test/Directory.Build.{props,targets} (2 files)
• test/IntegrationTests/TestAssets/Directory.Build.targets

---

Reference

This review applies the rule catalog defined in
.github/agents/msbuild-reviewer.agent.md.

Generated by MSBuild Quality Review workflow run 28146295060.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the MSBuild Quality Review workflow. · 1.1K AIC · ⌖ 13.1 AIC · ⊞ 41.7K · [◷]( · ◷)

• expires on Jul 2, 2026, 4:25 AM UTC

[Evangelink]

The remaining warnings are addressed in #9487:

• A-2: declared Returns="@(MSTestV2Files)" on GetMSTestV2CultureHierarchy in both the common and uwp MSTest.TestAdapter.targets.
• A-5: moved _ValidateBundledSdkFeatureVersions from Directory.Packages.props to Directory.Build.targets.

The E-1 suggestion was already done in #9431.