TCGC common layer refine (#517)

.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,
@@ -29,6 +30,7 @@ import {
   SdkHttpResponse,
   SdkMethodParameter,
   SdkModelPropertyType,
+  SdkModelType,
   SdkParameter,
   SdkPathParameter,
   SdkQueryParameter,
@@ -123,10 +125,14 @@ 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,
         isGeneratedName: true,
         description: getDocHelper(context, tspBody.type).description,
         details: getDocHelper(context, tspBody.type).details,
@@ -135,9 +141,7 @@ function getSdkHttpParameters(
         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,
@@ -393,33 +397,54 @@ export function getCorrespondingMethodParams(
   serviceParam: SdkHttpParameter
 ): SdkModelPropertyType[] {
   if (serviceParam.isApiVersionParam) {
-    if (!context.__api_version_parameter) throw new Error("No api version on the client");
+    if (!context.__api_version_parameter) {
+      const apiVersionParam = methodParameters.find((x) => x.name.includes("apiVersion"));
+      if (!apiVersionParam) {
+        throw new Error("Can't find corresponding api version parameter");
+      }
+      if (apiVersionParam.type.kind === "model") throw new Error(apiVersionParam.type.name);
+      throw new Error(apiVersionParam.type.kind);
+    }
     return [context.__api_version_parameter];
   }
   const correspondingMethodParameter = methodParameters.find((x) => x.name === serviceParam.name);
   if (correspondingMethodParameter) {
     return [correspondingMethodParameter];
   }
-  function paramInProperties(param: SdkModelPropertyType, type: SdkType): boolean {
-    if (type.kind !== "model") return false;
-    return Array.from(type.properties.values())
-      .filter((x) => x.kind === "property")
-      .map((x) => x.name)
-      .includes(param.name);
-  }
+
   const serviceParamType = serviceParam.type;
   if (serviceParam.kind === "body" && serviceParamType.kind === "model") {
-    // Here we have a spread body parameter
-    const correspondingProperties = methodParameters.filter((x) =>
-      paramInProperties(x, serviceParamType)
-    );
-    const bodyPropertyNames = serviceParamType.properties.filter((x) =>
-      paramInProperties(x, serviceParamType)
+    const serviceParamPropertyNames = Array.from(serviceParamType.properties.values())
+      .filter((x) => x.kind === "property")
+      .map((x) => x.name);
+    // Here we have a spread method parameter
+
+    // easy case is if the spread method parameter directly has the entire body as a property
+    const directBodyProperty = methodParameters
+      .map((x) => x.type)
+      .filter((x): x is SdkModelType => x.kind === "model")
+      .flatMap((x) => x.properties)
+      .find((x) => x.type === serviceParamType);
+    if (directBodyProperty) return [directBodyProperty];
+    let correspondingProperties: SdkModelPropertyType[] = methodParameters.filter((x) =>
+      serviceParamPropertyNames.includes(x.name)
     );
-    if (correspondingProperties.length !== bodyPropertyNames.length) {
-      throw new Error("Can't find corresponding properties for spread body parameter");
+    for (const methodParam of methodParameters) {
+      const methodParamIterable =
+        methodParam.type.kind === "model" ? methodParam.type.properties : [methodParam];
+      correspondingProperties = correspondingProperties.concat(
+        methodParamIterable.filter(
+          (x) =>
+            serviceParamPropertyNames.includes(x.name) &&
+            !correspondingProperties.find((e) => e.name === x.name)
+        )
+      );
     }
-    return correspondingProperties;
+    if (correspondingProperties.length === serviceParamType.properties.length)
+      return correspondingProperties;
+    throw new Error(
+      `Can't find corresponding parameter for ${serviceParam.name} out of ${methodParameters.map((m) => m.name).join(", ")}`
+    );
   }
   for (const methodParam of methodParameters) {
     if (methodParam.type.kind === "model") {

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/internal-utils.ts

@@ -1,7 +1,6 @@
 import { getUnionAsEnum } from "@azure-tools/typespec-azure-core";
 import {
   Model,
-  ModelProperty,
   Namespace,
   Operation,
   Program,
@@ -82,7 +81,7 @@ export function getClientNamespaceStringHelper(
  */
 export function updateWithApiVersionInformation(
   context: TCGCContext,
-  type: ModelProperty
+  type: { name: string }
 ): {
   isApiVersionParam: boolean;
   clientDefaultValue?: unknown;

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

@@ -1,14 +1,17 @@
 import { getLroMetadata, getPagedResult } from "@azure-tools/typespec-azure-core";
 import {
   Diagnostic,
+  Model,
   ModelProperty,
   Operation,
   createDiagnosticCollector,
   getNamespaceFullName,
   getService,
+  isKey,
 } from "@typespec/compiler";
 import { getServers } from "@typespec/http";
 import { resolveVersions } from "@typespec/versioning";
+import { camelCase } from "change-case";
 import {
   getAccess,
   listClients,
@@ -53,6 +56,7 @@ import {
   getHashForType,
   getSdkTypeBaseHelper,
   isNullable,
+  updateWithApiVersionInformation,
 } from "./internal-utils.js";
 import { createDiagnostic } from "./lib.js";
 import {
@@ -218,9 +222,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 && !isKey(context.program, prop.sourceProperty)) {
+      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 +350,29 @@ function getSdkInitializationType<
 
 function getSdkMethodParameter(
   context: TCGCContext,
-  type: ModelProperty
+  type: ModelProperty | Model
 ): [SdkMethodParameter, readonly Diagnostic[]] {
   const diagnostics = createDiagnosticCollector();
+  if (type.kind === "Model") {
+    const libraryName = getLibraryName(context, type);
+    const name = camelCase(libraryName ?? "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,
+      isGeneratedName: Boolean(libraryName),
+      optional: false,
+      nullable: false,
+      discriminator: false,
+      serializedName: name,
+      ...updateWithApiVersionInformation(context, type),
+    });
+  }
   return diagnostics.wrap({
     ...diagnostics.pipe(getSdkModelPropertyType(context, type)),
     kind: "method",

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

@@ -19,7 +19,6 @@ import {
 } from "@typespec/compiler";
 import {
   HttpOperation,
-  HttpOperationParameter,
   getHeaderFieldName,
   getHttpOperation,
   getPathParamName,
@@ -62,12 +61,10 @@ export function getDefaultApiVersion(
  * @param parameter
  * @returns
  */
-export function isApiVersion(
-  context: TCGCContext,
-  parameter: HttpOperationParameter | ModelProperty
-): boolean {
+export function isApiVersion(context: TCGCContext, type: { name: string }): boolean {
   return (
-    parameter.name.toLowerCase() === "apiversion" || parameter.name.toLowerCase() === "api-version"
+    type.name.toLowerCase().includes("apiversion") ||
+    type.name.toLowerCase().includes("api-version")
   );
 }
 
@@ -174,7 +171,13 @@ 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
+        .filter((arg): arg is Model => arg.kind === "Model" && arg.name.length > 0)
+        .map((arg) => pascalCase(arg.name))
+        .join("")
+    );
   }
 
   return typeof type.name === "string" ? type.name : "";

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

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