Skip to content
Open
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 @@ -169,7 +169,17 @@ bool TryRegisterCore()
// an error for config properties that don't map to object properties.
_namespaces.Add("System.Collections.Generic");

if (_typeIndex.HasBindableMembers(objectSpec))
bool hasBindableMembers = _typeIndex.HasBindableMembers(objectSpec);

// A type with a parameterized constructor gets its constructor parameters bound
// in the Initialize method regardless of whether it also has other bindable
// members; that binding capability must be registered even when
// HasBindableMembers is false (e.g. the only member is a ctor parameter backed
// by a non-bindable read-only collection type), otherwise the emitter can end up
// calling an Initialize method that was never generated.
bool needsInitializeMethod = objectSpec is { InstantiationStrategy: ObjectInstantiationStrategy.ParameterizedConstructor, InitExceptionMessage: null };

if (hasBindableMembers || needsInitializeMethod)
{
foreach (PropertySpec property in objectSpec.Properties!)
{
Expand All @@ -189,10 +199,13 @@ bool TryRegisterCore()
}
}

bool registeredForBindCore = TryRegisterTypeForBindCoreGen(objectSpec);
Debug.Assert(registeredForBindCore);
if (hasBindableMembers)
{
bool registeredForBindCore = TryRegisterTypeForBindCoreGen(objectSpec);
Debug.Assert(registeredForBindCore);
}

if (objectSpec is { InstantiationStrategy: ObjectInstantiationStrategy.ParameterizedConstructor, InitExceptionMessage: null })
if (needsInitializeMethod)
{
RegisterTypeForMethodGen(MethodsToGen_CoreBindingHelper.Initialize, objectSpec);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.IO;
using System.Linq;
using System.Reflection;
using System.Runtime.Loader;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
Expand Down Expand Up @@ -230,5 +231,21 @@ private static byte[] CreateAssemblyImage(Compilation compilation)
}
return stream.ToArray();
}

/// <summary>
/// Compiles the source-generated output to an in-memory assembly, invokes its Program.Main(),
/// and returns the value of the public static field named <paramref name="resultFieldName"/>.
/// Used to assert on real bound values rather than just that the generated code compiles,
/// since a generator can emit code that builds cleanly but binds the wrong data.
/// </summary>
private static object? LoadAndInvokeMain(Compilation compilation, string resultFieldName)
{
byte[] image = CreateAssemblyImage(compilation);
Assembly assembly = AssemblyLoadContext.Default.LoadFromStream(new MemoryStream(image));

Type programType = assembly.GetType("Program")!;
programType.GetMethod("Main", BindingFlags.Public | BindingFlags.Static)!.Invoke(null, null);
return programType.GetField(resultFieldName, BindingFlags.Public | BindingFlags.Static)!.GetValue(null);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,97 @@ public record Options(string Name, {{collectionType}}<string> Values);
AssertCanCreateAssemblyImage(result.OutputCompilation);
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
[InlineData("IReadOnlyList")]
[InlineData("IReadOnlyCollection")]
[InlineData("IReadOnlySet")]
[InlineData("IEnumerable")]
Comment thread
tarekgh marked this conversation as resolved.
public async Task SoleReadOnlyCollectionConstructorParameterIsBindable(string collectionType)
{
// Regression test: a type whose only member is a non-bindable, read-only collection
// 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.
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?>
{
["Values:0"] = "a",
["Values:1"] = "b",
});
IConfiguration config = configurationBuilder.Build();
Options options = config.Get<Options>();
Result = options.Values;
}
}

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two coverage gaps around this shape:

  1. Sole member plus complex element type is not exercised. The existing ComplexReadOnlyListConstructorParameterIsBindable pairs the collection with a bindable Name (so HasBindableMembers is true, the old path). Here the element is string. The intersection, for example record Options(IReadOnlyList<Child> Values) with no other member, is what newly drives the property walk transitive registration and RegisterForGen_AsConfigWithChildrenHelper in the changed code path, and it is untested.
  2. Nested usage is not exercised. Reaching this type as a member of an outer bound type goes through BindCore and EmitObjectInit rather than top level GetCore, which is a distinct emission path. It very likely works, but a nested case would lock it in.

""";

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

// Compiling only proves the Initialize method the fix registers is emitted; loading and
// running the assembly proves it also binds the right values, not just compilable code.
var boundValues = (IEnumerable<string>)LoadAndInvokeMain(result.OutputCompilation, "Result")!;
Assert.Equal(new[] { "a", "b" }, boundValues);
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
public async Task SoleReadOnlyCollectionConstructorParameterOfComplexElementIsBindable()
{
// Same regression as SoleReadOnlyCollectionConstructorParameterIsBindable, but the element
// type is itself a bindable object rather than a string. ComplexReadOnlyListConstructorParameterIsBindable
// below covers the complex-element case, but always pairs it with a second, ordinarily-bindable
// property, so it never exercises the fixed code path (the type has bindable members either way).
string source = """
using Microsoft.Extensions.Configuration;
using System.Collections.Generic;
using System.Linq;

public class Program
{
public static object? Result;

public static void Main()
{
ConfigurationBuilder configurationBuilder = new();
configurationBuilder.AddInMemoryCollection(new Dictionary<string, string?>
{
["Values:0:Value"] = "a",
["Values:1:Value"] = "b",
});
IConfiguration config = configurationBuilder.Build();
Options options = config.Get<Options>();
Result = options.Values.Select(v => v.Value).ToArray();
}
}

public class Child
{
public string Value { get; set; }
}

public record Options(IReadOnlyList<Child> Values);
""";

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

var boundValues = (string[])LoadAndInvokeMain(result.OutputCompilation, "Result")!;
Assert.Equal(new[] { "a", "b" }, boundValues);
}

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