[tcgc] spread behavior change

.chronus/changes/spread_model-2024-7-16-15-0-33.md

@@ -0,0 +1,7 @@
+---
+changeKind: feature
+packages:
+  - "@azure-tools/typespec-client-generator-core"
+---
+
+use original model for spread if it is from a simple spread

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

@@ -1,5 +1,6 @@
 import {
   Diagnostic,
+  Model,
   ModelProperty,
   Operation,
   Type,
@@ -46,11 +47,13 @@ import {
 import {
   getAvailableApiVersions,
   getDocHelper,
+  getHttpBodySpreadModel,
   getHttpOperationResponseHeaders,
   getLocationOfOperation,
   getTypeDecorators,
   isAcceptHeader,
   isContentTypeHeader,
+  isHttpBodySpread,
   isNeverOrVoidType,
   isSubscriptionId,
 } from "./internal-utils.js";
@@ -155,9 +158,21 @@ function getSdkHttpParameters(
       }
       retval.bodyParam = bodyParam;
     } else if (!isNeverOrVoidType(tspBody.type)) {
-      const type = diagnostics.pipe(
-        getClientTypeWithDiagnostics(context, tspBody.type, httpOperation.operation)
-      );
+      const spread = isHttpBodySpread(tspBody);
+      let type: SdkType;
+      if (spread) {
+        type = diagnostics.pipe(
+          getClientTypeWithDiagnostics(
+            context,
+            getHttpBodySpreadModel(tspBody.type as Model),
+            httpOperation.operation
+          )
+        );
+      } else {
+        type = diagnostics.pipe(
+          getClientTypeWithDiagnostics(context, tspBody.type, httpOperation.operation)
+        );
+      }
       const name = camelCase((type as { name: string }).name ?? "body");
       retval.bodyParam = {
         kind: "body",
@@ -575,8 +590,7 @@ function findMapping(
     if (
       methodParam.__raw &&
       serviceParam.__raw &&
-      (methodParam.__raw === serviceParam.__raw ||
-        methodParam.__raw === serviceParam.__raw.sourceProperty)
+      methodParam.__raw.node === serviceParam.__raw.node
     ) {
       return methodParam;
     }

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

@@ -37,7 +37,6 @@ export interface TCGCContext {
   modelsMap?: Map<Type, SdkModelType | SdkEnumType>;
   operationModelsMap?: Map<Operation, Map<Type, SdkModelType | SdkEnumType>>;
   generatedNames?: Map<Union | Model | TspLiteralType, string>;
-  spreadModels?: Map<Model, SdkModelType>;
   httpOperationCache?: Map<Operation, HttpOperation>;
   unionsMap?: Map<Union, SdkUnionType>;
   __namespaceToApiVersionParameter: Map<Interface | Namespace, SdkParameter>;

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

@@ -10,6 +10,7 @@ import {
   isNeverType,
   isNullType,
   isVoidType,
+  Model,
   ModelProperty,
   Namespace,
   Numeric,
@@ -21,7 +22,13 @@ import {
   Union,
   Value,
 } from "@typespec/compiler";
-import { HttpOperation, HttpOperationResponseContent, HttpStatusCodeRange } from "@typespec/http";
+import {
+  HttpOperation,
+  HttpOperationBody,
+  HttpOperationMultipartBody,
+  HttpOperationResponseContent,
+  HttpStatusCodeRange,
+} from "@typespec/http";
 import { getAddedOnVersions, getRemovedOnVersions, getVersions } from "@typespec/versioning";
 import {
   DecoratorInfo,
@@ -546,3 +553,38 @@ export function isXmlContentType(contentType: string): boolean {
   const regex = new RegExp(/^(application|text)\/(.+\+)?xml$/);
   return regex.test(contentType);
 }
+
+/**
+ * If body is from spread, then it does not directly from a model property.
+ * @param httpBody
+ * @param parameters
+ * @returns
+ */
+export function isHttpBodySpread(httpBody: HttpOperationBody | HttpOperationMultipartBody) {
+  return httpBody.property === undefined;
+}
+
+/**
+ * If body is from simple spread, then we use the original model as body model.
+ * @param type
+ * @returns
+ */
+export function getHttpBodySpreadModel(type: Model): Model {
+  if (type.sourceModels.length === 1 && type.sourceModels[0].usage === "spread") {
+    const innerModel = type.sourceModels[0].model;
+    // for case: `op test(...Model):void;`
+    if (innerModel.name !== "" && innerModel.properties.size === type.properties.size) {
+      return innerModel;
+    }
+    // for case: `op test(@header h: string, @query q: string, ...Model): void;`
+    if (
+      innerModel.sourceModels.length === 1 &&
+      innerModel.sourceModels[0].usage === "spread" &&
+      innerModel.sourceModels[0].model.name !== "" &&
+      innerModel.sourceModels[0].model.properties.size === type.properties.size
+    ) {
+      return innerModel.sourceModels[0].model;
+    }
+  }
+  return type;
+}

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

@@ -37,7 +37,9 @@ import {
 import {
   TspLiteralType,
   getClientNamespaceStringHelper,
+  getHttpBodySpreadModel,
   getHttpOperationResponseHeaders,
+  isHttpBodySpread,
   parseEmitterName,
   removeVersionsLargerThanExplicitlySpecified,
 } from "./internal-utils.js";
@@ -379,7 +381,13 @@ function getContextPath(
     if (httpOperation.parameters.body) {
       visited.clear();
       result = [{ name: root.name }];
-      if (dfsModelProperties(typeToFind, httpOperation.parameters.body.type, "Request")) {
+      let bodyType: Type;
+      if (isHttpBodySpread(httpOperation.parameters.body)) {
+        bodyType = getHttpBodySpreadModel(httpOperation.parameters.body.type as Model);
+      } else {
+        bodyType = httpOperation.parameters.body.type;
+      }
+      if (dfsModelProperties(typeToFind, bodyType, "Request")) {
         return result;
       }
     }

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

@@ -87,6 +87,7 @@ import {
   filterApiVersionsInEnum,
   getAvailableApiVersions,
   getDocHelper,
+  getHttpBodySpreadModel,
   getHttpOperationResponseHeaders,
   getLocationOfOperation,
   getNonNullOptions,
@@ -95,6 +96,7 @@ import {
   getTypeDecorators,
   intOrFloat,
   isAzureCoreModel,
+  isHttpBodySpread,
   isJsonContentType,
   isMultipartFormData,
   isMultipartOperation,
@@ -1378,15 +1380,17 @@ export function getSdkModelPropertyType(
     isMultipartFileInput: false,
     flatten: shouldFlattenProperty(context, type),
   };
-  if (operation) {
+  if (operation && type.model) {
     const httpOperation = getHttpOperationWithCache(context, operation);
-    if (
-      type.model &&
-      httpOperation.parameters.body &&
-      httpOperation.parameters.body.type === type.model
-    ) {
-      // only add multipartOptions for property of multipart body
-      diagnostics.pipe(updateMultiPartInfo(context, type, result, operation));
+    const httpBody = httpOperation.parameters.body;
+    if (httpBody) {
+      const httpBodyType = isHttpBodySpread(httpBody)
+        ? getHttpBodySpreadModel(httpBody.type as Model)
+        : httpBody.type;
+      if (type.model === httpBodyType) {
+        // only try to add multipartOptions for property of body
+        diagnostics.pipe(updateMultiPartInfo(context, type, result, operation));
+      }
     }
   }
   return diagnostics.wrap(result);
@@ -1491,6 +1495,7 @@ function updateModelsMap(
 interface ModelUsageOptions {
   seenModelNames?: Set<SdkType>;
   propagation?: boolean;
+  skipFirst?: boolean;
   // this is used to prevent propagation usage from subtype to base type's other subtypes
   ignoreSubTypeStack?: boolean[];
 }
@@ -1529,11 +1534,15 @@ function updateUsageOfModel(
   if (type.kind !== "model" && type.kind !== "enum") return;
   options.seenModelNames.add(type);
 
-  const usageOverride = getUsageOverride(context, type.__raw as any);
-  if (usageOverride) {
-    type.usage |= usageOverride | usage;
+  if (!options.skipFirst) {
+    const usageOverride = getUsageOverride(context, type.__raw as any);
+    if (usageOverride) {
+      type.usage |= usageOverride | usage;
+    } else {
+      type.usage |= usage;
+    }
   } else {
-    type.usage |= usage;
+    options.skipFirst = false;
   }
 
   if (type.kind === "enum") return;
@@ -1595,32 +1604,27 @@ function updateTypesFromOperation(
   }
   const httpBody = httpOperation.parameters.body;
   if (httpBody && !isNeverOrVoidType(httpBody.type)) {
-    const sdkType = diagnostics.pipe(
-      getClientTypeWithDiagnostics(context, httpBody.type, operation)
-    );
-    if (generateConvenient) {
-      // Special logic for spread body model:
-      // If body is from spread, then it should be an anonymous model.
-      // Also all model properties should be
-      // either equal to one of operation parameters (for case spread from model without property with metadata decorator)
-      // or its source property equal to one of operation parameters (for case spread from model with property with metadata decorator)
-      if (
-        httpBody.type.kind === "Model" &&
-        httpBody.type.name === "" &&
-        [...httpBody.type.properties.keys()].every(
-          (k) =>
-            operation.parameters.properties.has(k) &&
-            (operation.parameters.properties.get(k) ===
-              (httpBody.type as Model).properties.get(k) ||
-              operation.parameters.properties.get(k) ===
-                (httpBody.type as Model).properties.get(k)?.sourceProperty)
+    const spread = isHttpBodySpread(httpBody);
+    let sdkType: SdkType;
+    if (spread) {
+      sdkType = diagnostics.pipe(
+        getClientTypeWithDiagnostics(
+          context,
+          getHttpBodySpreadModel(httpBody.type as Model),
+          operation
         )
-      ) {
-        if (!context.spreadModels?.has(httpBody.type)) {
-          context.spreadModels?.set(httpBody.type as Model, sdkType as SdkModelType);
-        }
+      );
+    } else {
+      sdkType = diagnostics.pipe(getClientTypeWithDiagnostics(context, httpBody.type, operation));
+    }
+    if (generateConvenient) {
+      if (spread) {
+        updateUsageOfModel(context, UsageFlags.Spread, sdkType, { propagation: false });
+        updateUsageOfModel(context, UsageFlags.Input, sdkType, { skipFirst: true });
+      } else {
+        updateUsageOfModel(context, UsageFlags.Input, sdkType);
       }
-      updateUsageOfModel(context, UsageFlags.Input, sdkType);
+
       if (httpBody.contentTypes.some((x) => isJsonContentType(x))) {
         updateUsageOfModel(context, UsageFlags.Json, sdkType);
       }
@@ -1725,10 +1729,14 @@ function updateAccessOfModel(context: TCGCContext): void {
 }
 
 function updateSpreadModelUsageAndAccess(context: TCGCContext): void {
-  for (const sdkType of context.spreadModels?.values() ?? []) {
-    // if a type has spread usage, then it must be internal
-    sdkType.access = "internal";
-    sdkType.usage = (sdkType.usage & ~UsageFlags.Input) | UsageFlags.Spread;
+  for (const [_, sdkType] of context.modelsMap?.entries() ?? []) {
+    if (
+      (sdkType.usage & UsageFlags.Spread) > 0 &&
+      (sdkType.usage & (UsageFlags.Input | UsageFlags.Output)) === 0
+    ) {
+      // if a type has spread usage, but not used in any other operation, then set it to be internal
+      sdkType.access = "internal";
+    }
   }
 }
 
@@ -1811,9 +1819,6 @@ export function getAllModelsWithDiagnostics(
   if (context.operationModelsMap === undefined) {
     context.operationModelsMap = new Map<Operation, Map<Type, SdkModelType | SdkEnumType>>();
   }
-  if (context.spreadModels === undefined) {
-    context.spreadModels = new Map<Model, SdkModelType>();
-  }
   for (const client of listClients(context)) {
     for (const operation of listOperationsInOperationGroup(context, client)) {
       // operations on a client

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

@@ -4031,14 +4031,10 @@ describe("typespec-client-generator-core: decorators", () => {
         "stableFunctionality"
       );
       strictEqual(runnerWithVersion.context.sdkPackage.models.length, 2);
-      strictEqual(
-        runnerWithVersion.context.sdkPackage.models[0].name,
-        "PreviewFunctionalityRequest"
-      );
-      strictEqual(
-        runnerWithVersion.context.sdkPackage.models[1].name,
-        "StableFunctionalityRequest"
-      );
+      strictEqual(runnerWithVersion.context.sdkPackage.models[0].name, "PreviewModel");
+      strictEqual(runnerWithVersion.context.sdkPackage.models[0].access, "internal");
+      strictEqual(runnerWithVersion.context.sdkPackage.models[1].name, "StableModel");
+      strictEqual(runnerWithVersion.context.sdkPackage.models[1].access, "internal");
 
       runnerWithVersion = await createSdkTestRunner({
         emitterName: "@azure-tools/typespec-python",
@@ -4053,9 +4049,11 @@ describe("typespec-client-generator-core: decorators", () => {
         "stableFunctionality"
       );
       strictEqual(runnerWithVersion.context.sdkPackage.models.length, 1);
+      strictEqual(runnerWithVersion.context.sdkPackage.models[0].name, "StableModel");
+      strictEqual(runnerWithVersion.context.sdkPackage.models[0].access, "internal");
       strictEqual(
-        runnerWithVersion.context.sdkPackage.models[0].name,
-        "StableFunctionalityRequest"
+        runnerWithVersion.context.sdkPackage.models[0].usage,
+        UsageFlags.Spread | UsageFlags.Json
       );
     });
     it("add client", async () => {
@@ -4549,7 +4547,7 @@ describe("typespec-client-generator-core: decorators", () => {
       await runner.compileWithCustomization(mainCode, customizationCode);
       // runner has python scope, so shouldn't be overridden
 
-      ok(!runner.context.sdkPackage.models.find((x) => x.name === "Params"));
+      ok(runner.context.sdkPackage.models.find((x) => x.name === "Params"));
       const sdkPackage = runner.context.sdkPackage;
       const client = sdkPackage.clients[0];
       strictEqual(client.methods.length, 1);