Fix configuration binder dropping collection items with null constructor parameters

[rosebyte]

Fixes #131269

Description

When binding configuration to a collection of positional records (records with constructor parameters), any entry containing a null value for a nullable parameter is silently dropped. This is a regression introduced in .NET 10 by #116677 ("Support Null configuration").

Root cause

PR #116677 changed the JSON configuration provider to store JSON null as actual null (previously it was stored as ""). This exposed a latent issue in BindParameter: after BindInstance explicitly sets the binding point's value to null (a valid result for a nullable parameter), the code incorrectly interprets this as "no value was bound" and throws an InvalidOperationException. The exception is then swallowed by BindArray's try-catch, silently dropping the collection item.

Fix

Reflection-based binder (ConfigurationBinder.cs)

Changed the null check in BindParameter from:

if (propertyBindingPoint.Value is null)

to:

if (propertyBindingPoint.Value is null && !propertyBindingPoint.HasNewValue)

This distinguishes "the binder explicitly set null" (valid for nullable params) from "nothing was bound" (should fall back to default or throw).

Source generator (CoreBindingHelpers.cs)

For AssignFromSectionValue types (string, object) where the parameter type is nullable, the generated code now emits a simple assignment instead of a throw-on-null guard:

// Before (throws when configuration value is null):
if (configuration["Name"] is not string Name)
{
    throw new InvalidOperationException("...");
}

// After (null is a valid value for nullable types):
string Name = configuration["Name"];

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to 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.

[Copilot]

Pull request overview

Fixes a .NET 10 regression in configuration binding where collection elements were silently dropped when binding to positional-record constructor parameters and the configuration contained an explicit null for a nullable parameter. The PR updates both the reflection-based binder and the source-generator emitter, and adds coverage for the scenario.

Changes:

• Update ConfigurationBinder.BindParameter to distinguish “no value bound” from “explicitly bound to null” using BindingPoint.HasNewValue.
• Update the source-generator emitter to avoid throwing on null section values for AssignFromSectionValue ctor parameters when null is allowed.
• Add a regression test and supporting test types for binding a collection of positional records with a null value.

Reviewed changes

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

File	Description
src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs	Fixes ctor parameter binding logic to treat explicitly-bound null as a valid bound value.
src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs	Adjusts generator emission for required ctor parameters with section-value assignment, to accept null values.
src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Collections.cs	Adds regression test covering null values in a collection of positional records.
src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.Collections.cs	Adds test model types used by the new regression test.

[rosebyte]

I don't think we should validate options by binding - there might be a configuration callback assigning the field/property with a value so validation should happen only on the final instance. Moreover, if we will distinguish between explicit null and missing key, we may hurt users effectively removing a value by shadowing it with null, which is legit AFAIK.

[svick]

Shouldn't the source generator behave the same as the reflection implementation? (Copilot claims above this would cause different behaviors in the two, I haven't checked myself.)

[rosebyte]

I've changed both so they should be aligned.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs:1172

• The new else if (!CanBeNull(parameter.ParameterType)) changes BindParameter semantics for missing configuration: if a ctor parameter is a reference type or Nullable<T> and the config key is missing, we now silently pass null instead of throwing Error_ParameterHasNoMatchingConfig. The HasNewValue check already fixes the explicit-config-null case; keeping the throw here preserves the existing "missing key" behavior and avoids passing null into non-nullable ctor parameters (where the binder can't observe C# nullability annotations).

                else if (!CanBeNull(parameter.ParameterType))
                {
                    throw new InvalidOperationException(SR.Format(SR.Error_ParameterHasNoMatchingConfig, type, parameterName));
                }

src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs:1072

• CanBeNull is only used to relax the missing-config throw in BindParameter. If we keep the existing behavior of throwing on missing config (and rely on HasNewValue to allow explicit null), this helper becomes unused and should be removed to avoid dead code.

This issue also appears on line 1169 of the same file.

        private static bool CanBeNull(Type type)
            => !type.IsValueType || (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>));

[svick]

Do we want to backport this to .Net 10?

[rosebyte]

Do we want to backport this to .Net 10?

Makes sense as it was introduced in net10.0 and it's LTS.