Configuration Binder SG: ignore unresolvable metadata types to prevent ambiguous/missing-type emissions

[Copilot]

Summary

When the Configuration Binder source generator processes a type whose members reference types from a transitive assembly that isn't directly referenced by the consumer (or that resolve ambiguously), those member types degrade to Roslyn error symbols. The generator previously emitted binding code that named these types, producing uncompilable output — missing-type (CS0012/CS0234) and namespace-ambiguity errors.

The reflection-based binder doesn't hit this (the assemblies are present at runtime), so the source generator was strictly worse: it broke the build. This is the failure reported in #109759 (Microsoft.IdentityModel.S2S.DstsExtensions with trimming, involving CredentialDescription / HttpRequestMessage).

Fixes #109759.

Changes

1. Robust unsupported-type detection

IsUnsupportedType now treats a member type as unsupported when it — or a nested array element or generic type argument — is an error symbol, via a new recursive ContainsErrorType helper. This covers:

• direct error-typed members — e.g. HttpRequestMessage (unresolved reference type),
• Nullable<T> over an unresolved value type — e.g. ValueTypeMessage?,
• error types nested inside a resolvable generic — e.g. Wrapper<Missing>, System.Tuple<int, Missing>, KeyValuePair<int, Missing>?, Missing[].

2. Skips are no longer silent (SYSLIB1101)

When a property is skipped specifically because its type contains unresolvable metadata, the generator now emits the existing SYSLIB1101 warning ("Property '{0}' on type '{1}' is not supported.") at the binder call site, so users know binding was dropped and can add the missing reference. Other intentionally-unsupported types (IntPtr, delegates, …) remain silent, matching existing behavior.

This makes the reflection-vs-source-gen parity gap visible and actionable without falling back to reflection (which would undermine trimming in the reported scenario).

3. Regression coverage without disk I/O

IgnorePropertiesWithUnresolvableMetadataTypes composes a transitive-dependency assembly, a model assembly referencing it, and a generator input that references only the model — so the affected member types become unresolved error symbols. It asserts:

• the unresolvable members (value-type nullable, reference type, and generic wrappers) are excluded from the generated code,
• the bindable member still binds,
• a SYSLIB1101 warning is reported for each skipped member.

The test references the model as an in-memory MetadataReference (the test driver was extended to accept IEnumerable<MetadataReference>), replacing the previous approach of writing assemblies to disk and using Assembly.LoadFrom — which locked the files on Windows and leaked a temp directory on every run.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR improves robustness of the Microsoft.Extensions.Configuration.Binder source generator by filtering out members whose types resolve to Roslyn error symbols (e.g., missing/ambiguous metadata types), preventing the generator from emitting uncompilable references. It also adds a regression test that simulates a missing transitive dependency via in-memory metadata references.

Changes:

• Treat TypeKind.Error (including within arrays/generics) as unsupported during parsing, and skip emitting binding for those members.
• Add test infrastructure support for supplying extra MetadataReferences to the generator test project.
• Add a regression test that omits a transitive reference to ensure unresolved member types are excluded while valid members still bind.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Helpers.cs	Extends test helper to pass optional MetadataReferences through to the test driver.
src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs	Adds regression test covering unresolved metadata member types.
src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigBindingGenTestDriver.cs	Adds support for injecting extra Roslyn metadata references into the test compilation.
src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs	Skips members containing Roslyn error types and emits SYSLIB1101 warnings for skipped members to avoid uncompilable output.

[svick]

I don't know if this is a realistic situation that's worth handling, but otherwise LGTM.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[tarekgh]

In the property loop, this diagnostic is emitted before the IsOverride / duplicate-name check below. Because the loop walks the base chain (current = current.BaseType), an error-typed property that is overridden reports SYSLIB1101 twice: once for the override on the derived type and again for the declaration on the base type. Before this change those members went through the normal path where the override/duplicate filter capped it at a single diagnostic, so this is a new double-warning.

Computing the override/duplicate condition first and gating the diagnostic on it keeps the single-warning behavior, and leaves binding for supported properties unchanged:

Suggested change

	if (member is IPropertySymbol { IsIndexer: false, IsImplicitlyDeclared: false } property)
	{
	if (IsUnsupportedType(property.Type))
	{
	if (ContainsErrorType(property.Type))
	{
	RecordDiagnostic(DiagnosticDescriptors.PropertyNotSupported, typeParseInfo.BinderInvocation?.Location, [property.Name, typeParseInfo.FullName]);
	}
	continue;
	}
	string propertyName = property.Name;
	if (property.IsOverride || properties?.ContainsKey(propertyName) is true)
	if (member is IPropertySymbol { IsIndexer: false, IsImplicitlyDeclared: false } property)
	{
	string propertyName = property.Name;
	bool isDuplicateOrOverride = property.IsOverride || properties?.ContainsKey(propertyName) is true;
	if (IsUnsupportedType(property.Type))
	{
	if (ContainsErrorType(property.Type) && !isDuplicateOrOverride)
	{
	RecordDiagnostic(DiagnosticDescriptors.PropertyNotSupported, typeParseInfo.BinderInvocation?.Location, [propertyName, typeParseInfo.FullName]);
	}
	continue;
	}
	if (isDuplicateOrOverride)

This handles the override case. A new-shadowed error-typed property would still warn twice, since a skipped member is never added to properties for the base occurrence to match against, which is likely rare enough to leave as-is but worth a note.

[tarekgh]

This assertion doesn't actually verify that DstsOptions.Value binds. The generated binder always contains the substring "Value" from boilerplate unrelated to the property, for example HasValueOrChildren(...) (emitted unconditionally by GetCore for any Get<T>), TryGetConfigurationValue, and (configuration as IConfigurationSection)?.Value. So it passes for any Get<T> even if the Value property were dropped, which is the regression this test is meant to guard against.

Asserting on the actual assignment makes it meaningful:

Suggested change

	Assert.Contains("Value", result.GeneratedSource.Value.SourceText.ToString());
	Assert.Contains("instance.Value = ", result.GeneratedSource.Value.SourceText.ToString());

The DoesNotContain(... "'Value'") diagnostic check further down is already specific enough thanks to the quotes, so no change needed there.

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "999253e490e1d0a15ac7f561c2b905b064241e9d",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "6e0eb8307f56dc6fea81aa4cce2cf40c1013fa4d",
  "last_reviewed_commit": "999253e490e1d0a15ac7f561c2b905b064241e9d",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "6e0eb8307f56dc6fea81aa4cce2cf40c1013fa4d",
  "last_recorded_worker_run_id": "29684909583",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "999253e490e1d0a15ac7f561c2b905b064241e9d",
      "review_id": 4730671773
    }
  ]
}

[github-actions]

Holistic Review

Motivation: The Configuration Binder source generator emitted binding code that named member types which resolve to Roslyn error symbols — types from transitive assemblies not directly referenced by the consumer, or that resolve ambiguously. Naming such types produces uncompilable output (CS0012/CS0234/namespace-ambiguity), so the source generator was strictly worse than the reflection binder, which succeeds at runtime when the assemblies are present. This is the failure reported in #109759.

Approach: Adds a recursive ContainsErrorType helper that treats a type as unsupported when the type itself, a nested array element, or a generic type argument is an error symbol. IsUnsupportedType now short-circuits on this, and CreateObjectSpec skips such properties instead of emitting them. Skips caused specifically by unresolvable metadata now surface the existing SYSLIB1101 warning at the binder call site, so the parity gap is visible and actionable rather than silent, while other intentionally-unsupported types (IntPtr, delegates, ...) stay silent. Test coverage composes an in-memory transitive-dependency assembly plus a model assembly and references only the model, so the affected member types degrade to error symbols; it asserts exclusion from generated code, that the bindable member still binds, and that SYSLIB1101 is reported per skipped member. The test driver was extended to accept IEnumerable<MetadataReference>, replacing a prior on-disk Assembly.LoadFrom approach that locked files on Windows and leaked temp directories.

Summary: This is a well-targeted, correct fix that resolves a real build-breaking behavior with good, self-contained test coverage and no disk I/O. The ContainsErrorType recursion correctly covers direct error types, Nullable<T> over an unresolved value type, arrays, and error types nested inside resolvable generics. Emitting SYSLIB1101 only for the error-type case (not for all unsupported types) preserves existing behavior for IntPtr/delegates. The test-driver refactor is a clean improvement. I noted one non-blocking edge case inline: because the member loop walks base types, the SYSLIB1101 diagnostic is emitted before the override/name-shadowing dedup, so an overridden or shadowed error-typed property could report a duplicate warning. The PR already has multiple maintainer approvals. LGTM.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 103.4 AIC · ⌖ 10.6 AIC · ⊞ 10K

[github-actions]

Minor: this SYSLIB1101 is emitted before the override/shadowing dedup that the supported-property path performs below (property.IsOverride || properties?.ContainsKey(...)). Because the loop walks base types, a property that contains an error type and is overridden (or name-shadowed) in a derived class can produce a duplicate SYSLIB1101 for the same property name — once for the derived declaration and once for the base. Consider deduping (e.g. tracking already-reported names, or moving the override/ContainsKey check ahead of the diagnostic) so the skip warning is reported at most once per property. Not blocking.