Skip to content

Commit a4459b5

Browse files
github-actions[bot]buyaa-n
github-actions[bot]
and
buyaa-n
authored
[release/8.0] Fix: Config binder generator doesn't generate code when named arguments are out of order (#92257)
* Fix Named parameters bug * Test the generator only, don't compare generated file row by row * Add other named parameter combinatios for other overloads in the test, add test for OptionsBuilder... and ServiceCollection extensins * Adjust line numbers with source generator updates * Move similar code section into helper method, don't exact exact line count * Apply feedbacks --------- Co-authored-by: Buyaa Namnan <bunamnan@microsoft.com>
1 parent 4b7c754 commit a4459b5

File tree

6 files changed

+223
-4
lines changed

6 files changed

+223
-4
lines changed

‎src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Parser/ConfigurationBinder.cs‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ private void ParseBindInvocation_ConfigurationBinder(BinderInvocation invocation
7777
_ => throw new InvalidOperationException()
7878
};
7979

80-
IArgumentOperation instanceArg = operation.Arguments[instanceIndex];
80+
IArgumentOperation instanceArg = GetArgumentForParameterAtIndex(operation.Arguments, instanceIndex);
8181
if (instanceArg.Parameter.Type.SpecialType != SpecialType.System_Object)
8282
{
8383
return;
@@ -119,6 +119,19 @@ private void ParseBindInvocation_ConfigurationBinder(BinderInvocation invocation
119119
};
120120
}
121121

122+
private static IArgumentOperation GetArgumentForParameterAtIndex(ImmutableArray<IArgumentOperation> arguments, int parameterIndex)
123+
{
124+
foreach (var argument in arguments)
125+
{
126+
if (argument.Parameter?.Ordinal == parameterIndex)
127+
{
128+
return argument;
129+
}
130+
}
131+
132+
throw new InvalidOperationException();
133+
}
134+
122135
private void ParseGetInvocation(BinderInvocation invocation)
123136
{
124137
IInvocationOperation operation = invocation.Operation!;
@@ -158,7 +171,7 @@ private void ParseGetInvocation(BinderInvocation invocation)
158171
}
159172
else
160173
{
161-
ITypeOfOperation? typeOfOperation = operation.Arguments[1].ChildOperations.FirstOrDefault() as ITypeOfOperation;
174+
ITypeOfOperation? typeOfOperation = GetArgumentForParameterAtIndex(operation.Arguments, 1).ChildOperations.FirstOrDefault() as ITypeOfOperation;
162175
type = typeOfOperation?.TypeOperand;
163176

164177
if (paramCount is 2)
@@ -218,7 +231,7 @@ private void ParseGetValueInvocation(BinderInvocation invocation)
218231
return;
219232
}
220233

221-
ITypeOfOperation? typeOfOperation = operation.Arguments[1].ChildOperations.FirstOrDefault() as ITypeOfOperation;
234+
ITypeOfOperation? typeOfOperation = GetArgumentForParameterAtIndex(operation.Arguments, 1).ChildOperations.FirstOrDefault() as ITypeOfOperation;
222235
type = typeOfOperation?.TypeOperand;
223236

224237
if (paramCount is 3)

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBinderTests.Generator.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ internal class ClassWithThisIdentifier
317317
/// <summary>
318318
/// These are regression tests for https://github.com/dotnet/runtime/issues/90909.
319319
/// Ensure that we don't emit root interceptors to handle types/members that
320-
/// are inaccessible to the generated helpers. Tests for inaccessbile transitive members
320+
/// are inaccessible to the generated helpers. Tests for inaccessible transitive members
321321
/// are covered in the shared (reflection/src-gen) <see cref="ConfigurationBinderTests"/>,
322322
/// e.g. <see cref="NonPublicModeGetStillIgnoresReadonly"/>.
323323
/// </summary>

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.Options.cs‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,81 @@ public async Task Configure_T_BinderOptions() =>
6060
public async Task Configure_T_name_BinderOptions() =>
6161
await VerifyAgainstBaselineUsingFile("Configure_T_name_BinderOptions.generated.txt", GetConfigureSource(@""""", section, _ => { }"), extType: ExtensionClassType.ServiceCollection);
6262

63+
[Theory]
64+
[InlineData("OptionsConfigurationServiceCollectionExtensions.Configure<MyClass>(config: section, services: services);")]
65+
[InlineData("""OptionsConfigurationServiceCollectionExtensions.Configure<MyClass>(name: "", config: section, services: services);""")]
66+
[InlineData("OptionsConfigurationServiceCollectionExtensions.Configure<MyClass>(configureBinder: _ => { }, config: section, services: services);")]
67+
[InlineData("""OptionsConfigurationServiceCollectionExtensions.Configure<MyClass>(configureBinder: _ => { }, config: section, name: "", services: services);""")]
68+
[InlineData("""OptionsConfigurationServiceCollectionExtensions.Configure<MyClass>(name: "", services: services, configureBinder: _ => { }, config: section);""")]
69+
public async Task Configure_T_NamedParameters_OutOfOrder(string row)
70+
{
71+
string source = $$"""
72+
using System.Collections.Generic;
73+
using Microsoft.Extensions.Configuration;
74+
using Microsoft.Extensions.DependencyInjection;
75+
76+
public class Program
77+
{
78+
public static void Main()
79+
{
80+
ConfigurationBuilder configurationBuilder = new();
81+
IConfiguration config = configurationBuilder.Build();
82+
IConfigurationSection section = config.GetSection("MySection");
83+
ServiceCollection services = new();
84+
85+
{{row}}
86+
}
87+
88+
public class MyClass
89+
{
90+
public string MyString { get; set; }
91+
public int MyInt { get; set; }
92+
public List<int> MyList { get; set; }
93+
public Dictionary<string, string> MyDictionary { get; set; }
94+
}
95+
}
96+
""";
97+
98+
await VerifyThatSourceIsGenerated(source);
99+
}
100+
101+
[Theory]
102+
[InlineData("OptionsBuilderConfigurationExtensions.Bind(config: config, optionsBuilder: optionsBuilder);")]
103+
[InlineData("OptionsBuilderConfigurationExtensions.Bind(configureBinder: _ => { }, config: config, optionsBuilder: optionsBuilder);")]
104+
[InlineData("OptionsBuilderConfigurationExtensions.Bind(config: config, configureBinder: _ => { }, optionsBuilder: optionsBuilder);")]
105+
public async Task Bind_T_NamedParameters_OutOfOrder(string row)
106+
{
107+
string source = $$"""
108+
using System.Collections.Generic;
109+
using Microsoft.Extensions.Configuration;
110+
using Microsoft.Extensions.DependencyInjection;
111+
using Microsoft.Extensions.Options;
112+
113+
public class Program
114+
{
115+
public static void Main()
116+
{
117+
ConfigurationBuilder configurationBuilder = new();
118+
IConfiguration config = configurationBuilder.Build();
119+
var services = new ServiceCollection();
120+
OptionsBuilder<MyClass> optionsBuilder = new(services, "");
121+
122+
{{row}}
123+
}
124+
125+
public class MyClass
126+
{
127+
public string MyString { get; set; }
128+
public int MyInt { get; set; }
129+
public List<int> MyList { get; set; }
130+
public Dictionary<string, string> MyDictionary { get; set; }
131+
}
132+
}
133+
""";
134+
135+
await VerifyThatSourceIsGenerated(source);
136+
}
137+
63138
private string GetBindSource(string? configureActions = null) => $$"""
64139
using System.Collections.Generic;
65140
using Microsoft.Extensions.Configuration;

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.cs‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,112 @@ public partial class ConfigurationBindingGeneratorTests
1515
public async Task Bind() =>
1616
await VerifyAgainstBaselineUsingFile("Bind.generated.txt", BindCallSampleCode, extType: ExtensionClassType.ConfigurationBinder);
1717

18+
[Theory]
19+
[InlineData("ConfigurationBinder.Bind(instance: configObj, configuration: config);")]
20+
[InlineData("""ConfigurationBinder.Bind(key: "", instance: configObj, configuration: config);""")]
21+
[InlineData("""ConfigurationBinder.Bind(instance: configObj, key: "", configuration: config);""")]
22+
[InlineData("ConfigurationBinder.Bind(configureOptions: _ => { }, configuration: config, instance: configObj);")]
23+
[InlineData("ConfigurationBinder.Bind(configuration: config, configureOptions: _ => { }, instance: configObj);")]
24+
public async Task Bind_NamedParameters_OutOfOrder(string row)
25+
{
26+
string source = $$"""
27+
using System.Collections.Generic;
28+
using Microsoft.Extensions.Configuration;
29+
30+
public class Program
31+
{
32+
public static void Main()
33+
{
34+
ConfigurationBuilder configurationBuilder = new();
35+
IConfigurationRoot config = configurationBuilder.Build();
36+
37+
MyClass configObj = new();
38+
{{row}}
39+
}
40+
41+
public class MyClass
42+
{
43+
public string MyString { get; set; }
44+
public int MyInt { get; set; }
45+
public List<int> MyList { get; set; }
46+
public Dictionary<string, string> MyDictionary { get; set; }
47+
}
48+
}
49+
""";
50+
51+
await VerifyThatSourceIsGenerated(source);
52+
}
53+
54+
[Theory]
55+
[InlineData("var obj = ConfigurationBinder.Get(type: typeof(MyClass), configuration: config);")]
56+
[InlineData("var obj = ConfigurationBinder.Get<MyClass>(configureOptions: _ => { }, configuration: config);")]
57+
[InlineData("var obj = ConfigurationBinder.Get(configureOptions: _ => { }, type: typeof(MyClass), configuration: config);")]
58+
[InlineData("var obj = ConfigurationBinder.Get(type: typeof(MyClass), configureOptions: _ => { }, configuration: config);")]
59+
public async Task Get_TypeOf_NamedParametersOutOfOrder(string row)
60+
{
61+
string source = $$"""
62+
using System.Collections.Generic;
63+
using Microsoft.Extensions.Configuration;
64+
65+
public class Program
66+
{
67+
public static void Main()
68+
{
69+
ConfigurationBuilder configurationBuilder = new();
70+
IConfigurationRoot config = configurationBuilder.Build();
71+
72+
MyClass configObj = new();
73+
{{row}}
74+
}
75+
76+
public class MyClass
77+
{
78+
public string MyString { get; set; }
79+
public int MyInt { get; set; }
80+
public List<int> MyList { get; set; }
81+
public Dictionary<string, string> MyDictionary { get; set; }
82+
}
83+
}
84+
""";
85+
86+
await VerifyThatSourceIsGenerated(source);
87+
}
88+
89+
[Theory]
90+
[InlineData("""var str = ConfigurationBinder.GetValue(key: "key", configuration: config, type: typeof(string));""")]
91+
[InlineData("""var str = ConfigurationBinder.GetValue<string>(key: "key", configuration: config);""")]
92+
[InlineData("""var str = ConfigurationBinder.GetValue<string>(key: "key", defaultValue: "default", configuration: config);""")]
93+
[InlineData("""var str = ConfigurationBinder.GetValue<string>(configuration: config, key: "key", defaultValue: "default");""")]
94+
[InlineData("""var str = ConfigurationBinder.GetValue(defaultValue: "default", key: "key", configuration: config, type: typeof(string));""")]
95+
[InlineData("""var str = ConfigurationBinder.GetValue(defaultValue: "default", type: typeof(string), key: "key", configuration: config);""")]
96+
public async Task GetValue_NamedParametersOutOfOrder(string row)
97+
{
98+
string source = $$"""
99+
using System.Collections.Generic;
100+
using Microsoft.Extensions.Configuration;
101+
102+
public class Program
103+
{
104+
public static void Main()
105+
{
106+
ConfigurationBuilder configurationBuilder = new();
107+
IConfigurationRoot config = configurationBuilder.Build();
108+
{{row}}
109+
}
110+
111+
public class MyClass
112+
{
113+
public string MyString { get; set; }
114+
public int MyInt { get; set; }
115+
public List<int> MyList { get; set; }
116+
public Dictionary<string, string> MyDictionary { get; set; }
117+
}
118+
}
119+
""";
120+
121+
await VerifyThatSourceIsGenerated(source);
122+
}
123+
18124
[Fact]
19125
public async Task Bind_Instance()
20126
{

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Helpers.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,14 @@ private enum ExtensionClassType
8585
ServiceCollection,
8686
}
8787

88+
private static async Task VerifyThatSourceIsGenerated(string testSourceCode)
89+
{
90+
var (d, r) = await RunGenerator(testSourceCode);
91+
Assert.Equal(1, r.Length);
92+
Assert.Empty(d);
93+
Assert.True(r[0].SourceText.Lines.Count > 10);
94+
}
95+
8896
private static async Task VerifyAgainstBaselineUsingFile(
8997
string filename,
9098
string testSourceCode,

‎src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/SourceGenerationTests/ConfigurationExtensionsTest.Generator.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,5 +156,22 @@ public void TestBindingInvocationsWithNewlines_StaticCalls()
156156
)
157157
;
158158
}
159+
160+
[Fact]
161+
public void TestBindAndConfigureWithNamedParameters()
162+
{
163+
OptionsBuilder<FakeOptions>? optionsBuilder = CreateOptionsBuilder();
164+
IServiceCollection services = new ServiceCollection();
165+
166+
OptionsBuilderConfigurationExtensions.Bind(config: s_emptyConfig, optionsBuilder: optionsBuilder);
167+
OptionsBuilderConfigurationExtensions.Bind(configureBinder: _ => { }, config: s_emptyConfig, optionsBuilder: optionsBuilder);
168+
169+
OptionsBuilderConfigurationExtensions.BindConfiguration(configureBinder: _ => { }, configSectionPath: "path", optionsBuilder: optionsBuilder);
170+
171+
OptionsConfigurationServiceCollectionExtensions.Configure<FakeOptions>(config: s_emptyConfig, services: services);
172+
OptionsConfigurationServiceCollectionExtensions.Configure<FakeOptions>(name: "", config: s_emptyConfig, services: services);
173+
OptionsConfigurationServiceCollectionExtensions.Configure<FakeOptions>(configureBinder: _ => { }, config: s_emptyConfig, services: services);
174+
OptionsConfigurationServiceCollectionExtensions.Configure<FakeOptions>(name: "", configureBinder: _ => { }, config: s_emptyConfig, services: services);
175+
}
159176
}
160177
}

0 commit comments

Comments
 (0)