TCGC common layer refine

.chronus/changes/fix_tcgc_type_issue-2024-2-26-17-43-28.md

@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@azure-tools/typespec-client-generator-core"
+---
+
+fix wrong union `generatedName` flag and refine templated model naming

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

@@ -20,6 +20,7 @@ import {
   isPathParam,
   isQueryParam,
 } from "@typespec/http";
+import { camelCase } from "change-case";
 import {
   CollectionFormat,
   SdkBodyParameter,
@@ -123,20 +124,22 @@ function getSdkHttpParameters(
       if (getParamResponse.kind !== "body") throw new Error("blah");
       retval.bodyParam = getParamResponse;
     } else {
+      const type = diagnostics.pipe(
+        getClientTypeWithDiagnostics(context, tspBody.type, httpOperation.operation)
+      );
+      const name = camelCase((type as { name: string }).name ?? "body");
       retval.bodyParam = {
         kind: "body",
-        name: "body",
-        nameInClient: "body",
+        name,
+        nameInClient: name,
         description: getDocHelper(context, tspBody.type).description,
         details: getDocHelper(context, tspBody.type).details,
         onClient: false,
         contentTypes: [],
         defaultContentType: "application/json", // actual content type info is added later
         isApiVersionParam: false,
         apiVersions: getAvailableApiVersions(context, tspBody.type),
-        type: diagnostics.pipe(
-          getClientTypeWithDiagnostics(context, tspBody.type, httpOperation.operation)
-        ),
+        type,
         optional: false,
         nullable: isNullable(tspBody.type),
         correspondingMethodParams,
@@ -411,10 +414,7 @@ export function getCorrespondingMethodParams(
     const correspondingProperties = methodParameters.filter((x) =>
       paramInProperties(x, serviceParamType)
     );
-    const bodyPropertyNames = serviceParamType.properties.filter((x) =>
-      paramInProperties(x, serviceParamType)
-    );
-    if (correspondingProperties.length !== bodyPropertyNames.length) {
+    if (correspondingProperties.length !== serviceParamType.properties.length) {
       throw new Error("Can't find corresponding properties for spread body parameter");
     }
     return correspondingProperties;

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

@@ -256,7 +256,7 @@ export interface SdkEnumValueType extends SdkTypeBase {
   name: string;
   value: string | number;
   enumType: SdkEnumType;
-  valueType: SdkType;
+  valueType: SdkBuiltInType;
   description?: string;
   details?: string;
 }

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

@@ -1,6 +1,7 @@
 import { getLroMetadata, getPagedResult } from "@azure-tools/typespec-azure-core";
 import {
   Diagnostic,
+  Model,
   ModelProperty,
   Operation,
   createDiagnosticCollector,
@@ -9,6 +10,7 @@ import {
 } from "@typespec/compiler";
 import { getServers } from "@typespec/http";
 import { resolveVersions } from "@typespec/versioning";
+import { camelCase } from "change-case";
 import {
   getAccess,
   listClients,
@@ -218,9 +220,24 @@ function getSdkBasicServiceMethod<
 ): [SdkServiceMethod<TServiceOperation>, readonly Diagnostic[]] {
   const diagnostics = createDiagnosticCollector();
   // when we spread, all of the inputtable properties of our model get flattened onto the method
-  const methodParameters = Array.from(operation.parameters.properties.values())
-    .map((x) => diagnostics.pipe(getSdkMethodParameter(context, x)))
-    .filter((x): x is SdkMethodParameter => x.kind === "method");
+  const methodParameters: SdkMethodParameter[] = [];
+  const spreadModelNames: string[] = [];
+  for (const prop of operation.parameters.properties.values()) {
+    if (prop.sourceProperty?.model?.name) {
+      if (!spreadModelNames.includes(prop.sourceProperty.model.name)) {
+        spreadModelNames.push(prop.sourceProperty.model.name);
+        methodParameters.push(
+          diagnostics.pipe(getSdkMethodParameter(context, prop.sourceProperty.model))
+        );
+      }
+    } else {
+      const methodParameter = diagnostics.pipe(getSdkMethodParameter(context, prop));
+      if (methodParameter.kind === "method") {
+        methodParameters.push(methodParameter);
+      }
+    }
+  }
+
   // if there's an api version parameter, we want to bubble it up to the client
   // we don't want it on the method level, but we will keep it on the service operation level
   const apiVersionParam = methodParameters.find((x) => x.isApiVersionParam);
@@ -331,9 +348,28 @@ function getSdkInitializationType<
 
 function getSdkMethodParameter(
   context: TCGCContext,
-  type: ModelProperty
+  type: ModelProperty | Model
 ): [SdkMethodParameter, readonly Diagnostic[]] {
   const diagnostics = createDiagnosticCollector();
+  if (type.kind === "Model") {
+    const name = camelCase(getLibraryName(context, type) ?? "body");
+    const propertyType = diagnostics.pipe(getClientTypeWithDiagnostics(context, type));
+    return diagnostics.wrap({
+      kind: "method",
+      description: getDocHelper(context, type).description,
+      details: getDocHelper(context, type).details,
+      apiVersions: getAvailableApiVersions(context, type),
+      type: propertyType,
+      nameInClient: name,
+      name,
+      optional: false,
+      nullable: false,
+      discriminator: false,
+      serializedName: name,
+      onClient: false,
+      isApiVersionParam: false,
+    });
+  }
   return diagnostics.wrap({
     ...diagnostics.pipe(getSdkModelPropertyType(context, type)),
     kind: "method",

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

@@ -174,7 +174,9 @@ export function getLibraryName(
 
   // 5. if type is derived from template and name is the same as template, add template parameters' name as suffix
   if (typeof type.name === "string" && type.kind === "Model" && type.templateMapper?.args) {
-    return type.name + type.templateMapper.args.map((arg) => (arg as Model).name).join("");
+    return (
+      type.name + type.templateMapper.args.map((arg) => pascalCase((arg as Model).name)).join("")
+    );
   }
 
   return typeof type.name === "string" ? type.name : "";

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

@@ -895,7 +895,7 @@ function getSdkCredentialType(
       values: credentialTypes,
       nullable: false,
       name: createGeneratedName(client.service, "CredentialUnion"),
-      generatedName: false,
+      generatedName: true,
     };
   }
   return credentialTypes[0];

packages/typespec-client-generator-core/test/package.test.ts

@@ -223,6 +223,8 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(credentialParam.onClient, true);
       strictEqual(credentialParam.optional, false);
       strictEqual(credentialParam.type.kind, "union");
+      strictEqual(credentialParam.type.name, "ServiceCredentialUnion");
+      strictEqual(credentialParam.type.generatedName, true);
       strictEqual(credentialParam.type.values.length, 2);
       const schemes = credentialParam.type.values
         .filter((v): v is SdkCredentialType => v.kind === "credential")
@@ -995,11 +997,11 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(method.parameters.length, 1);
       const pathMethod = method.parameters[0];
       strictEqual(pathMethod.kind, "method");
-      strictEqual(pathMethod.name, "name");
+      strictEqual(pathMethod.name, "nameParameter");
       strictEqual(pathMethod.optional, false);
       strictEqual(pathMethod.onClient, false);
       strictEqual(pathMethod.isApiVersionParam, false);
-      strictEqual(pathMethod.type.kind, "string");
+      strictEqual(pathMethod.type.kind, "model");
       strictEqual(pathMethod.nullable, false);
 
       const serviceOperation = method.operation;
@@ -1016,7 +1018,7 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(pathParam.urlEncode, true);
       strictEqual(pathParam.nullable, false);
       strictEqual(pathParam.correspondingMethodParams.length, 1);
-      deepStrictEqual(pathParam.correspondingMethodParams[0], pathMethod);
+      deepStrictEqual(pathParam.correspondingMethodParams[0], pathMethod.type.properties[0]);
     });
 
     it("header basic", async () => {
@@ -1357,13 +1359,13 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(method.kind, "basic");
       strictEqual(method.parameters.length, 2);
 
-      const methodParam = method.parameters.find((x) => x.name === "key");
+      const methodParam = method.parameters.find((x) => x.name === "input");
       ok(methodParam);
       strictEqual(methodParam.kind, "method");
       strictEqual(methodParam.optional, false);
       strictEqual(methodParam.onClient, false);
       strictEqual(methodParam.isApiVersionParam, false);
-      strictEqual(methodParam.type.kind, "string");
+      strictEqual(methodParam.type.kind, "model");
 
       const contentTypeParam = method.parameters.find((x) => x.name === "contentType");
       ok(contentTypeParam);
@@ -1384,11 +1386,7 @@ describe("typespec-client-generator-core: package", () => {
 
       const correspondingMethodParams = bodyParameter.correspondingMethodParams;
       strictEqual(correspondingMethodParams.length, 1);
-      strictEqual(
-        bodyParameter.type.properties[0].nameInClient, //eslint-disable-line deprecation/deprecation
-        correspondingMethodParams[0].nameInClient //eslint-disable-line deprecation/deprecation
-      );
-      strictEqual(bodyParameter.type.properties[0].name, correspondingMethodParams[0].name);
+      strictEqual(bodyParameter.type, correspondingMethodParams[0].type);
     });
 
     it("body alias spread", async () => {
@@ -1446,6 +1444,68 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(bodyParameter.type.properties[0].name, correspondingMethodParams[0].name);
     });
 
+    it("spread with discriminate type", async () => {
+      await runner.compile(`@server("http://localhost:3000", "endpoint")
+        @service({})
+        namespace My.Service;
+
+        @discriminator("kind")
+        model Pet {
+          name?: string;
+        }
+
+        model Dog {
+          kind: "dog";
+        }
+
+        model Cat {
+          kind: "cat";
+        }
+
+        op test(...Pet): void;
+        `);
+      const sdkPackage = runner.context.experimental_sdkPackage;
+      const method = getServiceMethodOfClient(sdkPackage);
+      strictEqual(sdkPackage.models.length, 1);
+      strictEqual(method.name, "test");
+      strictEqual(method.kind, "basic");
+      strictEqual(method.parameters.length, 2);
+
+      const methodParam = method.parameters.find((x) => x.name === "pet");
+      ok(methodParam);
+      strictEqual(methodParam.kind, "method");
+      strictEqual(methodParam.optional, false);
+      strictEqual(methodParam.onClient, false);
+      strictEqual(methodParam.isApiVersionParam, false);
+      strictEqual(methodParam.type.kind, "model");
+
+      const contentTypeMethodParam = method.parameters.find((x) => x.name === "contentType");
+      ok(contentTypeMethodParam);
+      strictEqual(contentTypeMethodParam.clientDefaultValue, undefined);
+      strictEqual(contentTypeMethodParam.type.kind, "constant");
+
+      const serviceOperation = method.operation;
+      const bodyParameter = serviceOperation.bodyParam;
+      ok(bodyParameter);
+      strictEqual(bodyParameter.kind, "body");
+      deepStrictEqual(bodyParameter.contentTypes, ["application/json"]);
+      strictEqual(bodyParameter.defaultContentType, "application/json");
+      strictEqual(bodyParameter.onClient, false);
+      strictEqual(bodyParameter.optional, false);
+      strictEqual(bodyParameter.type.kind, "model");
+      strictEqual(bodyParameter.type.properties.length, 2);
+      //eslint-disable-next-line deprecation/deprecation
+      strictEqual(bodyParameter.type.properties[0].nameInClient, "kind");
+      strictEqual(bodyParameter.type.properties[0].name, "kind");
+      //eslint-disable-next-line deprecation/deprecation
+      strictEqual(bodyParameter.type.properties[1].nameInClient, "name");
+      strictEqual(bodyParameter.type.properties[1].name, "name");
+
+      const correspondingMethodParams = bodyParameter.correspondingMethodParams;
+      strictEqual(correspondingMethodParams.length, 1);
+      strictEqual(bodyParameter.type, correspondingMethodParams[0].type);
+    });
+
     it("parameter grouping", async () => {
       await runner.compile(`@server("http://localhost:3000", "endpoint")
         @service({})
@@ -1993,18 +2053,18 @@ describe("typespec-client-generator-core: package", () => {
       const method = getServiceMethodOfClient(sdkPackage);
       strictEqual(method.name, "create");
       strictEqual(method.kind, "basic");
-      strictEqual(method.parameters.length, 5);
+      strictEqual(method.parameters.length, 3);
       deepStrictEqual(
         method.parameters.map((x) => x.name),
-        ["id", "weight", "color", "contentType", "accept"]
+        ["widget", "contentType", "accept"]
       );
 
       const bodyParameter = method.operation.bodyParam;
       ok(bodyParameter);
       strictEqual(bodyParameter.kind, "body");
       //eslint-disable-next-line deprecation/deprecation
-      strictEqual(bodyParameter.nameInClient, "body");
-      strictEqual(bodyParameter.name, "body");
+      strictEqual(bodyParameter.nameInClient, "widget");
+      strictEqual(bodyParameter.name, "widget");
       strictEqual(bodyParameter.onClient, false);
       strictEqual(bodyParameter.optional, false);
       strictEqual(bodyParameter.type.kind, "model");
@@ -2104,41 +2164,17 @@ describe("typespec-client-generator-core: package", () => {
       const method = getServiceMethodOfClient(sdkPackage);
       strictEqual(method.name, "update");
       strictEqual(method.kind, "basic");
-      strictEqual(method.parameters.length, 5);
-
-      const methodParamId = method.parameters[0];
-      strictEqual(methodParamId.kind, "method");
-      //eslint-disable-next-line deprecation/deprecation
-      strictEqual(methodParamId.nameInClient, "id");
-      strictEqual(methodParamId.name, "id");
-      strictEqual(methodParamId.optional, false);
-      strictEqual(methodParamId.onClient, false);
-      strictEqual(methodParamId.isApiVersionParam, false);
-      strictEqual(methodParamId.type.kind, "string");
-
-      const methodParamWeight = method.parameters[1];
-      strictEqual(methodParamWeight.kind, "method");
-      //eslint-disable-next-line deprecation/deprecation
-      strictEqual(methodParamWeight.nameInClient, "weight");
-      strictEqual(methodParamWeight.name, "weight");
-      strictEqual(methodParamWeight.optional, false);
-      strictEqual(methodParamWeight.onClient, false);
-      strictEqual(methodParamWeight.isApiVersionParam, false);
-      strictEqual(methodParamWeight.type.kind, "int32");
-
-      const methodParamColor = method.parameters[2];
-      strictEqual(methodParamColor.kind, "method");
-      //eslint-disable-next-line deprecation/deprecation
-      strictEqual(methodParamColor.nameInClient, "color");
-      strictEqual(methodParamColor.name, "color");
-      strictEqual(methodParamColor.optional, false);
-      strictEqual(methodParamColor.onClient, false);
-      strictEqual(methodParamColor.isApiVersionParam, false);
-      strictEqual(methodParamColor.type.kind, "enum");
-      strictEqual(methodParamColor.type.values[0].value, "red");
-      strictEqual(methodParamColor.type.values[0].valueType.kind, "string");
-      strictEqual(methodParamColor.type.values[1].value, "blue");
-      strictEqual(methodParamColor.type.values[1].valueType.kind, "string");
+      strictEqual(method.parameters.length, 3);
+
+      const methodParam = method.parameters[0];
+      strictEqual(methodParam.kind, "method");
+      //eslint-disable-next-line deprecation/deprecation
+      strictEqual(methodParam.nameInClient, "widget");
+      strictEqual(methodParam.name, "widget");
+      strictEqual(methodParam.optional, false);
+      strictEqual(methodParam.onClient, false);
+      strictEqual(methodParam.isApiVersionParam, false);
+      strictEqual(methodParam.type.kind, "model");
 
       const methodContentTypeParam = method.parameters.find((x) => x.name === "contentType");
       ok(methodContentTypeParam);
@@ -2192,7 +2228,7 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(operationAcceptParam.optional, false);
 
       const correspondingMethodParams = bodyParameter.correspondingMethodParams.map((x) => x.name);
-      deepStrictEqual(correspondingMethodParams, ["weight", "color"]);
+      deepStrictEqual(correspondingMethodParams, ["widget"]);
       deepStrictEqual(
         bodyParameter.type.properties.map((p) => p.name),
         ["id", "weight", "color"]