Skip to content

Commit f4f2f32

Browse files
Add OriginalName to InputOperation to prevent CollectionResult name collisions (#10021)
`CollectionResultDefinition.BuildName()` derives its type name from `Operation.Name`, which is mutable via `InputOperation.Update()`. When multiple operations get renamed to the same name (e.g., `List` and `ListAll` both becoming `GetAll` via `CleanOperationNames`), their CollectionResult types collide. `BuildName()` now detects collisions by checking if another paging operation in the same client would produce the same name, and only falls back to `OriginalName` when a collision is detected. When there is no collision, the current (cleaned) name is used as before. ### Changes - **`InputOperation`** — Added nullable `OriginalName` property (`string?`), never modified by `Update()`, with no default assignment - **`InputOperationConverter`** — Set `OriginalName` during deserialization (sole assignment point for production code) - **`ClientProvider`** — Exposed `_inputClient` as an `internal InputClient` property so `CollectionResultDefinition` can access the client's methods for collision detection - **`CollectionResultDefinition.BuildName()`** — Added `HasPagingOperationNameCollision()` check; uses `OriginalName` (with null fallback to `Operation.Name`) only when another paging operation in the same client would produce the same name - **`CollectionResultDefinitionTests`** — Added collision test (two ops `list`+`listAll` both renamed to `GetAll` produce unique names) and no-collision test (single op uses cleaned name) - **`InputFactory`** — Updated test factory to explicitly set `OriginalName` after construction <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>The generator should make sure CollectionResults types never collide names with each other</issue_title> > <issue_description>Mirror issue of Azure/azure-sdk-for-net#57063, but for the unbranded generator. </issue_description> > > <agent_instructions>Lets fix this by updating InputOperation to have a new public `OriginalName` string property which will contain the original name of the operation before any mutations are done on it. Then in `CollectionResultDefinition` we should simply use OriginalName inside the BuildName override when constructing the name. This should work since typespec doesn't allow duplicate operation names within a client.</agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #10020 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 Send tasks to Copilot coding agent from [Slack](https://gh.io/cca-slack-docs) and [Teams](https://gh.io/cca-teams-docs) to turn conversations into code. Copilot posts an update in your thread when it's finished. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jorgerangel-msft <[email protected]>
1 parent a22a859 commit f4f2f32

File tree

6 files changed

+113
-2
lines changed

6 files changed

+113
-2
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ private record ApiVersionFields(FieldProvider Field, PropertyProvider? Correspon
4242
private const string ClientSuffix = "Client";
4343
private readonly FormattableString _publicCtorDescription;
4444
private readonly InputClient _inputClient;
45+
internal InputClient InputClient => _inputClient;
4546
private readonly InputAuth? _inputAuth;
4647
private readonly ParameterProvider _endpointParameter;
4748
/// <summary>

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,34 @@ private PropertyProvider FindPropertyInModelHierarchy(TypeProvider model, string
182182
protected override string BuildNamespace() => Client.Type.Namespace;
183183

184184
protected override string BuildName()
185-
=> $"{Client.Type.Name}{Operation.Name.ToIdentifierName()}{(IsAsync ? "Async" : "")}CollectionResult{(ItemModelType == null ? "" : "OfT")}";
185+
{
186+
var operationName = Operation.Name.ToIdentifierName();
187+
// Check if there is another paging operation in the same client whose name would produce a collision.
188+
// If so, use the OriginalName to differentiate.
189+
if (HasPagingOperationNameCollision(operationName))
190+
{
191+
operationName = (Operation.OriginalName ?? Operation.Name).ToIdentifierName();
192+
}
193+
return $"{Client.Type.Name}{operationName}{(IsAsync ? "Async" : "")}CollectionResult{(ItemModelType == null ? "" : "OfT")}";
194+
}
195+
196+
private bool HasPagingOperationNameCollision(string operationName)
197+
{
198+
var pagingMethods = Client.InputClient.Methods.OfType<InputPagingServiceMethod>();
199+
int count = 0;
200+
foreach (var method in pagingMethods)
201+
{
202+
if (method.Operation.Name.ToIdentifierName() == operationName)
203+
{
204+
count++;
205+
if (count > 1)
206+
{
207+
return true;
208+
}
209+
}
210+
}
211+
return false;
212+
}
186213

187214
protected override TypeSignatureModifiers BuildDeclarationModifiers()
188215
=> TypeSignatureModifiers.Internal | TypeSignatureModifiers.Partial | TypeSignatureModifiers.Class;

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

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,81 @@ public void TestEmptyStringHandlingForUriNextLink()
205205
"Generated code should check for null URI");
206206
}
207207

208+
[Test]
209+
public void TestCollectionResultNamesDoNotCollideWhenOperationsAreRenamed()
210+
{
211+
// Two paging operations "list" and "listAll" both get renamed to "GetAll" by CleanOperationNames.
212+
// The CollectionResult names should use OriginalName to avoid collision.
213+
var thingModel = InputFactory.Model("thing", properties:
214+
[
215+
InputFactory.Property("name", InputPrimitiveType.String, isRequired: true),
216+
]);
217+
var thingsProperty = InputFactory.Property("things", InputFactory.Array(thingModel));
218+
var nextProperty = InputFactory.Property("next", InputPrimitiveType.Url);
219+
var pageModel = InputFactory.Model("page", properties: [thingsProperty, nextProperty]);
220+
var response = InputFactory.OperationResponse([200], pageModel);
221+
222+
var pagingMetadata = InputFactory.NextLinkPagingMetadata(["things"], ["next"], InputResponseLocation.Body);
223+
224+
// "list" will be renamed to "GetAll", "listAll" will also be renamed to "GetAll"
225+
var listOperation = InputFactory.Operation("list", responses: [response]);
226+
var listAllOperation = InputFactory.Operation("listAll", responses: [response]);
227+
228+
var listServiceMethod = InputFactory.PagingServiceMethod("list", listOperation, pagingMetadata: pagingMetadata);
229+
var listAllServiceMethod = InputFactory.PagingServiceMethod("listAll", listAllOperation, pagingMetadata: pagingMetadata);
230+
231+
var client = InputFactory.Client("FooClient", methods: [listServiceMethod, listAllServiceMethod]);
232+
233+
MockHelpers.LoadMockGenerator(inputModels: () => [thingModel], clients: () => [client]);
234+
235+
var collectionResults = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders
236+
.Where(t => t is CollectionResultDefinition)
237+
.ToList();
238+
239+
// Should have 8 CollectionResult types (2 ops × 2 sync/async × 2 typed/untyped) and they should all have unique names
240+
Assert.AreEqual(8, collectionResults.Count,
241+
$"Expected 8 CollectionResult types but found {collectionResults.Count}");
242+
var collectionResultNames = collectionResults.Select(t => t.Name).ToList();
243+
Assert.AreEqual(collectionResultNames.Distinct().Count(), collectionResultNames.Count,
244+
$"CollectionResult names should be unique but found duplicates: {string.Join(", ", collectionResultNames)}");
245+
246+
// Both should use the original names for disambiguation
247+
Assert.IsTrue(collectionResultNames.Any(n => n == "FooClientListCollectionResult"),
248+
$"Expected 'FooClientListCollectionResult' in [{string.Join(", ", collectionResultNames)}]");
249+
Assert.IsTrue(collectionResultNames.Any(n => n == "FooClientListAllCollectionResult"),
250+
$"Expected 'FooClientListAllCollectionResult' in [{string.Join(", ", collectionResultNames)}]");
251+
}
252+
253+
[Test]
254+
public void TestCollectionResultNameUsesCurrentNameWhenNoCollision()
255+
{
256+
// A single paging operation should use the current (cleaned) name, not the original name.
257+
var thingModel = InputFactory.Model("thing", properties:
258+
[
259+
InputFactory.Property("name", InputPrimitiveType.String, isRequired: true),
260+
]);
261+
var thingsProperty = InputFactory.Property("things", InputFactory.Array(thingModel));
262+
var nextProperty = InputFactory.Property("next", InputPrimitiveType.Url);
263+
var pageModel = InputFactory.Model("page", properties: [thingsProperty, nextProperty]);
264+
var response = InputFactory.OperationResponse([200], pageModel);
265+
266+
var pagingMetadata = InputFactory.NextLinkPagingMetadata(["things"], ["next"], InputResponseLocation.Body);
267+
268+
// "listAll" gets renamed to "GetAll" by CleanOperationNames, no collision
269+
var listAllOperation = InputFactory.Operation("listAll", responses: [response]);
270+
var listAllServiceMethod = InputFactory.PagingServiceMethod("listAll", listAllOperation, pagingMetadata: pagingMetadata);
271+
272+
var client = InputFactory.Client("FooClient", methods: [listAllServiceMethod]);
273+
274+
MockHelpers.LoadMockGenerator(inputModels: () => [thingModel], clients: () => [client]);
275+
276+
// When there's no collision, the cleaned name "GetAll" should be used
277+
var collectionResultDefinition = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders.FirstOrDefault(
278+
t => t is CollectionResultDefinition && t.Name == "FooClientGetAllCollectionResult") as CollectionResultDefinition;
279+
Assert.IsNotNull(collectionResultDefinition,
280+
"CollectionResult should use cleaned name 'GetAll' when there's no collision");
281+
}
282+
208283
internal static void CreatePagingOperation(InputResponseLocation responseLocation, bool isNested = false)
209284
{
210285
var inputModel = InputFactory.Model("cat", properties:

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.Input/src/InputTypes/InputOperation.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,11 @@ public InputOperation() : this(
7373
{ }
7474

7575
public string Name { get; internal set; }
76+
77+
/// <summary>
78+
/// Gets the original name of the operation as defined in the TypeSpec before any mutations.
79+
/// </summary>
80+
public string? OriginalName { get; internal set; }
7681
public string? ResourceName { get; internal set; }
7782
public string? Summary { get; internal set; }
7883
public string? Doc { get; internal set; }

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.Input/src/InputTypes/Serialization/InputOperationConverter.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ public override void Write(Utf8JsonWriter writer, InputOperation value, JsonSeri
8787
}
8888

8989
operation.Name = name ?? throw new JsonException("InputOperation must have name");
90+
operation.OriginalName = name;
9091
operation.ResourceName = resourceName;
9192
operation.Summary = summary;
9293
operation.Doc = doc;

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/InputFactory.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ public static InputOperation Operation(
639639
bool generateConvenienceMethod = true,
640640
string? ns = null)
641641
{
642-
return new InputOperation(
642+
var operation = new InputOperation(
643643
name,
644644
null,
645645
"",
@@ -658,6 +658,8 @@ public static InputOperation Operation(
658658
generateConvenienceMethod,
659659
name,
660660
ns);
661+
operation.OriginalName = name;
662+
return operation;
661663
}
662664

663665
public static InputPagingServiceMetadata NextLinkPagingMetadata(

0 commit comments

Comments
 (0)