[msbuild-quality] MSBuild shipped targets: missing FileWrites, stale fallback, DependsOn overwrite

[github-actions]

🔧 MSBuild File Quality Report — 2026-08-26

Files reviewed: 20 (all category-1 shipped SDK build logic + VS shims)
Findings: 🔴 1 error · 🟡 2 warnings · 🔵 1 suggestion

---

🔴 Errors

src/FSharp.Build/Microsoft.FSharp.Targets — Rule A-1: CreateManifestResourceNamesDependsOn overwrite

• Line: 123
• Current: <CreateManifestResourceNamesDependsOn></CreateManifestResourceNamesDependsOn>
• Issue: Sets the property to empty, dropping any value a prior import may have appended. If any SDK target or NuGet package adds to CreateManifestResourceNamesDependsOn, that dependency is silently lost.
• Suggested: <CreateManifestResourceNamesDependsOn>$(CreateManifestResourceNamesDependsOn)</CreateManifestResourceNamesDependsOn> — or add a comment explaining why the overwrite is intentional (F# replaces the SDK’s CreateManifestResourceNames entirely).

Note: The CoreCompileDependsOn assignment at line 224 (<CoreCompileDependsOn>_ComputeNonExistentFileProperty</CoreCompileDependsOn>) looks like the same pattern, but is safe because F# defines its own CoreCompile target (line 280) that replaces the C# SDK’s, and the property is read at execution time after Microsoft.Common.targets (imported at line 430) has a chance to contribute. Still, a brief comment would help future readers.

🟡 Warnings

src/FSharp.Build/Microsoft.FSharp.NetSdk.targets — Rule A-4: Missing FileWrites for ILLink substitutions

• Line: 213–219 (GenerateFSharpILLinkSubstitutions target)
• Current: The GenerateILLinkSubstitutions task outputs items to @(EmbeddedResource) but does not register the generated file in @(FileWrites).
• Impact: dotnet clean won’t remove the generated ILLink.Substitutions.xml from obj/.
• Suggested: Add <Output TaskParameter="GeneratedItems" ItemName="FileWrites" /> (or a subsequent <ItemGroup><FileWrites Include="@(_generatedILLinkItems)" /></ItemGroup>).

vsintegration/shims/Microsoft.FSharp.ShimHelpers.props — Stale TODO / hardcoded fallback

• Line: 35–38
• Current:

  <!-- TBD: Remove before shipping. Temporary workaround ... -->
  <PropertyGroup Condition="!Exists(...)">
    <_FSCorePackageVersionSet>true</_FSCorePackageVersionSet>
    <FSCorePackageVersion>6.0.4</FSCorePackageVersion>
  </PropertyGroup>

• Issue: The comment says “Remove before shipping” but the block is still present and pins a hardcoded 6.0.4 version as a fallback. This was added as a temporary workaround; if it’s now permanent, the comment should be updated. If it’s truly obsolete, it should be removed.

🔵 Suggestions

src/FSharp.Build/Microsoft.FSharp.Targets — Add explanatory comments on intentional DependsOn overwrites

• Lines: 123, 224
• Suggestion: Both CreateManifestResourceNamesDependsOn and CoreCompileDependsOn intentionally override the SDK’s chain because F# replaces those targets entirely. A one-line comment (e.g., <!-- Intentional overwrite: F# provides its own CreateManifestResourceNames -->) would prevent future reviewers from filing this as a bug.

Files reviewed (no issues found)

• src/FSharp.Build/Microsoft.FSharp.NetSdk.props — Good condition guards, proper semicolon-list composition for WarningsAsErrors, NoWarn, DefineConstants
• src/FSharp.Build/Microsoft.FSharp.Core.NetSdk.props — Clean sentinel-guarded import pattern
• src/FSharp.Build/Microsoft.FSharp.Overrides.NetSdk.targets — Good FileWrites registration for generated assembly info
• src/FSharp.Build/Microsoft.Portable.FSharp.Targets — Good Exists() guards on all imports
• src/fsc/fsc.targets — Proper $(NoWarn) composition
• src/fsi/fsi.targets — Proper $(NoWarn) and $(DefineConstants) composition
• vsintegration/shims/Microsoft.FSharp.NetSdk.Shim.props — Clean
• 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 (project references, no build logic)
• src/FSharp.Build/Directory.Build.props, src/fsc/Directory.Build.props, src/fsi/Directory.Build.props, vsintegration/Vsix/Directory.Build.props, vsintegration/Vsix/Directory.Build.targets — Infrastructure, 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 3.8M · ◷