diff --git a/.chronus/changes/fix_naming_issue-2024-6-12-17-6-31.md b/.chronus/changes/fix_naming_issue-2024-6-12-17-6-31.md new file mode 100644 index 0000000000..9f1aa23e3b --- /dev/null +++ b/.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 \ No newline at end of file diff --git a/packages/typespec-client-generator-core/src/public-utils.ts b/packages/typespec-client-generator-core/src/public-utils.ts index b5b8dd902b..130c8099ef 100644 --- a/packages/typespec-client-generator-core/src/public-utils.ts +++ b/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 []; diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 95e738f182..d18c57c2ce 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/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)), }); } diff --git a/packages/typespec-client-generator-core/test/public-utils.test.ts b/packages/typespec-client-generator-core/test/public-utils.test.ts index be398f4a06..92af96a4fa 100644 --- a/packages/typespec-client-generator-core/test/public-utils.test.ts +++ b/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", () => {