Tcgc/keep core models (#1700)

fixes #1412

---------

Co-authored-by: iscai-msft <isabellavcai@gmail.com>
Co-authored-by: Chenjie Shi <tadelesh.shi@live.cn>

.chronus/changes/tcgc-keepCoreModels-2024-9-16-19-20-24.md

@@ -0,0 +1,7 @@
+---
+changeKind: breaking
+packages:
+  - "@azure-tools/typespec-client-generator-core"
+---
+
+We no longer filter out core models. The `filter-out-core-models` parameter to `SdkContext` is also removed

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

@@ -62,7 +62,7 @@ import {
   AllScopes,
   clientNameKey,
   getValidApiVersion,
-  isAzureCoreModel,
+  isAzureCoreTspModel,
   parseEmitterName,
 } from "./internal-utils.js";
 import { createStateSymbol, reportDiagnostic } from "./lib.js";
@@ -650,7 +650,6 @@ export async function createSdkContext<
     sdkPackage: undefined!,
     generateProtocolMethods: generateProtocolMethods,
     generateConvenienceMethods: generateConvenienceMethods,
-    filterOutCoreModels: context.options["filter-out-core-models"] ?? true,
     packageName: context.options["package-name"],
     flattenUnionAsEnum: context.options["flatten-union-as-enum"] ?? true,
     apiVersion: options?.versioning?.strategy === "ignore" ? "all" : context.options["api-version"],
@@ -910,7 +909,7 @@ function collectParams(
         while (sourceProp.sourceProperty) {
           sourceProp = sourceProp.sourceProperty;
         }
-        if (sourceProp.model && !isAzureCoreModel(sourceProp.model)) {
+        if (sourceProp.model && !isAzureCoreTspModel(sourceProp.model)) {
           params.push(value);
         } else if (!sourceProp.model) {
           params.push(value);

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

@@ -34,7 +34,6 @@ export interface TCGCContext {
   emitterName: string;
   generateProtocolMethods?: boolean;
   generateConvenienceMethods?: boolean;
-  filterOutCoreModels?: boolean;
   packageName?: string;
   flattenUnionAsEnum?: boolean;
   arm?: boolean;

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

@@ -329,7 +329,7 @@ export function intOrFloat(value: number): "int32" | "float32" {
  * @param t
  * @returns
  */
-export function isAzureCoreModel(t: Type): boolean {
+export function isAzureCoreTspModel(t: Type): boolean {
   return (
     (t.kind === "Model" || t.kind === "Enum" || t.kind === "Union") &&
     t.namespace !== undefined &&

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

@@ -32,13 +32,15 @@ import {
   SdkClientType,
   SdkHttpOperationExample,
   SdkServiceOperation,
+  SdkType,
   TCGCContext,
 } from "./interfaces.js";
 import {
   TspLiteralType,
   getClientNamespaceStringHelper,
   getHttpBodySpreadModel,
   getHttpOperationResponseHeaders,
+  isAzureCoreTspModel,
   isHttpBodySpread,
   parseEmitterName,
   removeVersionsLargerThanExplicitlySpecified,
@@ -675,3 +677,7 @@ export function listSubClients<TServiceOperation extends SdkServiceOperation>(
   }
   return subClients;
 }
+
+export function isAzureCoreModel(t: SdkType): boolean {
+  return t.__raw !== undefined && isAzureCoreTspModel(t.__raw);
+}

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

@@ -92,7 +92,6 @@ import {
   getSdkTypeBaseHelper,
   getTypeDecorators,
   intOrFloat,
-  isAzureCoreModel,
   isHttpBodySpread,
   isJsonContentType,
   isMultipartOperation,
@@ -1754,15 +1753,7 @@ function filterOutTypes(
   filter: number,
 ): (SdkModelType | SdkEnumType | SdkUnionType | SdkNullableType)[] {
   const result = new Set<SdkModelType | SdkEnumType | SdkUnionType | SdkNullableType>();
-  for (const [type, sdkType] of context.referencedTypeMap?.entries() ?? []) {
-    // filter models/enums/union of Core
-    if (
-      context.filterOutCoreModels &&
-      ["Enum", "Model", "Union"].includes(type.kind) &&
-      isAzureCoreModel(type)
-    ) {
-      continue;
-    }
+  for (const sdkType of context.referencedTypeMap?.values() ?? []) {
     // filter models with unexpected usage
     if ((sdkType.usage & filter) === 0) {
       continue;

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

@@ -10,6 +10,7 @@ import {
   SdkPackage,
   SdkServiceMethod,
 } from "../src/interfaces.js";
+import { isAzureCoreModel } from "../src/public-utils.js";
 import { SdkTestRunner, createSdkTestRunner } from "./test-host.js";
 
 describe("typespec-client-generator-core: package", () => {
@@ -1038,14 +1039,13 @@ describe("typespec-client-generator-core: package", () => {
       ok(exception.type);
       strictEqual(exception.type.kind, "model");
       strictEqual(exception.type.crossLanguageDefinitionId, "Azure.Core.Foundations.ErrorResponse");
-      // we shouldn't generate this model
-      strictEqual(
+      ok(
         sdkPackage.models.find(
-          (x) => x.crossLanguageDefinitionId === "Azure.Core.Foundations.ErrorResponse",
+          (x) =>
+            x.crossLanguageDefinitionId === "Azure.Core.Foundations.ErrorResponse" &&
+            isAzureCoreModel(x),
         ),
-        undefined,
       );
-
       const methodResponse = createOrUpdate.response;
       strictEqual(methodResponse.kind, "method");
       strictEqual(methodResponse.type, widgetModel);
@@ -1067,8 +1067,16 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(method.name, "delete");
       strictEqual(method.kind, "lro");
       strictEqual(method.response.type, undefined);
-      strictEqual(runnerWithCore.context.sdkPackage.models.length, 0);
-      strictEqual(runnerWithCore.context.sdkPackage.enums.length, 1);
+      strictEqual(runnerWithCore.context.sdkPackage.models.length, 3);
+      strictEqual(
+        runnerWithCore.context.sdkPackage.models.filter((x) => !isAzureCoreModel(x)).length,
+        0,
+      );
+      strictEqual(runnerWithCore.context.sdkPackage.enums.length, 2);
+      strictEqual(
+        runnerWithCore.context.sdkPackage.enums.filter((x) => !isAzureCoreModel(x)).length,
+        1,
+      );
     });
     it("paging", async () => {
       const runnerWithCore = await createSdkTestRunner({
@@ -1085,8 +1093,12 @@ describe("typespec-client-generator-core: package", () => {
       );
       const sdkPackage = runnerWithCore.context.sdkPackage;
       strictEqual(sdkPackage.clients.length, 1);
-      strictEqual(sdkPackage.models.length, 1);
-      strictEqual(sdkPackage.models[0].name, "Manufacturer");
+      strictEqual(sdkPackage.models.length, 5);
+      strictEqual(sdkPackage.models.filter((x) => !isAzureCoreModel(x)).length, 1);
+      const manufacturerModel = sdkPackage.models.find(
+        (x) => !isAzureCoreModel(x) && x.name === "Manufacturer",
+      );
+      ok(manufacturerModel);
       const widgetClient = sdkPackage.clients[0].methods.find((x) => x.kind === "clientaccessor")
         ?.response as SdkClientType<SdkHttpOperation>;
       ok(widgetClient);
@@ -1115,7 +1127,7 @@ describe("typespec-client-generator-core: package", () => {
       const methodResponse = listManufacturers.response.type;
       ok(methodResponse);
       strictEqual(methodResponse.kind, "array");
-      deepStrictEqual(methodResponse.valueType, sdkPackage.models[0]);
+      deepStrictEqual(methodResponse.valueType, manufacturerModel);
 
       const operation = listManufacturers.operation;
       strictEqual(operation.kind, "http");
@@ -1154,7 +1166,7 @@ describe("typespec-client-generator-core: package", () => {
       ok(valueProperty);
       strictEqual(valueProperty.kind, "property");
       strictEqual(valueProperty.type.kind, "array");
-      strictEqual(valueProperty.type.valueType, sdkPackage.models[0]);
+      strictEqual(valueProperty.type.valueType, manufacturerModel);
 
       const nextLinkProperty = pagingModel.properties.find((x) => x.name === "nextLink");
       ok(nextLinkProperty);

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

@@ -7,6 +7,7 @@ import {
   SdkServiceMethod,
   UsageFlags,
 } from "../../src/interfaces.js";
+import { isAzureCoreModel } from "../../src/public-utils.js";
 import { getAllModels } from "../../src/types.js";
 import { SdkTestRunner, createSdkTestRunner } from "../test-host.js";
 import { getServiceMethodOfClient } from "./utils.js";
@@ -250,7 +251,8 @@ describe("typespec-client-generator-core: spread", () => {
         }
       `);
     const sdkPackage = runnerWithCore.context.sdkPackage;
-    strictEqual(sdkPackage.models.length, 2);
+    const nonCoreModels = sdkPackage.models.filter((x) => !isAzureCoreModel(x));
+    strictEqual(nonCoreModels.length, 2);
 
     const client = sdkPackage.clients[0].methods.find((x) => x.kind === "clientaccessor")
       ?.response as SdkClientType<SdkHttpOperation>;

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

@@ -21,6 +21,7 @@ import {
   getLibraryName,
   getPropertyNames,
   isApiVersion,
+  isAzureCoreModel,
 } from "../src/public-utils.js";
 import { getAllModels, getSdkUnion } from "../src/types.js";
 import {
@@ -1777,7 +1778,7 @@ describe("typespec-client-generator-core: public-utils", () => {
         emitterName: "@azure-tools/typespec-java",
       });
       await runnerWithCore.compile(lroCode);
-      const models = runnerWithCore.context.sdkPackage.models;
+      const models = runnerWithCore.context.sdkPackage.models.filter((x) => !isAzureCoreModel(x));
       strictEqual(models.length, 1);
       deepStrictEqual(models[0].name, "ExportedUser");
     });
@@ -1788,7 +1789,6 @@ describe("typespec-client-generator-core: public-utils", () => {
         emitterName: "@azure-tools/typespec-java",
       });
       await runnerWithCore.compile(lroCode);
-      runnerWithCore.context.filterOutCoreModels = false;
       const models = getAllModels(runnerWithCore.context);
       strictEqual(models.length, 8);
       // there should only be one non-core model

packages/typespec-client-generator-core/test/types/model-types.test.ts

@@ -1,9 +1,10 @@
 import { AzureCoreTestLibrary } from "@azure-tools/typespec-azure-core/testing";
 import { isErrorModel } from "@typespec/compiler";
 import { deepStrictEqual, ok, strictEqual } from "assert";
-import { afterEach, beforeEach, describe, it } from "vitest";
+import { beforeEach, describe, it } from "vitest";
 import { SdkBodyModelPropertyType, UsageFlags } from "../../src/interfaces.js";
-import { isAzureCoreModel } from "../../src/internal-utils.js";
+import { isAzureCoreTspModel } from "../../src/internal-utils.js";
+import { isAzureCoreModel } from "../../src/public-utils.js";
 import { getAllModels } from "../../src/types.js";
 import { SdkTestRunner, createSdkTestRunner } from "../test-host.js";
 
@@ -13,16 +14,6 @@ describe("typespec-client-generator-core: model types", () => {
   beforeEach(async () => {
     runner = await createSdkTestRunner({ emitterName: "@azure-tools/typespec-java" });
   });
-  afterEach(async () => {
-    for (const modelsOrEnums of [
-      runner.context.sdkPackage.models,
-      runner.context.sdkPackage.enums,
-    ]) {
-      for (const item of modelsOrEnums) {
-        ok(item.name !== "");
-      }
-    }
-  });
   it("basic", async () => {
     await runner.compile(`
         @service({})
@@ -713,18 +704,14 @@ describe("typespec-client-generator-core: model types", () => {
       @doc("Creates or updates a User")
       op createOrUpdate is StandardResourceOperations.ResourceCreateOrUpdate<User>;
       `);
-    const models = runner.context.sdkPackage.models;
+    const models = runner.context.sdkPackage.models.filter((x) => !isAzureCoreModel(x));
     strictEqual(models.length, 1);
     strictEqual(models[0].name, "User");
     strictEqual(models[0].crossLanguageDefinitionId, "My.Service.User");
 
     for (const [type, sdkType] of runner.context.referencedTypeMap?.entries() ?? []) {
-      if (isAzureCoreModel(type)) {
-        ok(
-          sdkType.kind !== "union" &&
-            sdkType.kind !== "nullable" &&
-            sdkType.usage !== UsageFlags.None,
-        );
+      if (isAzureCoreTspModel(type)) {
+        ok(sdkType.usage !== UsageFlags.None);
       }
     }
   });
@@ -787,7 +774,7 @@ describe("typespec-client-generator-core: model types", () => {
       @pollingOperation(My.Service.getStatus)
       op createOrUpdateUser is StandardResourceOperations.LongRunningResourceCreateOrUpdate<User>;
       `);
-    const models = runner.context.sdkPackage.models;
+    const models = runner.context.sdkPackage.models.filter((x) => !isAzureCoreModel(x));
     strictEqual(models.length, 1);
     strictEqual(models[0].name, "User");
     strictEqual(models[0].crossLanguageDefinitionId, "My.Service.User");
@@ -832,6 +819,35 @@ describe("typespec-client-generator-core: model types", () => {
     strictEqual(runner.context.sdkPackage.enums.length, 1);
     strictEqual(runner.context.sdkPackage.enums[0].name, "OperationState");
   });
+
+  it("model with core property", async () => {
+    const runnerWithCore = await createSdkTestRunner({
+      librariesToAdd: [AzureCoreTestLibrary],
+      autoUsings: ["Azure.Core"],
+      emitterName: "@azure-tools/typespec-java",
+    });
+    await runnerWithCore.compileWithBuiltInAzureCoreService(`
+      @usage(Usage.input)
+      model MyError {
+        innerError: Azure.Core.Foundations.Error;
+      }
+      `);
+    const models = runnerWithCore.context.sdkPackage.models;
+    strictEqual(models.length, 3);
+    const myError = models.find((x) => x.name === "MyError");
+    ok(myError);
+
+    const azureError = models.find((x) => x.name === "Error");
+    ok(azureError);
+    strictEqual(isAzureCoreModel(azureError), true);
+
+    const azureInnerError = models.find((x) => x.name === "InnerError");
+    ok(azureInnerError);
+    strictEqual(isAzureCoreModel(azureInnerError), true);
+
+    strictEqual(myError.properties.length, 1);
+    strictEqual(myError.properties[0].type, azureError);
+  });
   it("no models filter core", async () => {
     await runner.compile(`
         @service({})