[tcgc] findContextPath need to handle nested operation group, also …

…refine the logic for naming and composing cross language definition id (#1157)

fix: #1150
cc: @ArcturusZhang  @archerzz

.chronus/changes/fix_naming_issue-2024-6-12-17-6-31.md

@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@azure-tools/typespec-client-generator-core"
+---
+
+findContextPath need to handle nested operation group, also refine the logic for naming and composing cross language definition id

packages/typespec-client-generator-core/src/public-utils.ts

@@ -215,6 +215,7 @@ export function getWireName(context: TCGCContext, type: Type & { name: string })
 export function getCrossLanguageDefinitionId(
   context: TCGCContext,
   type: Union | Model | Enum | Scalar | ModelProperty | Operation | Namespace | Interface,
+  operation?: Operation,
   appendNamespace: boolean = true
 ): string {
   let retval = type.name || "anonymous";
@@ -226,7 +227,9 @@ export function getCrossLanguageDefinitionId(
       if (type.name) {
         break;
       }
-      const contextPath = findContextPath(context, type);
+      const contextPath = operation
+        ? getContextPath(context, operation, type)
+        : findContextPath(context, type);
       retval =
         contextPath
           .slice(findLastNonAnonymousModelNode(contextPath))
@@ -241,12 +244,12 @@ export function getCrossLanguageDefinitionId(
       break;
     case "ModelProperty":
       if (type.model) {
-        retval = `${getCrossLanguageDefinitionId(context, type.model, false)}.${retval}`;
+        retval = `${getCrossLanguageDefinitionId(context, type.model, undefined, false)}.${retval}`;
       }
       break;
     case "Operation":
       if (type.interface) {
-        retval = `${getCrossLanguageDefinitionId(context, type.interface, false)}.${retval}`;
+        retval = `${getCrossLanguageDefinitionId(context, type.interface, undefined, false)}.${retval}`;
       }
       break;
   }
@@ -331,13 +334,18 @@ function findContextPath(
         return result;
       }
     }
-    for (const operationGroup of listOperationGroups(context, client)) {
-      for (const operation of listOperationsInOperationGroup(context, operationGroup)) {
+    const ogs = listOperationGroups(context, client);
+    while (ogs.length) {
+      const operationGroup = ogs.pop();
+      for (const operation of listOperationsInOperationGroup(context, operationGroup!)) {
         const result = getContextPath(context, operation, type);
         if (result.length > 0) {
           return result;
         }
       }
+      if (operationGroup?.subOperationGroups) {
+        ogs.push(...operationGroup.subOperationGroups);
+      }
     }
   }
   return [];

packages/typespec-client-generator-core/src/types.ts

@@ -306,7 +306,7 @@ export function getSdkArrayOrDictWithDiagnostics(
           ...diagnostics.pipe(getSdkTypeBaseHelper(context, type, "array")),
           name: getLibraryName(context, type),
           valueType: valueType,
-          crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type),
+          crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type, operation),
         });
       }
     }
@@ -377,12 +377,12 @@ export function getSdkUnionWithDiagnostics(
   if (retval === undefined) {
     retval = {
       ...diagnostics.pipe(getSdkTypeBaseHelper(context, type, "union")),
-      name: getLibraryName(context, type) || getGeneratedName(context, type),
+      name: getLibraryName(context, type) || getGeneratedName(context, type, operation),
       isGeneratedName: !type.name,
       values: nonNullOptions.map((x) =>
         diagnostics.pipe(getClientTypeWithDiagnostics(context, x, operation))
       ),
-      crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type),
+      crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type, operation),
     };
   }
 
@@ -556,8 +556,7 @@ export function getSdkModelWithDiagnostics(
     updateModelsMap(context, type, sdkType, operation);
   } else {
     const docWrapper = getDocHelper(context, type);
-    const generatedName = getGeneratedName(context, type);
-    const name = getLibraryName(context, type) || generatedName;
+    const name = getLibraryName(context, type) || getGeneratedName(context, type, operation);
     const usage = isErrorModel(context.program, type) ? UsageFlags.Error : UsageFlags.None; // initial usage we can tell just by looking at the model
     sdkType = {
       ...diagnostics.pipe(getSdkTypeBaseHelper(context, type, "model")),
@@ -569,7 +568,7 @@ export function getSdkModelWithDiagnostics(
       additionalProperties: undefined, // going to set additional properties in the next few lines when we look at base model
       access: "public",
       usage,
-      crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type),
+      crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type, operation),
       apiVersions: getAvailableApiVersions(context, type, type.namespace),
       isFormDataType: isMultipartFormData(context, type, operation),
     };
@@ -714,7 +713,7 @@ function getSdkEnumWithDiagnostics(
       isFlags: false,
       usage: UsageFlags.None, // We will add usage as we loop through the operations
       access: "public", // Dummy value until we update models map
-      crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type),
+      crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type, operation),
       apiVersions: getAvailableApiVersions(context, type, type.namespace),
       isUnionAsEnum: false,
     };
@@ -765,8 +764,7 @@ function getSdkUnionEnumWithDiagnostics(
   let sdkType = context.modelsMap?.get(union) as SdkEnumType | undefined;
   if (!sdkType) {
     const docWrapper = getDocHelper(context, union);
-    const generatedName = getGeneratedName(context, union);
-    const name = getLibraryName(context, type.union) || generatedName;
+    const name = getLibraryName(context, type.union) || getGeneratedName(context, union, operation);
     sdkType = {
       ...diagnostics.pipe(getSdkTypeBaseHelper(context, type.union, "enum")),
       name,
@@ -781,7 +779,7 @@ function getSdkUnionEnumWithDiagnostics(
       isFlags: false,
       usage: UsageFlags.None, // We will add usage as we loop through the operations
       access: "public", // Dummy value until we update models map
-      crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, union),
+      crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, union, operation),
       apiVersions: getAvailableApiVersions(context, type.union, type.union.namespace),
       isUnionAsEnum: true,
     };
@@ -820,7 +818,7 @@ function getKnownValuesEnum(
         isFlags: false,
         usage: UsageFlags.None, // We will add usage as we loop through the operations
         access: "public", // Dummy value until we update models map
-        crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type),
+        crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type, operation),
         apiVersions: getAvailableApiVersions(context, type, type.namespace),
         isUnionAsEnum: false,
       };
@@ -1057,7 +1055,7 @@ export function getSdkModelPropertyTypeBase(
       type,
       operation ? getLocationOfOperation(operation) : undefined
     ),
-    crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type),
+    crossLanguageDefinitionId: getCrossLanguageDefinitionId(context, type, operation),
     decorators: diagnostics.pipe(getTypeDecorators(context, type)),
   });
 }

packages/typespec-client-generator-core/test/public-utils.test.ts

@@ -17,6 +17,7 @@ import {
   getClientNamespaceString,
   getCrossLanguageDefinitionId,
   getDefaultApiVersion,
+  getGeneratedName,
   getLibraryName,
   getPropertyNames,
   isApiVersion,
@@ -1641,11 +1642,11 @@ describe("typespec-client-generator-core: public-utils", () => {
         strictEqual(repeatabilityResult.type.kind, "Union");
         const unionEnum = getSdkUnion(runner.context, repeatabilityResult.type);
         strictEqual(unionEnum.kind, "enum");
-        strictEqual(unionEnum.name, "RequestParameterWithAnonymousUnionRepeatabilityResult");
+        strictEqual(unionEnum.name, "TestRequestRepeatabilityResult");
         // not a defined type in tsp, so no crossLanguageDefinitionId
         strictEqual(
           unionEnum.crossLanguageDefinitionId,
-          "RequestParameterWithAnonymousUnion.repeatabilityResult.anonymous"
+          "test.RequestRepeatabilityResult.anonymous"
         );
         ok(unionEnum.isGeneratedName);
       });
@@ -1675,13 +1676,37 @@ describe("typespec-client-generator-core: public-utils", () => {
         strictEqual(stringType.values[1].kind, "enumvalue");
         strictEqual(stringType.values[1].value, "rejected");
         strictEqual(stringType.valueType.kind, "string");
-        strictEqual(stringType.name, "RequestParameterWithAnonymousUnionRepeatabilityResult");
+        strictEqual(stringType.name, "TestRequestRepeatabilityResult");
         strictEqual(stringType.isGeneratedName, true);
         strictEqual(
           stringType.crossLanguageDefinitionId,
-          "RequestParameterWithAnonymousUnion.repeatabilityResult.anonymous"
+          "test.RequestRepeatabilityResult.anonymous"
         );
       });
+
+      it("anonymous model naming in multi layer operation group", async () => {
+        const { TestModel } = (await runner.compile(`
+        @service({})
+        namespace MyService {
+          namespace Test {
+            namespace InnerTest {
+              @test
+              model TestModel {
+                anonymousProp: {prop: string}
+              }
+              op test(): TestModel;
+            }
+          }
+        }
+        `)) as { TestModel: Model };
+
+        runner.context.generatedNames?.clear();
+        const name = getGeneratedName(
+          runner.context,
+          [...TestModel.properties.values()][0].type as Model
+        );
+        strictEqual(name, "TestModelAnonymousProp");
+      });
     });
 
     describe("getLroMetadata", () => {