diff --git a/package.json b/package.json index 1434331466..9c3dfe9c6a 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "fix-version-mismatch": "syncpack fix-mismatches", "format": "prettier --write .", "format:check": "prettier . --check", + "format:dir": "prettier --write", "lint": "eslint . --max-warnings=0", "lint:fix": "eslint . --fix", "pack:all": "chronus pack --pack-destination ./temp/artifacts", diff --git a/packages/typespec-client-generator-core/CHANGELOG.md b/packages/typespec-client-generator-core/CHANGELOG.md index 5b7477dbc2..0b3b33e13c 100644 --- a/packages/typespec-client-generator-core/CHANGELOG.md +++ b/packages/typespec-client-generator-core/CHANGELOG.md @@ -1,5 +1,17 @@ # Change Log - @azure-tools/typespec-client-generator-core +## 0.48.3 + +### Bug Fixes + +- [#1907](https://github.com/Azure/typespec-azure/pull/1907) use the right path parameter name when filtering method parameter + +### Features + +- [#1919](https://github.com/Azure/typespec-azure/pull/1919) support typespec common paging +- [#1918](https://github.com/Azure/typespec-azure/pull/1918) remove none visibility property for model + + ## 0.48.2 ### Bug Fixes diff --git a/packages/typespec-client-generator-core/package.json b/packages/typespec-client-generator-core/package.json index 884fee25c8..ee4de77310 100644 --- a/packages/typespec-client-generator-core/package.json +++ b/packages/typespec-client-generator-core/package.json @@ -1,6 +1,6 @@ { "name": "@azure-tools/typespec-client-generator-core", - "version": "0.48.2", + "version": "0.48.3", "author": "Microsoft Corporation", "description": "TypeSpec Data Plane Generation library", "homepage": "https://azure.github.io/typespec-azure", diff --git a/packages/typespec-client-generator-core/src/decorators.ts b/packages/typespec-client-generator-core/src/decorators.ts index 3303cecd60..287fdb15a2 100644 --- a/packages/typespec-client-generator-core/src/decorators.ts +++ b/packages/typespec-client-generator-core/src/decorators.ts @@ -667,6 +667,7 @@ export function createTCGCContext(program: Program, emitterName: string): TCGCCo __clientToApiVersionClientDefaultValue: new Map(), previewStringRegex: /-preview$/, disableUsageAccessPropagationToBase: false, + __pagedResultSet: new Set(), }; } diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index 094627e899..8d08f2bfa3 100644 --- a/packages/typespec-client-generator-core/src/http.ts +++ b/packages/typespec-client-generator-core/src/http.ts @@ -655,7 +655,9 @@ function filterOutUselessPathParameters( param.__raw && isPathParam(context.program, param.__raw) && httpOperation.parameters.parameters.filter( - (p) => p.type === "path" && p.name === getWireName(context, param.__raw!), + (p) => + p.type === "path" && + p.name === (getPathParamName(context.program, param.__raw!) ?? param.name), ).length === 0 ) { methodParameters.splice(i, 1); diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index a4518886e1..1abadadc74 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/packages/typespec-client-generator-core/src/interfaces.ts @@ -54,6 +54,7 @@ export interface TCGCContext { decoratorsAllowList?: string[]; previewStringRegex: RegExp; disableUsageAccessPropagationToBase: boolean; + __pagedResultSet: Set; } export interface SdkContext< @@ -595,7 +596,7 @@ export interface SdkBasicServiceMethod, ): [SdkPagingServiceMethod, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - const pagedMetadata = getPagedResult(context.program, operation)!; + const basic = diagnostics.pipe( getSdkBasicServiceMethod(context, operation, client), ); - if (pagedMetadata.itemsProperty) { + + // normal paging + if (isList(context.program, operation)) { + const pagingOperation = diagnostics.pipe(getPagingOperation(context.program, operation)); + + if (basic.response.type?.__raw?.kind !== "Model" || !pagingOperation) { + diagnostics.add( + createDiagnostic({ + code: "unexpected-pageable-operation-return-type", + target: operation, + format: { + operationName: operation.name, + }, + }), + ); + // return as page method with no paging info + return diagnostics.wrap({ + ...basic, + kind: "paging", + }); + } + + basic.response.resultPath = getPropertyPathFromModel( + context, + basic.response.type?.__raw, + (p) => p === pagingOperation.output.pageItems.property, + ); + const nextLinkPath = pagingOperation.output.nextLink + ? getPropertyPathFromModel( + context, + basic.response.type?.__raw, + (p) => p === pagingOperation.output.nextLink!.property, + ) + : undefined; + + context.__pagedResultSet.add(basic.response.type); + // tcgc will let all paging method return a list of items basic.response.type = diagnostics.pipe( - getClientTypeWithDiagnostics(context, pagedMetadata.itemsProperty.type), + getClientTypeWithDiagnostics(context, pagingOperation?.output.pageItems.property.type), + ); + + return diagnostics.wrap({ + ...basic, + kind: "paging", + nextLinkPath, + }); + } + + // azure core paging + const pagedMetadata = getPagedResult(context.program, operation)!; + + if (basic.response.type?.__raw?.kind !== "Model" || !pagedMetadata.itemsProperty) { + diagnostics.add( + createDiagnostic({ + code: "unexpected-pageable-operation-return-type", + target: operation, + format: { + operationName: operation.name, + }, + }), ); + // return as page method with no paging info + return diagnostics.wrap({ + ...basic, + kind: "paging", + }); } - basic.response.resultPath = getPathFromSegment( + + context.__pagedResultSet.add(basic.response.type); + + // tcgc will let all paging method return a list of items + basic.response.type = diagnostics.pipe( + getClientTypeWithDiagnostics(context, pagedMetadata.itemsProperty.type), + ); + + basic.response.resultPath = getPropertyPathFromSegment( context, pagedMetadata.modelType, pagedMetadata.itemsSegments, ); + return diagnostics.wrap({ ...basic, __raw_paged_metadata: pagedMetadata, kind: "paging", - nextLinkPath: getPathFromSegment( + nextLinkPath: getPropertyPathFromSegment( context, pagedMetadata.modelType, pagedMetadata?.nextLinkSegments, @@ -167,7 +241,45 @@ function getSdkPagingServiceMethod boolean, +): string | undefined { + const queue: { model: Model; path: ModelProperty[] }[] = []; + + for (const prop of model.properties.values()) { + if (predicate(prop)) { + return getLibraryName(context, prop); + } + if (prop.type.kind === "Model") { + queue.push({ model: prop.type, path: [prop] }); + } + } + + while (queue.length > 0) { + const { model, path } = queue.shift()!; + for (const prop of model.properties.values()) { + if (predicate(prop)) { + return path + .concat(prop) + .map((s) => getLibraryName(context, s)) + .join("."); + } + if (prop.type.kind === "Model") { + queue.push({ model: prop.type, path: path.concat(prop) }); + } + } + } + + return undefined; +} + +function getPropertyPathFromSegment( + context: TCGCContext, + type: Model, + segments?: string[], +): string { if (!segments || segments.length === 0) { return ""; } @@ -393,7 +505,7 @@ function getSdkServiceMethod( client: SdkClientType, ): [SdkServiceMethod, readonly Diagnostic[]] { const lro = getLroMetadata(context.program, operation); - const paging = getPagedResult(context.program, operation); + const paging = getPagedResult(context.program, operation) || isList(context.program, operation); if (lro && paging) { return getSdkLroPagingServiceMethod(context, operation, client); } else if (paging) { diff --git a/packages/typespec-client-generator-core/src/public-utils.ts b/packages/typespec-client-generator-core/src/public-utils.ts index c32fcfcbb5..3d100ac2c8 100644 --- a/packages/typespec-client-generator-core/src/public-utils.ts +++ b/packages/typespec-client-generator-core/src/public-utils.ts @@ -1,4 +1,3 @@ -import { getPagedResult } from "@azure-tools/typespec-azure-core"; import { Diagnostic, Enum, @@ -15,6 +14,7 @@ import { getFriendlyName, getNamespaceFullName, getProjectedName, + getVisibility, ignoreDiagnostics, listServices, resolveEncodedName, @@ -113,7 +113,11 @@ export function getEffectivePayloadType(context: TCGCContext, type: Model): Mode return type; } - const effective = getEffectiveModelType(program, type, (t) => !isMetadata(context.program, t)); + const effective = getEffectiveModelType( + program, + type, + (t) => !isMetadata(context.program, t) && !getVisibility(context.program, t)?.includes("none"), // eslint-disable-line @typescript-eslint/no-deprecated + ); if (effective.name) { return effective; } @@ -691,9 +695,5 @@ export function isAzureCoreModel(t: SdkType): boolean { * @returns */ export function isPagedResultModel(context: TCGCContext, t: SdkType): boolean { - return ( - t.__raw !== undefined && - t.__raw.kind === "Model" && - getPagedResult(context.program, t.__raw) !== undefined - ); + return context.__pagedResultSet.has(t); } diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 9d3ae45afb..923cb76a53 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -1332,6 +1332,7 @@ function addPropertiesToModelType( if ( isStatusCode(context.program, property) || isNeverOrVoidType(property.type) || + getVisibility(context.program, property)?.includes("none") || // eslint-disable-line @typescript-eslint/no-deprecated sdkType.kind !== "model" ) { continue; diff --git a/packages/typespec-client-generator-core/test/packages/paged-operation.test.ts b/packages/typespec-client-generator-core/test/packages/paged-operation.test.ts index 237e667b36..018ddf8c24 100644 --- a/packages/typespec-client-generator-core/test/packages/paged-operation.test.ts +++ b/packages/typespec-client-generator-core/test/packages/paged-operation.test.ts @@ -1,6 +1,8 @@ import { AzureCoreTestLibrary } from "@azure-tools/typespec-azure-core/testing"; +import { Model, ModelProperty } from "@typespec/compiler"; import { strictEqual } from "assert"; import { beforeEach, describe, it } from "vitest"; +import { getPropertyPathFromModel } from "../../src/package.js"; import { SdkTestRunner, createSdkTestRunner } from "../test-host.js"; import { getServiceMethodOfClient } from "./utils.js"; @@ -15,33 +17,139 @@ describe("typespec-client-generator-core: paged operation", () => { }); }); - it("paged result with encoded name", async () => { - await runner.compile(`@server("http://localhost:3000", "endpoint") - @service({}) - namespace My.Service; - op test(): ListTestResult; - @pagedResult - model ListTestResult { - @items - @clientName("values") - tests: Test[]; - @Azure.Core.nextLink - @clientName("nextLink") - next: string; - } - model Test { - id: string; - } - `); + it("azure paged result with encoded name", async () => { + await runner.compileWithBuiltInService(` + op test(): ListTestResult; + @pagedResult + model ListTestResult { + @items + @clientName("values") + tests: Test[]; + @Azure.Core.nextLink + @clientName("nextLink") + next: string; + } + model Test { + id: string; + } + `); const sdkPackage = runner.context.sdkPackage; const method = getServiceMethodOfClient(sdkPackage); strictEqual(method.name, "test"); strictEqual(method.kind, "paging"); - strictEqual(method.crossLanguageDefintionId, "My.Service.test"); strictEqual(method.nextLinkPath, "nextLink"); const response = method.response; strictEqual(response.kind, "method"); strictEqual(response.resultPath, "values"); }); + + it("normal paged result", async () => { + await runner.compileWithBuiltInService(` + @list + op test(): ListTestResult; + model ListTestResult { + @pageItems + tests: Test[]; + @TypeSpec.nextLink + next: string; + } + model Test { + id: string; + } + `); + const sdkPackage = runner.context.sdkPackage; + const method = getServiceMethodOfClient(sdkPackage); + strictEqual(method.name, "test"); + strictEqual(method.kind, "paging"); + strictEqual(method.nextLinkPath, "next"); + + const response = method.response; + strictEqual(response.kind, "method"); + strictEqual(response.resultPath, "tests"); + }); + + it("normal paged result with encoded name", async () => { + await runner.compileWithBuiltInService(` + @list + op test(): ListTestResult; + model ListTestResult { + @pageItems + @clientName("values") + tests: Test[]; + @TypeSpec.nextLink + @clientName("nextLink") + next: string; + } + model Test { + id: string; + } + `); + const sdkPackage = runner.context.sdkPackage; + const method = getServiceMethodOfClient(sdkPackage); + strictEqual(method.name, "test"); + strictEqual(method.kind, "paging"); + strictEqual(method.nextLinkPath, "nextLink"); + + const response = method.response; + strictEqual(response.kind, "method"); + strictEqual(response.resultPath, "values"); + }); + + // skip for current paging implementation does not support nested paging value + it.skip("normal paged result with nested paging value", async () => { + await runner.compileWithBuiltInService(` + @list + op test(): ListTestResult; + model ListTestResult { + results: { + @pageItems + values: Test[]; + }; + pagination: { + @TypeSpec.nextLink + nextLink: string; + }; + } + model Test { + id: string; + } + `); + const sdkPackage = runner.context.sdkPackage; + const method = getServiceMethodOfClient(sdkPackage); + strictEqual(method.name, "test"); + strictEqual(method.kind, "paging"); + strictEqual(method.nextLinkPath, "pagination.nextLink"); + + const response = method.response; + strictEqual(response.kind, "method"); + strictEqual(response.resultPath, "results.values"); + }); + + it("getPropertyPathFromModel test for nested case", async () => { + const { Test, a, d } = (await runner.compileWithBuiltInService(` + op test(): Test; + @test + model Test { + a: { + b: { + @test + a: string; + }; + }; + b: { + @test + d: string; + }; + } + `)) as { Test: Model; a: ModelProperty; d: ModelProperty }; + strictEqual( + getPropertyPathFromModel(runner.context, Test, (x: any) => x === a), + "a.b.a", + ); + strictEqual( + getPropertyPathFromModel(runner.context, Test, (x: any) => x === d), + "b.d", + ); + }); }); diff --git a/packages/typespec-client-generator-core/test/packages/parameters.test.ts b/packages/typespec-client-generator-core/test/packages/parameters.test.ts index 7ce538e6b0..42705c2284 100644 --- a/packages/typespec-client-generator-core/test/packages/parameters.test.ts +++ b/packages/typespec-client-generator-core/test/packages/parameters.test.ts @@ -1103,6 +1103,17 @@ describe("typespec-client-generator-core: parameters", () => { strictEqual(method.operation.uriTemplate, "/test"); }); + it("normal case with different wire name", async () => { + await runner.compileWithBuiltInService(` + @autoRoute + op test(@path("param-wire") param: string): void; + `); + const sdkPackage = runner.context.sdkPackage; + const method = getServiceMethodOfClient(sdkPackage); + strictEqual(method.parameters.length, 1); + strictEqual(method.operation.parameters.length, 1); + }); + it("singleton resource", async () => { const runnerWithArm = await createSdkTestRunner({ librariesToAdd: [AzureResourceManagerTestLibrary, AzureCoreTestLibrary, OpenAPITestLibrary], diff --git a/packages/typespec-client-generator-core/test/packages/responses.test.ts b/packages/typespec-client-generator-core/test/packages/responses.test.ts index f08b89706f..a74c4ced6b 100644 --- a/packages/typespec-client-generator-core/test/packages/responses.test.ts +++ b/packages/typespec-client-generator-core/test/packages/responses.test.ts @@ -1,5 +1,6 @@ import { deepStrictEqual, ok, strictEqual } from "assert"; import { beforeEach, describe, it } from "vitest"; +import { SdkHttpOperation, SdkServiceMethod } from "../../src/interfaces.js"; import { SdkTestRunner, createSdkTestRunner } from "../test-host.js"; import { getServiceMethodOfClient } from "./utils.js"; @@ -279,4 +280,23 @@ describe("typespec-client-generator-core: responses", () => { strictEqual(method.response.type?.kind, "model"); strictEqual(method.response.type.usage, 0); }); + + it("response model with property with none visibility", async function () { + await runner.compileWithBuiltInService(` + model Test{ + prop: string; + @visibility("none") + nonProp: string; + } + op get(): Test; + `); + const sdkPackage = runner.context.sdkPackage; + const models = sdkPackage.models; + strictEqual(models.length, 1); + strictEqual(models[0].properties.length, 1); + strictEqual( + (sdkPackage.clients[0].methods[0] as SdkServiceMethod).response.type, + models[0], + ); + }); }); 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 f52dbda182..eb3267aca6 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 @@ -1611,4 +1611,19 @@ describe("typespec-client-generator-core: model types", () => { strictEqual(p.multipartOptions, undefined); } }); + + it("remove property with none visibility", async function () { + await runner.compileWithBuiltInService(` + model Test{ + prop: string; + @visibility("none") + nonProp: string; + } + @post + op do(@body body: Test): void; + `); + const models = runner.context.sdkPackage.models; + strictEqual(models.length, 1); + strictEqual(models[0].properties.length, 1); + }); });