Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -966,17 +966,28 @@ private bool IsPropertyReboundInBindCore(PropertySpec property)
case ConfigurationSectionSpec:
return property.CanSet;
case ComplexTypeSpec complexType:
// EmitBindImplForMember skips a complex member only when it is a
// parameterized-constructor object with no bindable members. Every other complex member is bound.
return _typeIndex.HasBindableMembers(complexType) ||
complexType.IsValueType ||
complexType is CollectionSpec ||
complexType is not ObjectSpec { InstantiationStrategy: ObjectInstantiationStrategy.ParameterizedConstructor };
return IsBindableAsMember(complexType, property.CanSet);
default:
return false;
}
}

/// <summary>
/// Whether binding a member of type <paramref name="complexType"/> emits any binding logic. An object
/// created through a parameterized constructor binds its constructor parameters in its Initialize method,
/// so it is bindable even without any bindable member of its own (e.g. its only member is a constructor
/// parameter backed by a read-only collection type) - but only where the member can be assigned the
/// instance that Initialize creates. One that has neither bindable members nor an instance to assign has
/// nothing to bind, and is skipped. A value-type member is bindable only when it can be set at all, since
/// binding one in place would only mutate the copy its getter returns.
/// </summary>
private bool IsBindableAsMember(ComplexTypeSpec complexType, bool canSet) =>
complexType.IsValueType
? canSet && (_typeIndex.HasBindableMembers(complexType) || _typeIndex.CanInstantiate(complexType))
: _typeIndex.HasBindableMembers(complexType) ||
complexType is not ObjectSpec { InstantiationStrategy: ObjectInstantiationStrategy.ParameterizedConstructor } ||
(canSet && _typeIndex.CanInstantiate(complexType));

private bool EmitBindImplForMember(
MemberSpec member,
string memberAccessExpr,
Expand Down Expand Up @@ -1082,10 +1093,7 @@ private bool EmitBindImplForMember(
case ComplexTypeSpec complexType:
{
// Early detection of types we cannot bind to and skip it.
if (!_typeIndex.HasBindableMembers(complexType) &&
!complexType.IsValueType &&
complexType is not CollectionSpec &&
((ObjectSpec)complexType).InstantiationStrategy == ObjectInstantiationStrategy.ParameterizedConstructor)
if (!IsBindableAsMember(complexType, canSet))
{
return false;
}
Expand Down Expand Up @@ -1142,7 +1150,17 @@ private void EmitBindingLogicForComplexMember(
return;
}

Debug.Assert(canSet);
// A value type with nothing to bind beyond its constructor parameters is created outright by its
// Initialize method. There is nothing to bind in place, so assign the created instance straight to
// the member rather than through the temporary the in-place binding path below needs.
if (!_typeIndex.HasBindableMembers(effectiveMemberType) &&
effectiveMemberType is ObjectSpec { InstantiationStrategy: ObjectInstantiationStrategy.ParameterizedConstructor } &&
_typeIndex.CanInstantiate(effectiveMemberType))
{
EmitObjectInit(effectiveMemberType, memberAccessExpr, InitializationKind.SimpleAssignment, configArgExpr);
return;
}

string effectiveMemberTypeFQN = effectiveMemberType.TypeRef.FullyQualifiedName;
initKind = InitializationKind.None;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2479,5 +2479,25 @@ public void BindIEnumerableOfPositionalRecordWithNullProperty()
Assert.Equal(2, users[1].Id);
Assert.Null(users[1].Name);
}

[Fact]
public void CanBindNestedTypeWhoseSoleMemberIsAReadOnlyCollectionConstructorParameter()
{
IConfiguration config = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string>
{
{ "Class:Values:0", "a" },
{ "Struct:Values:0", "b" },
{ "NullableStruct:Values:0", "c" },
})
.Build();

var options = config.Get<SoleReadOnlyCollectionParamHolder>();

Assert.NotNull(options);
Assert.Equal(new[] { "a" }, options.Class?.Values);
Assert.Equal(new[] { "b" }, options.Struct.Values);
Assert.Equal(new[] { "c" }, options.NullableStruct?.Values);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -515,5 +515,16 @@ public class OptionsWithPositionalRecordCollection
public string? EnableFeatureX { get; set; }
public List<PositionalRecordWithNullableParam> Users { get; set; } = new();
}

public record SoleReadOnlyCollectionParamRecord(IReadOnlyList<string> Values);

public readonly record struct SoleReadOnlyCollectionParamStruct(IReadOnlyList<string> Values);

public class SoleReadOnlyCollectionParamHolder
{
public SoleReadOnlyCollectionParamRecord? Class { get; set; }
public SoleReadOnlyCollectionParamStruct Struct { get; set; }
public SoleReadOnlyCollectionParamStruct? NullableStruct { get; set; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -553,9 +553,8 @@ public async Task SoleReadOnlyCollectionConstructorParameterIsBindable(string co
// constructor parameter (no other bindable property) used to make the generator emit a
// call to an Initialize method that was never generated, producing CS0103 at compile time.
//
// This only covers the top-level GetCore path. Binding this same shape as a *nested*
// member (reached via BindCore/EmitObjectInit) silently produces null instead of the
// real value, a separate pre-existing bug tracked in dotnet/runtime#131399.
// This only covers the top-level GetCore path; NestedSoleReadOnlyCollectionConstructorParameterIsBindable
// covers the same shape reached as a nested member.
string source = $$"""
using Microsoft.Extensions.Configuration;
using System.Collections.Generic;
Expand Down Expand Up @@ -591,6 +590,160 @@ public record Options({{collectionType}}<string> Values);
Assert.Equal(new[] { "a", "b" }, boundValues);
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
[InlineData("IReadOnlyList")]
[InlineData("IReadOnlyCollection")]
[InlineData("IReadOnlySet")]
[InlineData("IEnumerable")]
public async Task NestedSoleReadOnlyCollectionConstructorParameterIsBindable(string collectionType)
{
// Regression test: the same shape as SoleReadOnlyCollectionConstructorParameterIsBindable, but
// reached as a nested member rather than as the top-level bound type. The member was silently
// left at null, since the emitter skipped every complex member without bindable members - even
// one whose constructor parameters its Initialize method binds. Covers every way such a member is
// reached: a constructor parameter (bound in Initialize), a settable property (bound in BindCore),
// and a settable property with a matching constructor parameter (bound in Initialize when the
// instance is created, and in BindCore when binding an existing one).
string source = $$"""
using Microsoft.Extensions.Configuration;
using System.Collections.Generic;

public class Program
{
public static object? Result;

public static void Main()
{
ConfigurationBuilder configurationBuilder = new();
configurationBuilder.AddInMemoryCollection(new Dictionary<string, string?>
{
["Nested:Values:0"] = "a",
["Nested:Values:1"] = "b",
});
IConfiguration config = configurationBuilder.Build();

Outer outer = config.Get<Outer>();
Holder holder = config.Get<Holder>();
Rebindable rebindable = config.Get<Rebindable>();

Rebindable existing = new(null!);
config.Bind(existing);

Result = new object?[]
{
outer.Nested?.Values,
holder.Nested?.Values,
rebindable.Nested?.Values,
existing.Nested?.Values,
};
}
}

public record Inner({{collectionType}}<string> Values);

public record Outer(Inner Nested);

public class Holder
{
public Inner Nested { get; set; }
}

public class Rebindable
{
public Rebindable(Inner nested) => Nested = nested;

public Inner Nested { get; set; }
}
""";

ConfigBindingGenRunResult result = await RunGeneratorAndUpdateCompilation(source, assemblyReferences: GetAssemblyRefsWithAdditional(typeof(ConfigurationBuilder), typeof(List<>)));
Assert.NotNull(result.GeneratedSource);
Assert.Empty(result.Diagnostics);

var boundValues = (object?[])LoadAndInvokeMain(result.OutputCompilation, "Result")!;
Assert.All(boundValues, boundValue => Assert.Equal(new[] { "a", "b" }, (IEnumerable<string>?)boundValue));
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
[InlineData("IReadOnlyList")]
[InlineData("IReadOnlyCollection")]
[InlineData("IReadOnlySet")]
[InlineData("IEnumerable")]
public async Task NestedSoleReadOnlyCollectionConstructorParameterOfStructIsBindable(string collectionType)
{
// The value-type counterpart of NestedSoleReadOnlyCollectionConstructorParameterIsBindable. A struct
// member is bound through a temporary (binding one in place would only mutate the copy its getter
// returns), and that path never instantiated a type without bindable members - so a struct whose only
// member is a read-only collection constructor parameter was left at its default. Covers a settable
// property, a nullable one, a constructor parameter, and a settable property with a matching
// constructor parameter.
string source = $$"""
using Microsoft.Extensions.Configuration;
using System.Collections.Generic;

public class Program
{
public static object? Result;

public static void Main()
{
ConfigurationBuilder configurationBuilder = new();
configurationBuilder.AddInMemoryCollection(new Dictionary<string, string?>
{
["Nested:Values:0"] = "a",
["Nested:Values:1"] = "b",
});
IConfiguration config = configurationBuilder.Build();

Holder holder = config.Get<Holder>();
NullableHolder nullableHolder = config.Get<NullableHolder>();
Outer outer = config.Get<Outer>();
Rebindable rebindable = config.Get<Rebindable>();

Rebindable existing = new(default);
config.Bind(existing);

Result = new object?[]
{
holder.Nested.Values,
nullableHolder.Nested?.Values,
outer.Nested.Values,
rebindable.Nested.Values,
existing.Nested.Values,
};
}
}

public readonly record struct Inner({{collectionType}}<string> Values);

public class Holder
{
public Inner Nested { get; set; }
}

public class NullableHolder
{
public Inner? Nested { get; set; }
}

public record Outer(Inner Nested);

public class Rebindable
{
public Rebindable(Inner nested) => Nested = nested;

public Inner Nested { get; set; }
}
""";

ConfigBindingGenRunResult result = await RunGeneratorAndUpdateCompilation(source, assemblyReferences: GetAssemblyRefsWithAdditional(typeof(ConfigurationBuilder), typeof(List<>)));
Assert.NotNull(result.GeneratedSource);
Assert.Empty(result.Diagnostics);

var boundValues = (object?[])LoadAndInvokeMain(result.OutputCompilation, "Result")!;
Assert.All(boundValues, boundValue => Assert.Equal(new[] { "a", "b" }, (IEnumerable<string>?)boundValue));
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
public async Task SoleReadOnlyCollectionConstructorParameterOfComplexElementIsBindable()
{
Expand Down
Loading