[msbuild-quality] MSBuild file quality findings: DotnetFsiCompilerPath bug + unquoted conditions

[github-actions]

🔧 MSBuild File Quality Report — 2026-06-24

Files reviewed: 65
Findings: 🔴 2 errors · 🟡 4 warnings · 🔵 1 suggestion

🔴 Errors

src/FSharp.Build/Microsoft.FSharp.NetSdk.props

• Rule B-1: Copy-paste bug — DotnetFsiCompilerPath condition checks wrong variable
  • Line: 79
  • Current: <DotnetFsiCompilerPath Condition="'$(DotnetFscCompilerPath)' == ''">
  • Suggested: <DotnetFsiCompilerPath Condition="'$(DotnetFsiCompilerPath)' == ''">
  • Impact: Since DotnetFscCompilerPath is set on line 75, its condition is always false by line 79, so DotnetFsiCompilerPath is never configured in the shipped SDK flow. Any scenario that routes through the Fsi MSBuild task (rather than dotnet fsi) would not find the FSI compiler path.

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

• Rule B-2: Unquoted property references in shipped target condition
  • Line: 174
  • Current: Condition="$(DisableILLinkSubstitutions) != 'true' and $(AssemblyName) != 'FSharp.Core'"
  • Suggested: Condition="'$(DisableILLinkSubstitutions)' != 'true' and '$(AssemblyName)' != 'FSharp.Core'"
  • Impact: If either property contains spaces or special characters, condition evaluation could fail. This is a shipped SDK target imported by every F# project.

🟡 Warnings

src/FSharp.Build/Microsoft.FSharp.Targets

• Rule B-2: Unquoted boolean literal in conditions
  • Lines: 333, 335
  • Current: '$(UsingXBuild)' == true / '$(UsingXBuild)' != true
  • Suggested: '$(UsingXBuild)' == 'true' / '$(UsingXBuild)' != 'true'

vsintegration/shims/Microsoft.FSharp.ShimHelpers.props

• Stale workaround: Comment says "TBD: Remove before shipping" but the file ships with a hardcoded FSCorePackageVersion = 6.0.4 fallback
  • Lines: 35–39
  • Impact: If the fallback fires, projects get an outdated FSharp.Core version. The workaround may no longer be needed.

FSharp.Profiles.props

• Rule B-2: Unquoted property references in conditions
  • Lines: 9, 14
  • $(TolerateUnusedBindings) != 'true' → '$(TolerateUnusedBindings)' != 'true'

FSharpBuild.Directory.Build.props

• Rule B-2: Unquoted property reference in condition
  • Line: 33
  • $(AdditionalFscCmdFlags) != '' → '$(AdditionalFscCmdFlags)' != ''

🔵 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 file via GenerateILLinkSubstitutions task but doesn't register the output with @(FileWrites), so dotnet clean won't remove it.

Files reviewed (no issues found)

Shipped SDK build logic (highest priority):

• src/FSharp.Build/Microsoft.FSharp.Core.NetSdk.props — clean, good use of sentinel properties
• src/FSharp.Build/Microsoft.FSharp.Overrides.NetSdk.targets — clean, properly registers FileWrites
• src/FSharp.Build/Microsoft.Portable.FSharp.Targets — clean, uses Exists() guards on all imports
• src/fsc/fsc.targets — clean, properly preserves $(NoWarn) and $(DefineConstants)
• src/fsi/fsi.targets — clean, properly preserves $(NoWarn) and $(DefineConstants)
• All 6 vsintegration/shims/ files — clean, proper sentinel patterns and Exists() guards
• vsintegration/Vsix/VisualFSharpFull/VisualFSharp.Core.targets — clean

Repository infrastructure:

• Directory.Build.props / Directory.Build.targets — clean
• FSharpBuild.Directory.Build.targets — clean, targets properly use Inputs/Outputs and FileWrites
• FSharpTests.Directory.Build.props / .targets — clean
• CoordinateXliff.targets — clean
• buildtools/buildtools.targets — clean, properly registers FileWrites for generated lex/yacc files
• All eng/ files — clean
• All tests/ Directory.Build files — clean
• All vsintegration/ Directory.Build files — clean
• All setup/ files — clean
• All 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 15.3M · ◷