Resolve features from @(ReferencePath) after ResolveAssemblyReferences (#531)

Feature detection now runs in a single PolyfillResolveFeatures target
  hooked AfterTargets="ResolveAssemblyReferences" BeforeTargets="CoreCompile",
  and inspects @(ReferencePath) — the final list of assemblies csc will see,
  with NuGetPackageId / NuGetPackageVersion preserved on NuGet-resolved items.

  Replaces the previous two-target arrangement (PreparePolyfill running early
  off @(ResolvedCompileFileDefinitions), plus a DetectBclMemoryPackageRef
  fallback off @(PackageReference)). The fallback was too optimistic: it
  fired whenever a PackageReference for Microsoft.Bcl.Memory existed, even
  when the dll was not actually in csc's /reference list (stale IDE state,
  ExcludeAssets="compile", etc.). The result was that RefsBclMemory got set,
  Polyfill's own Index/Range types were suppressed, and the BCL types were
  also missing — producing CS0246 in Polyfill_IEnumerable_ElementAt and
  Polyfill_IEnumerable_Take.

  Inspecting @(ReferencePath) post-RAR makes the feature flags reflect what
  csc will actually see, eliminating that whole class of bug.

  Adds ConsumeBclMemoryExcludeCompile as a regression test that uses
  ExcludeAssets="compile" to reproduce the failure mode without needing an
  IDE in the loop. ConsumeBclMemoryNoResolve loses its now-impossible
  PreparePolyfill override and stays as a plain smoke test.

  If you want it shorter, the body can collapse to:

  Single PolyfillResolveFeatures target runs after ResolveAssemblyReferences
  and inspects @(ReferencePath), so feature flags only fire when the
  corresponding assembly is actually in the reference list csc will see.

  Replaces the early PreparePolyfill + DetectBclMemoryPackageRef pair, whose
  @(PackageReference)-based fallback could set RefsBclMemory while the dll
  was missing from /reference (stale IDE state, ExcludeAssets="compile"),
  causing CS0246 in Polyfill_IEnumerable_ElementAt / Take.

  Adds ConsumeBclMemoryExcludeCompile as a regression test.

Author: SimonCropp

src/ConsumeBclMemoryExcludeCompile/ConsumeBclMemoryExcludeCompile.csproj

@@ -0,0 +1,17 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <NoWarn>$(NoWarn);PolyfillTargetsForNuget</NoWarn>
+    <TargetFramework>netstandard2.0</TargetFramework>
+  </PropertyGroup>
+  <ItemGroup>
+    <!-- ExcludeAssets="compile" exercises the case where a PackageReference is
+         present in the csproj but the resolved assembly is not in the compile
+         reference list that csc sees. Polyfill must keep emitting its own
+         Index/Range types in this configuration; suppressing them on the basis
+         of the PackageReference alone would leave the consumer with neither
+         the Polyfill-defined nor the BCL-provided types. -->
+    <PackageReference Include="Microsoft.Bcl.Memory" ExcludeAssets="compile" />
+  </ItemGroup>
+  <Import Project="$(SolutionDir)\TestIncludes.targets" />
+  <Import Project="$(SolutionDir)\Polyfill\Polyfill.targets" />
+</Project>

src/ConsumeBclMemoryNoResolve/ConsumeBclMemoryNoResolve.csproj

@@ -8,13 +8,5 @@
   </ItemGroup>
   <Import Project="$(SolutionDir)\TestIncludes.targets" />
   <Import Project="$(SolutionDir)\Polyfill\Polyfill.targets" />
-  <!-- Simulate IDE builds (e.g. Rider surface heuristics) where
-       ResolvedCompileFileDefinitions is empty because ResolvePackageAssets
-       did not run. The framework-static defines like FeatureValueTuple
-       are set, but package-dynamic ones like RefsBclMemory are not. -->
-  <Target Name="PreparePolyfill">
-    <PropertyGroup>
-      <DefineConstants Condition="$(LowerFramework.StartsWith('netstandard2.0'))">$(DefineConstants);FeatureValueTuple;FeatureRuntimeInformation;FeatureCompression</DefineConstants>
-    </PropertyGroup>
-  </Target>
+  <!-- Smoke test: Microsoft.Bcl.Memory referenced on netstandard2.0. -->
 </Project>

src/Polyfill.slnx

@@ -21,6 +21,7 @@
   <Project Path="ApiBuilderTests/ApiBuilderTests.csproj" />
   <Project Path="Consume/Consume.csproj" />
   <Project Path="ConsumeBclMemory/ConsumeBclMemory.csproj" />
+  <Project Path="ConsumeBclMemoryExcludeCompile/ConsumeBclMemoryExcludeCompile.csproj" />
   <Project Path="ConsumeBclMemoryNoResolve/ConsumeBclMemoryNoResolve.csproj" />
   <Project Path="ConsumeClassicReferences/ConsumeClassicReferences.csproj" />
   <Project Path="ConsumeConsumeBclMemory/ConsumeConsumeBclMemory.csproj" />

src/Polyfill/Polyfill.targets

@@ -1,6 +1,5 @@
 <Project>
   <PropertyGroup>
-    <PrepareForBuildDependsOn>$(PrepareForBuildDependsOn);PreparePolyfill</PrepareForBuildDependsOn>
     <LowerFramework>$(TargetFramework.ToLower())</LowerFramework>
     <LowerFrameworks>$(TargetFrameworks.ToLower())</LowerFrameworks>
   </PropertyGroup>
@@ -77,48 +76,48 @@ For example:
   </Target>
   <!-- end-snippet -->
 
-  <!-- Fallback: detect Microsoft.Bcl.Memory from PackageReference when
-       ResolvedCompileFileDefinitions is unavailable (e.g. IDE builds). -->
-  <Target Name="DetectBclMemoryPackageRef" BeforeTargets="PreparePolyfill;CoreCompile">
+  <!-- Feature detection runs after ResolveAssemblyReferences and before
+       CoreCompile. By that point @(ReferencePath) holds the final list of
+       assemblies that csc will see, with NuGetPackageId and NuGetPackageVersion
+       metadata preserved on items that came from NuGet packages. Each Feature*
+       / Refs* DefineConstant is set only when the corresponding assembly is
+       actually present in @(ReferencePath), so the conditional source files
+       that depend on those defines compile against a reference list that is
+       guaranteed to contain the types they use. -->
+  <Target Name="PolyfillResolveFeatures"
+          AfterTargets="ResolveAssemblyReferences"
+          BeforeTargets="CoreCompile">
     <PropertyGroup>
       <DefineConstants
-        Condition="!$(DefineConstants.Contains('RefsBclMemory')) AND
-                   @(PackageReference->AnyHaveMetadataValue('Identity', 'Microsoft.Bcl.Memory'))">$(DefineConstants);RefsBclMemory</DefineConstants>
-    </PropertyGroup>
-  </Target>
+        Condition="@(ReferencePath->AnyHaveMetadataValue('NuGetPackageId', 'System.ValueTuple'))">$(DefineConstants);FeatureValueTuple</DefineConstants>
 
-  <Target Name="PreparePolyfill" DependsOnTargets="ResolvePackageAssets">
-    <PropertyGroup>
       <DefineConstants
-        Condition="@(ResolvedCompileFileDefinitions->AnyHaveMetadataValue('NuGetPackageId', 'System.ValueTuple'))">$(DefineConstants);FeatureValueTuple</DefineConstants>
+        Condition="@(ReferencePath->AnyHaveMetadataValue('NuGetPackageId', 'Microsoft.Bcl.Memory'))">$(DefineConstants);RefsBclMemory</DefineConstants>
 
-      <DefineConstants
-        Condition="@(ResolvedCompileFileDefinitions->AnyHaveMetadataValue('NuGetPackageId', 'Microsoft.Bcl.Memory'))">$(DefineConstants);RefsBclMemory</DefineConstants>
+      <MemoryVersion>@(ReferencePath->WithMetadataValue('NuGetPackageId', 'System.Memory')->Metadata('NuGetPackageVersion'))</MemoryVersion>
 
-      <MemoryVersion>@(ResolvedCompileFileDefinitions->WithMetadataValue('NuGetPackageId', 'System.Memory')->Metadata('NuGetPackageVersion'))</MemoryVersion>
-      
       <DefineConstants
         Condition="'$(MemoryVersion)' != '' AND $([MSBuild]::VersionGreaterThanOrEquals($(MemoryVersion), '4.5.5'))">$(DefineConstants);FeatureMemory</DefineConstants>
-      
+
       <DefineConstants
-        Condition="@(ResolvedCompileFileDefinitions->AnyHaveMetadataValue('NuGetPackageId', 'System.Runtime.InteropServices.RuntimeInformation'))">$(DefineConstants);FeatureRuntimeInformation</DefineConstants>
-      
+        Condition="@(ReferencePath->AnyHaveMetadataValue('NuGetPackageId', 'System.Runtime.InteropServices.RuntimeInformation'))">$(DefineConstants);FeatureRuntimeInformation</DefineConstants>
+
       <DefineConstants
-        Condition="@(ResolvedCompileFileDefinitions->AnyHaveMetadataValue('NuGetPackageId', 'System.Threading.Tasks.Extensions'))">$(DefineConstants);FeatureValueTask</DefineConstants>
+        Condition="@(ReferencePath->AnyHaveMetadataValue('NuGetPackageId', 'System.Threading.Tasks.Extensions'))">$(DefineConstants);FeatureValueTask</DefineConstants>
 
       <DefineConstants
-        Condition="@(ResolvedCompileFileDefinitions->AnyHaveMetadataValue('NuGetPackageId', 'System.Net.Http')) OR
-                   @(Reference->AnyHaveMetadataValue('Identity', 'System.Net.Http'))">$(DefineConstants);FeatureHttp</DefineConstants>
+        Condition="@(ReferencePath->AnyHaveMetadataValue('NuGetPackageId', 'System.Net.Http')) OR
+                   @(ReferencePath->AnyHaveMetadataValue('Filename', 'System.Net.Http'))">$(DefineConstants);FeatureHttp</DefineConstants>
 
       <DefineConstants
-        Condition="@(ResolvedCompileFileDefinitions->AnyHaveMetadataValue('NuGetPackageId', 'System.IO.Compression')) OR
-                   @(Reference->AnyHaveMetadataValue('Identity', 'System.IO.Compression'))">$(DefineConstants);FeatureCompression</DefineConstants>
+        Condition="@(ReferencePath->AnyHaveMetadataValue('NuGetPackageId', 'System.IO.Compression')) OR
+                   @(ReferencePath->AnyHaveMetadataValue('Filename', 'System.IO.Compression'))">$(DefineConstants);FeatureCompression</DefineConstants>
 
       <DefineConstants
-        Condition="@(ResolvedCompileFileDefinitions->AnyHaveMetadataValue('NuGetPackageId', 'Microsoft.Bcl.AsyncInterfaces'))">$(DefineConstants);FeatureAsyncInterfaces</DefineConstants>
+        Condition="@(ReferencePath->AnyHaveMetadataValue('NuGetPackageId', 'Microsoft.Bcl.AsyncInterfaces'))">$(DefineConstants);FeatureAsyncInterfaces</DefineConstants>
 
       <DefineConstants
-        Condition="@(ResolvedCompileFileDefinitions->AnyHaveMetadataValue('NuGetPackageId', 'System.Runtime.CompilerServices.Unsafe'))">$(DefineConstants);FeatureUnsafe</DefineConstants>
+        Condition="@(ReferencePath->AnyHaveMetadataValue('NuGetPackageId', 'System.Runtime.CompilerServices.Unsafe'))">$(DefineConstants);FeatureUnsafe</DefineConstants>
 
       <DefineConstants Condition="$(LowerFramework.StartsWith('net11'))">$(DefineConstants);FeatureMemory;FeatureValueTuple;FeatureValueTask;FeatureRuntimeInformation;FeatureHttp;FeatureCompression;FeatureAsyncInterfaces;FeatureUnsafe</DefineConstants>
       <DefineConstants Condition="$(LowerFramework.StartsWith('net10'))">$(DefineConstants);FeatureMemory;FeatureValueTuple;FeatureValueTask;FeatureRuntimeInformation;FeatureHttp;FeatureCompression;FeatureAsyncInterfaces;FeatureUnsafe</DefineConstants>

target-frameworks.md

@@ -96,7 +96,7 @@ At build time, if a project is detected to not properly multi target, then a war
     >true</MaxNetClassicRequired>
 </PropertyGroup>
 ```
-<sup><a href='/src/Polyfill/Polyfill.targets#L35-L62' title='Snippet source file'>snippet source</a> | <a href='#snippet-MaxNetRequired' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Polyfill/Polyfill.targets#L34-L61' title='Snippet source file'>snippet source</a> | <a href='#snippet-MaxNetRequired' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 
@@ -120,7 +120,7 @@ For example:
       Condition="$(MaxNetRequired)" />
 </Target>
 ```
-<sup><a href='/src/Polyfill/Polyfill.targets#L63-L78' title='Snippet source file'>snippet source</a> | <a href='#snippet-PolyfillValidateNugetTargets' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Polyfill/Polyfill.targets#L62-L77' title='Snippet source file'>snippet source</a> | <a href='#snippet-PolyfillValidateNugetTargets' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->