[msbuild-quality] MSBuild defaults and incremental resource tracking regressions

Start with the four named files: src/FSharp.Build/Microsoft.FSharp.NetSdk.props, eng/Packages.props, FSharpBuild.Directory.Build.targets, and vsintegration/tests/MockTypeProviders/Directory.Build.props. Review the reported lines and surrounding MSBuild evaluation order, item names, and property composition. Done means the four reported regressions are corrected without overwriting consumer settings or inherited suppressions, and the computed versions and resource files are actually consumed.

🔧 MSBuild File Quality Report — 2026-09-30

Files reviewed: 96
New findings: 🔴 2 errors · 🟡 2 warnings · 🔵 0 suggestions

🔴 Errors

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

• Rule B-1: ValueTupleImplicitPackageVersion is consumed before its default is assigned, and the later unconditional assignment overwrites consumer/repository choices.
  • Lines: 87–97
  • Current:

    <ItemGroup Condition="...">
      <PackageReference Include="System.ValueTuple" Version="$(ValueTupleImplicitPackageVersion)" />
    </ItemGroup>
    
    <PropertyGroup>
      <ValueTupleImplicitPackageVersion>4.6.2</ValueTupleImplicitPackageVersion>
    </PropertyGroup>
    

  • Impact: For consumer projects targeting .NET Standard/.NET Core below 2.0, the package item is evaluated before the default exists and can receive an empty version. A value supplied by the project is also replaced before the later .NET Framework package item is evaluated.
  • Suggested:

    <PropertyGroup>
      <ValueTupleImplicitPackageVersion Condition="'$(ValueTupleImplicitPackageVersion)' == ''">4.6.2</ValueTupleImplicitPackageVersion>
    </PropertyGroup>
    
    <ItemGroup Condition="...">
      <PackageReference Include="System.ValueTuple" Version="$(ValueTupleImplicitPackageVersion)" />
    </ItemGroup>
    

eng/Packages.props

• Rule B-1: The three computed central package-version floors are never consumed.
  • Lines: 12–17, 111, 116, 118
  • Current:

    <SystemCollectionsImmutableCentralVersion>...</SystemCollectionsImmutableCentralVersion>
    <SystemReflectionMetadataCentralVersion>...</SystemReflectionMetadataCentralVersion>
    <SystemSecurityCryptographyXmlCentralVersion>...</SystemSecurityCryptographyXmlCentralVersion>
    
    <PackageVersion Include="System.Collections.Immutable" Version="$(SystemCollectionsImmutableVersion)" />
    <PackageVersion Include="System.Reflection.Metadata" Version="$(SystemReflectionMetadataVersion)" />
    <PackageVersion Include="System.Security.Cryptography.Xml" Version="$(SystemSecurityCryptographyXmlVersion)" />
    

  • Impact: The documented 10.0.9 floor is ineffective, retaining the CPM downgrade/prebuilt-package risk that the surrounding comment says these aliases are intended to prevent.
  • Suggested:

    <PackageVersion Include="System.Collections.Immutable" Version="$(SystemCollectionsImmutableCentralVersion)" />
    <PackageVersion Include="System.Reflection.Metadata" Version="$(SystemReflectionMetadataCentralVersion)" />
    <PackageVersion Include="System.Security.Cryptography.Xml" Version="$(SystemSecurityCryptographyXmlCentralVersion)" />
    

🟡 Warnings

FSharpBuild.Directory.Build.targets

• Rule A-3: The incremental resource comparison hashes an undefined item instead of the files collected immediately above it.
  • Lines: 131–155
  • Current:

    <IntermediateResourcesFiles Include="$(IntermediateOutputPath)resources\*.resx" />
    ...
    <GetFileHash Files="@(IntermediateResourceFilesForHash)">
    

  • Impact: @(IntermediateResourceFilesForHash) is never populated, so IntermediateResourceFilesHash is based on an empty item list and the target cannot correctly determine whether transformed resources changed.
  • Suggested:

    <GetFileHash Files="@(IntermediateResourcesFiles)">
    

vsintegration/tests/MockTypeProviders/Directory.Build.props

• Rule B-3: The test-specific NoWarn assignment discards inherited suppressions.
  • Line: 13
  • Current: <NoWarn>0067;0169;1591</NoWarn>
  • Suggested: <NoWarn>$(NoWarn);0067;0169;1591</NoWarn>

🔵 Suggestions

None.

Existing issue #20357 already tracks the CreateManifestResourceNamesDependsOn overwrite, missing FileWrites registration for ILLink substitutions, and stale shim fallback. Those unchanged findings are intentionally not duplicated here.

Files reviewed (no issues found)

• .github/skills/fsharp-diagnostics/server/Directory.Build.props
• CoordinateXliff.targets
• Directory.Build.props
• Directory.Build.targets
• Directory.Packages.props
• FSharp.Profiles.props
• FSharpBuild.Directory.Build.props
• FSharpTests.Directory.Build.props
• FSharpTests.Directory.Build.targets
• UseLocalCompiler.Directory.Build.props
• UseLocalCompiler.Directory.Build.targets
• buildtools/buildtools.targets
• buildtools/checkpackages/Directory.Build.props
• buildtools/checkpackages/Directory.Build.targets
• docs/fcs-samples/Directory.Build.props
• eng/Publishing.props
• eng/Signing.props
• eng/TargetFrameworks.props
• eng/Version.Details.props
• eng/Versions.props
• eng/common/internal/Directory.Build.props
• eng/common/native/LocateNativeCompiler.targets
• eng/common/native/NativeAotSupported.props
• eng/restore/optimizationData.targets
• eng/targets/Imports.targets
• eng/targets/NuGet.targets
• eng/targets/Settings.props
• setup/Directory.Build.props
• setup/Swix/Directory.Build.props
• setup/Swix/Directory.Build.targets
• src/Compiler/Directory.Build.props
• src/Directory.Build.props
• src/Directory.Build.targets
• src/FSharp.Build/Directory.Build.props
• src/FSharp.Build/Microsoft.FSharp.Core.NetSdk.props
• src/FSharp.Build/Microsoft.FSharp.Overrides.NetSdk.targets
• src/FSharp.Build/Microsoft.Portable.FSharp.Targets
• src/FSharp.Compiler.Interactive.Settings/Directory.Build.props
• src/FSharp.Core/Directory.Build.props
• src/Microsoft.FSharp.Compiler/Directory.Build.props
• src/fsc/Directory.Build.props
• src/fsc/fsc.targets
• src/fsi/Directory.Build.props
• src/fsi/fsi.targets
• tests/AheadOfTime/Directory.Build.props
• tests/AheadOfTime/Directory.Build.targets
• tests/Directory.Build.props
• tests/Directory.Build.targets
• tests/EndToEndBuildTests/DesignTimeProviderPackaging/Directory.Build.props
• tests/EndToEndBuildTests/Directory.Build.props
• tests/EndToEndBuildTests/Directory.Build.targets
• tests/FSharp.Build.UnitTests/Directory.Build.props
• tests/FSharp.Compiler.ComponentTests/Directory.Build.props
• tests/FSharp.Compiler.Private.Scripting.UnitTests/Directory.Build.props
• tests/FSharp.Compiler.Service.Tests/Directory.Build.props
• tests/FSharp.Test.Utilities/Directory.Build.props
• tests/Resources/Directory.Build.props
• tests/Resources/Directory.Build.targets
• tests/benchmarks/Directory.Build.props
• tests/fsharp/Directory.Build.props
• tests/fsharp/SDKTests/Directory.Build.props
• tests/fsharp/SDKTests/Directory.Build.targets
• tests/fsharp/SDKTests/tests/FSharpCoreVersionTest.props
• tests/fsharp/SDKTests/tests/FSharpCoreVersionTest.targets
• tests/fsharp/SDKTests/tests/PackageTest.props
• tests/fsharp/SDKTests/tests/PackageTest.targets
• tests/fsharp/SDKTests/tests/ToolsTest.props
• tests/fsharp/SDKTests/tests/ToolsTest.targets
• tests/projects/CompilerCompat/Directory.Build.props
• vsintegration/Directory.Build.props
• vsintegration/Directory.Build.targets
• vsintegration/ItemTemplates/Directory.Build.props
• vsintegration/ItemTemplates/Directory.Build.targets
• vsintegration/ProjectTemplates/Directory.Build.props
• vsintegration/ProjectTemplates/Directory.Build.targets
• vsintegration/Templates.Directory.Build.props
• vsintegration/Templates.Directory.Build.targets
• vsintegration/Vsix/Directory.Build.props
• vsintegration/Vsix/Directory.Build.targets
• vsintegration/Vsix/VisualFSharpFull/VisualFSharp.Core.targets
• vsintegration/shims/Microsoft.FSharp.NetSdk.Shim.props
• vsintegration/shims/Microsoft.FSharp.NetSdk.Shim.targets
• vsintegration/shims/Microsoft.FSharp.Overrides.NetSdk.Shim.targets
• vsintegration/shims/Microsoft.FSharp.Shim.targets
• vsintegration/shims/Microsoft.Portable.FSharp.Shim.targets
• vsintegration/src/Directory.Build.props
• vsintegration/tests/Directory.Build.props
• vsintegration/tests/Directory.Build.targets
• vsintegration/tests/MockTypeProviders/Directory.Build.targets

---

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 · gpt56 1.8M · ◷