Skip to content

Fix crash in ClientProvider.BuildMethods when children list contains duplicates #10355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 6 commits into
base: main
Choose a base branch
from
+94 −3
Draft

Fix crash in ClientProvider.BuildMethods when children list contains duplicates #10355

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
82780a6
Initial plan
Copilot Apr 13, 2026
f7d04d9
fix: deduplicate sub-clients in GetSubClients to prevent ToDictionary…
Copilot Apr 13, 2026
3b6159d
refactor: use HashSet for O(1) dedup lookup in GetSubClients
Copilot Apr 13, 2026
4cd0636
fix: deduplicate children in emitter client-converter instead of gene…
Copilot Apr 13, 2026
07b25f5
refactor: use loop with Set for dedup and add TODO for upstream fix
Copilot Apr 13, 2026
10ef6a7
test: add additional nested interface to TestClientNs in dedup test
Copilot Apr 13, 2026
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
3 changes: 2 additions & 1 deletion ...csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1501,12 +1501,13 @@ private ParameterProvider BuildClientEndpointParameter()

private IReadOnlyList<ClientProvider> GetSubClients()
{
var seen = new HashSet<ClientProvider>();
Copy link
Copy Markdown
Contributor

@jorgerangel-msft jorgerangel-msft Apr 13, 2026

Choose a reason for hiding this comment

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

@copilot instead of having this change in the generator, can we de-dup the children in the emitter so the generated code model has no duplicate entries?

Copy link
Copy Markdown
Contributor Author

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

Moved the deduplication from the C# generator to the emitter in 4cd0636. The client-converter.ts now deduplicates children when populating the code model, and the ClientProvider.cs/ClientProviderTests.cs changes have been reverted.

var subClients = new List<ClientProvider>(_inputClient.Children.Count);

foreach (var client in _inputClient.Children)
{
var subClient = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(client);
if (subClient != null)
if (subClient != null && seen.Add(subClient))
{
subClients.Add(subClient);
}
Expand Down
32 changes: 32 additions & 0 deletions ...soft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,38 @@ public void TestBuildFields_ForParent_InitializedByParentOnly_WithSubClientParam
Assert.IsNull(cachingField, "Parent should not have caching field for subclient that has subclient-specific parameters in its accessor");
}

[Test]
public void TestBuildMethods_ForParent_DuplicateChildrenDoesNotCrash()
{
var parentClient = InputFactory.Client("ParentClient");
var subClient = InputFactory.Client(
"SubClient",
parent: parentClient,
initializedBy: InputClientInitializedBy.Parent);

// Simulate duplicate children (same InputClient appearing multiple times in children list)
parentClient.Update(children: [subClient, subClient]);

MockHelpers.LoadMockGenerator(
clients: () => [parentClient]);

var parentProvider = new ClientProvider(parentClient);

Assert.IsNotNull(parentProvider);

// Accessing Methods should not throw due to duplicate children
Assert.DoesNotThrow(() => { var _ = parentProvider.Methods; });

// The parent should have only one factory method for the subclient (not duplicated)
var factoryMethods = parentProvider.Methods.Where(
m => m.Signature?.Name == "GetSubClient" || m.Signature?.Name == "GetSubClientClient").ToList();
Assert.AreEqual(1, factoryMethods.Count, "Parent should have exactly one factory method for the subclient despite duplicate children");

// The parent should have only one caching field (not duplicated)
var cachingFields = parentProvider.Fields.Where(f => f.Name == "_cachedSubClient").ToList();
Assert.AreEqual(1, cachingFields.Count, "Parent should have exactly one caching field despite duplicate children");
}

private void ValidatePrimaryConstructor(
ConstructorProvider primaryPublicConstructor,
List<InputParameter> inputParameters,
Expand Down