diff --git a/.chronus/changes/tcgc-keepCoreModels-2024-9-16-19-20-24.md b/.chronus/changes/tcgc-keepCoreModels-2024-9-16-19-20-24.md new file mode 100644 index 000000000..a506435f5 --- /dev/null +++ b/.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 \ No newline at end of file diff --git a/packages/typespec-client-generator-core/src/decorators.ts b/packages/typespec-client-generator-core/src/decorators.ts index 9226397f9..1052eaf23 100644 --- a/packages/typespec-client-generator-core/src/decorators.ts +++ b/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); diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index 1cad00202..7fa8a789d 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/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; diff --git a/packages/typespec-client-generator-core/src/internal-utils.ts b/packages/typespec-client-generator-core/src/internal-utils.ts index a7779e10d..d632dab0e 100644 --- a/packages/typespec-client-generator-core/src/internal-utils.ts +++ b/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 && diff --git a/packages/typespec-client-generator-core/src/public-utils.ts b/packages/typespec-client-generator-core/src/public-utils.ts index b06836dca..313e4fa46 100644 --- a/packages/typespec-client-generator-core/src/public-utils.ts +++ b/packages/typespec-client-generator-core/src/public-utils.ts @@ -32,6 +32,7 @@ import { SdkClientType, SdkHttpOperationExample, SdkServiceOperation, + SdkType, TCGCContext, } from "./interfaces.js"; import { @@ -39,6 +40,7 @@ import { getClientNamespaceStringHelper, getHttpBodySpreadModel, getHttpOperationResponseHeaders, + isAzureCoreTspModel, isHttpBodySpread, parseEmitterName, removeVersionsLargerThanExplicitlySpecified, @@ -675,3 +677,7 @@ export function listSubClients( } return subClients; } + +export function isAzureCoreModel(t: SdkType): boolean { + return t.__raw !== undefined && isAzureCoreTspModel(t.__raw); +} diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 4b15d0680..22224e990 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/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(); - 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; diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index 7d36eb30b..174eb0417 100644 --- a/packages/typespec-client-generator-core/test/package.test.ts +++ b/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; 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); diff --git a/packages/typespec-client-generator-core/test/packages/spread.test.ts b/packages/typespec-client-generator-core/test/packages/spread.test.ts index d35fce0da..07ebcdd79 100644 --- a/packages/typespec-client-generator-core/test/packages/spread.test.ts +++ b/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; 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 cdc71503f..4ec7f2784 100644 --- a/packages/typespec-client-generator-core/test/public-utils.test.ts +++ b/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 diff --git a/packages/typespec-client-generator-core/test/types/model-types.test.ts b/packages/typespec-client-generator-core/test/types/model-types.test.ts index e15ae8829..bab5dd49c 100644 --- a/packages/typespec-client-generator-core/test/types/model-types.test.ts +++ b/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; `); - 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; `); - 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({})