Skip to content

Commit ce6ef84

Browse files
fix(http-client-csharp): exclude custom constructors with settings parameter from property discovery (#10260)
## Problem When building settings properties from custom constructors, we consider all public constructors on the client. If a custom constructor takes the client's settings type as a parameter, it doesn't make sense to consider that constructor — the settings type is the thing we're building, so its constructor parameters aren't meaningful inputs for configuration binding. ## Fix Skip custom constructors that have a parameter whose type matches \ClientProvider.ClientSettings.Type\ when discovering parameters for settings properties, \BindCore\ bindings, and \ConfigurationSchema.json\ generation. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d6cbe8d commit ce6ef84

3 files changed

Lines changed: 49 additions & 3 deletions

File tree

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/ConfigurationSchemaGenerator.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ private static JsonObject BuildClientEntry(ClientProvider client, string options
127127
knownProps.Add("Options");
128128
foreach (var ctor in customConstructors)
129129
{
130-
if (!ctor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public))
130+
if (!ctor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) ||
131+
settings.HasSettingsParameter(ctor))
131132
{
132133
continue;
133134
}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientSettingsProvider.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,8 @@ protected override PropertyProvider[] BuildProperties()
117117
knownProps.Add("Options");
118118
foreach (var ctor in customConstructors)
119119
{
120-
if (!ctor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public))
120+
if (!ctor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) ||
121+
HasSettingsParameter(ctor))
121122
{
122123
continue;
123124
}
@@ -190,7 +191,8 @@ protected override MethodProvider[] BuildMethods()
190191
knownProps.Add("Options");
191192
foreach (var ctor in customConstructors)
192193
{
193-
if (!ctor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public))
194+
if (!ctor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) ||
195+
HasSettingsParameter(ctor))
194196
{
195197
continue;
196198
}
@@ -498,5 +500,14 @@ internal static bool IsStandardParameterType(CSharpType type)
498500

499501
return false;
500502
}
503+
504+
/// <summary>
505+
/// Returns true if the constructor has a parameter whose type matches this client's
506+
/// settings type.
507+
/// </summary>
508+
internal bool HasSettingsParameter(ConstructorProvider ctor)
509+
{
510+
return ctor.Signature.Parameters.Any(p => p.Type.Equals(Type));
511+
}
501512
}
502513
}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientSettingsProviderTests.cs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,5 +964,39 @@ await MockHelpers.LoadMockGeneratorAsync(
964964
Assert.IsTrue(bodyString.Contains("ConnectionString"),
965965
"BindCore should bind the custom constructor parameter 'ConnectionString' from configuration");
966966
}
967+
968+
[Test]
969+
public void TestSettingsType_DoesNotContainSelfReferentialSettingsProperty()
970+
{
971+
var client = InputFactory.Client("TestClient");
972+
var clientProvider = new ClientProvider(client);
973+
var settingsProvider = clientProvider.ClientSettings;
974+
975+
Assert.IsNotNull(settingsProvider);
976+
977+
// The settings type should not have a property named "Settings" that references itself.
978+
// This would happen if the generated settings constructor (which takes the settings type
979+
// as a parameter) is incorrectly treated as a custom constructor.
980+
var settingsProperty = settingsProvider!.Properties.FirstOrDefault(p => p.Name == "Settings");
981+
Assert.IsNull(settingsProperty,
982+
"Settings type should not contain a self-referential 'Settings' property");
983+
}
984+
985+
[Test]
986+
public void TestBindCoreMethod_DoesNotBindSettingsParameter()
987+
{
988+
var client = InputFactory.Client("TestClient");
989+
var clientProvider = new ClientProvider(client);
990+
var settingsProvider = clientProvider.ClientSettings;
991+
992+
Assert.IsNotNull(settingsProvider);
993+
994+
var bindCoreMethod = settingsProvider!.Methods.FirstOrDefault(m => m.Signature.Name == "BindCore");
995+
Assert.IsNotNull(bindCoreMethod);
996+
997+
var bodyString = bindCoreMethod!.BodyStatements!.ToDisplayString();
998+
Assert.IsFalse(bodyString.Contains("GetSection(\"Settings\")"),
999+
"BindCore should not bind a self-referential Settings section");
1000+
}
9671001
}
9681002
}

0 commit comments

Comments
 (0)