[msbuild-quality] MSBuild File Quality Report — F# SDK build logic

Start by reading src/FSharp.Build/Microsoft.FSharp.Targets around CoreCompileDependsOn and CreateManifestResourceNames, then inspect Microsoft.FSharp.NetSdk.targets around GenerateFSharpILLinkSubstitutions. Review the shim props and targets for the property-name and import-guard findings, and compare the established FileWrites pattern. Done means each finding is either safely corrected or documented with its intended behavior verified.

🔧 MSBuild File Quality Report

Scope: Category-1 shipped F# SDK build logic (src/FSharp.Build/, src/fsc/, src/fsi/) and the vsintegration/shims/ — the files imported by every F# project.

Files reviewed: 13
Findings: 🔴 0 errors · 🟡 2 warnings · 🔵 3 suggestions

---

🟡 Warnings

src/FSharp.Build/Microsoft.FSharp.Targets

• Rule A-1 (DependsOn chain overwrite): CoreCompileDependsOn is assigned without preserving its prior value.
  • Line: ~224
  • Current: <CoreCompileDependsOn>_ComputeNonExistentFileProperty</CoreCompileDependsOn>
  • Suggested: <CoreCompileDependsOn>$(CoreCompileDependsOn);_ComputeNonExistentFileProperty</CoreCompileDependsOn>
  • Note: F# defines its own CoreCompile (DependsOnTargets="$(CoreCompileDependsOn);FSharpSourceCodeCompileOrder"), so any contributions other SDK/props files appended to CoreCompileDependsOn before this line are silently dropped. If this blanking is deliberate (F# fully owns CoreCompile), a short comment explaining why would prevent future regressions. Verify against the SDK before changing — ordering-sensitive.

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

• Rule A-4 (missing FileWrites registration): The GenerateFSharpILLinkSubstitutions target generates ILLink.Substitutions.xml into $(IntermediateOutputPath) and registers it as EmbeddedResource, but not as @(FileWrites).
  • Line: ~213–219
  • Current: only <Output TaskParameter="GeneratedItems" ItemName="EmbeddedResource" />
  • Suggested: also emit the generated file to @(FileWrites) so dotnet clean removes it.
  • Note: The sibling target GenerateFSharpTextResources (in Microsoft.FSharp.Targets) correctly registers its generated .fs/.resx outputs in @(FileWrites); this target is inconsistent with that established pattern.

---

🔵 Suggestions

src/FSharp.Build/Microsoft.FSharp.Targets

• Rule A-1: <CreateManifestResourceNamesDependsOn></CreateManifestResourceNamesDependsOn> (~line 123) explicitly blanks the property. This mirrors the C#/VB targets and is likely intentional (F# overrides CreateManifestResourceNames wholesale), but an explanatory comment would make the intent clear.

vsintegration/shims/Microsoft.FSharp.ShimHelpers.props

• Property-name mismatch: the fallback branch (~line 37) sets <_FSCorePackageVersionSet>true</_FSCorePackageVersionSet> (leading underscore), whereas the guard property consumed by Microsoft.FSharp.Core.NetSdk.props is FSCorePackageVersionSet (no underscore). The underscore-prefixed property appears to be read by nothing, so the write looks dead. Confirm the intended name.

vsintegration/shims/*.targets / *.props

• Rule D-1 (Exists() guard on optional imports): The shim imports guarded only by '$(FSharpCompilerPath)' != '' (e.g. Microsoft.FSharp.NetSdk.Shim.props lines 4–5, Microsoft.FSharp.Shim.targets line 4) have no Exists('...') guard. If FSharpCompilerPath is set but the target file is absent, the build fails with a cryptic missing-import error rather than falling back. These are Windows/VS-only shims where the path is expected to exist, so impact is low — consider adding and Exists('...') for a clearer failure mode.

Files reviewed (no issues found)

• src/FSharp.Build/Microsoft.FSharp.NetSdk.props — correct semicolon composition (DefineConstants, WarningsAsErrors), guarded defaults, EnsureTrailingSlash usage
• src/FSharp.Build/Microsoft.FSharp.Core.NetSdk.props — clean guarded version property
• src/FSharp.Build/Microsoft.FSharp.Overrides.NetSdk.targets — CoreGenerateAssemblyInfo correctly declares Inputs/Outputs and registers FileWrites
• src/FSharp.Build/Microsoft.Portable.FSharp.Targets — correct Exists()-guarded, case-sensitivity-aware imports
• src/fsc/fsc.targets, src/fsi/fsi.targets — project files; correct $(NoWarn)/$(DefineConstants) composition
• vsintegration/shims/Microsoft.FSharp.Overrides.NetSdk.Shim.targets, Microsoft.Portable.FSharp.Shim.targets

---

Review Rules Reference

• Target authoring: DependsOn chains, Returns vs Outputs, incremental build, FileWrites
• Property patterns: conditional defaults, quoted conditions, semicolon composition, path normalization
• Item management: Include/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 · opus48 1M · ◷