[msbuild-quality] MSBuild file quality findings — 2026-07-01 (update: 5 of 7 prior findings fixed)

[github-actions]

🔧 MSBuild File Quality Report — 2026-07-01

Files reviewed: 90
Findings: 🔴 0 errors · 🟡 2 warnings · 🔵 1 suggestion

Progress since #19989: 5 of 7 previously reported findings have been fixed ✅ — the DotnetFsiCompilerPath copy-paste bug (🔴), unquoted conditions in NetSdk.targets (🔴), Microsoft.FSharp.Targets (🟡), FSharp.Profiles.props (🟡), and FSharpBuild.Directory.Build.props (🟡) are all resolved. Two lower-severity items remain.

---

🟡 Warnings

vsintegration/shims/Microsoft.FSharp.ShimHelpers.props

• Stale workaround with hardcoded FSCorePackageVersion
  • Lines: 35–39
  • Current:

    <!-- TBD: Remove before shipping. Temporary workaround ... -->
    <PropertyGroup Condition="!Exists('$(Fsc_DotNet_CompilerPath)Microsoft.FSharp.Core.NetSdk.props') ...">
      <_FSCorePackageVersionSet>true</_FSCorePackageVersionSet>
      <FSCorePackageVersion>6.0.4</FSCorePackageVersion>
    </PropertyGroup>

  • Impact: If the fallback fires, consumer projects resolve an outdated FSharp.Core 6.0.4 instead of the SDK-bundled version. The comment says "TBD: Remove before shipping" — this workaround may no longer be needed now that the SDK ships the matching .props file.

vsintegration/tests/MockTypeProviders/Directory.Build.props

• Rule B-3: NoWarn overwrite drops prior suppressions
  • Line: 13
  • Current: <NoWarn>0067;0169;1591</NoWarn>
  • Suggested: <NoWarn>$(NoWarn);0067;0169;1591</NoWarn>
  • Impact: Any NoWarn values set by parent Directory.Build.props files (e.g., the repo root's FS2003 for F# projects) are silently dropped. Low impact since these are C# test mock projects, but still an anti-pattern.

🔵 Suggestions

src/FSharp.Build/Microsoft.FSharp.NetSdk.targets

• Rule A-4: Missing FileWrites registration for GenerateFSharpILLinkSubstitutions
  • Lines: 174–180
  • The target generates an ILLink substitutions XML file via the GenerateILLinkSubstitutions task and adds it to @(EmbeddedResource), but does not register the output with @(FileWrites). This means dotnet clean will not remove the generated file from obj/.
  • Suggested: Add <Output TaskParameter="GeneratedItems" ItemName="FileWrites" /> alongside the existing EmbeddedResource output.

Files reviewed (no issues found)

Shipped SDK build logic (highest priority — ships in .NET SDK):

• src/FSharp.Build/Microsoft.FSharp.NetSdk.props — clean ✅ (DotnetFsiCompilerPath fix confirmed)
• src/FSharp.Build/Microsoft.FSharp.NetSdk.targets — clean ✅ (unquoted conditions fix confirmed; FileWrites suggestion noted above)
• src/FSharp.Build/Microsoft.FSharp.Overrides.NetSdk.targets — clean, properly registers FileWrites
• src/FSharp.Build/Microsoft.FSharp.Targets — clean ✅ (UsingXBuild quoting fix confirmed), proper Inputs/Outputs on CoreCompile
• src/FSharp.Build/Microsoft.FSharp.Core.NetSdk.props — clean, good sentinel pattern with FSCorePackageVersionSet
• src/FSharp.Build/Microsoft.Portable.FSharp.Targets — clean, all imports Exists()-guarded
• src/fsc/fsc.targets — clean, properly preserves $(NoWarn) and $(DefineConstants)
• src/fsi/fsi.targets — clean, properly preserves $(NoWarn) and $(DefineConstants)

VS integration shims:

• vsintegration/shims/Microsoft.FSharp.NetSdk.Shim.props — clean, sentinel guard pattern
• vsintegration/shims/Microsoft.FSharp.NetSdk.Shim.targets — clean
• vsintegration/shims/Microsoft.FSharp.Overrides.NetSdk.Shim.targets — clean
• vsintegration/shims/Microsoft.FSharp.Shim.targets — clean
• vsintegration/shims/Microsoft.Portable.FSharp.Shim.targets — clean
• vsintegration/Vsix/VisualFSharpFull/VisualFSharp.Core.targets — clean

Repository infrastructure:

• Directory.Build.props / Directory.Build.targets — clean
• FSharpBuild.Directory.Build.props — clean ✅ (quoting fix confirmed)
• FSharpBuild.Directory.Build.targets — clean, targets use Inputs/Outputs and FileWrites
• FSharp.Profiles.props — clean ✅ (quoting fix confirmed)
• CoordinateXliff.targets — clean
• buildtools/buildtools.targets — clean, registers FileWrites for lex/yacc outputs
• All eng/ files — clean
• All tests/ Directory.Build files — clean
• All vsintegration/ Directory.Build files — clean (except MockTypeProviders noted above)
• All setup/ files — clean
• SDK test .props/.targets — clean

---

Review Rules Reference

This review checks against MSBuild canonical patterns for:

• Target authoring: DependsOn chains, Returns vs Outputs, incremental build, FileWrites
• Property patterns: Conditional defaults, quoted conditions, semicolon composition, path normalization
• Item management: Include/Remove/Update, batching, generated file placement
• Extension points: Import guards, CustomBefore/After hooks, cross-platform paths

Generated by MSBuild Quality Review

Generated by F# MSBuild File Quality Review Agent · opus46 13.3M · ◷