From 9fb60d69f0df191bb609713f045670f2e3c078fb Mon Sep 17 00:00:00 2001 From: tadelesh Date: Thu, 31 Oct 2024 18:01:53 +0800 Subject: [PATCH 1/5] remove useless path parameter from method --- .../fix_route_param-2024-9-31-18-1-39.md | 7 +++ .../src/http.ts | 26 ++++++++--- .../test/packages/parameters.test.ts | 44 +++++++++++++++++++ 3 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 .chronus/changes/fix_route_param-2024-9-31-18-1-39.md diff --git a/.chronus/changes/fix_route_param-2024-9-31-18-1-39.md b/.chronus/changes/fix_route_param-2024-9-31-18-1-39.md new file mode 100644 index 0000000000..d4ff2a14ef --- /dev/null +++ b/.chronus/changes/fix_route_param-2024-9-31-18-1-39.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +remove useless path parameter from method \ No newline at end of file diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index 05f37afa6a..a49bae7992 100644 --- a/packages/typespec-client-generator-core/src/http.ts +++ b/packages/typespec-client-generator-core/src/http.ts @@ -85,6 +85,7 @@ export function getSdkHttpOperation( const parameters = diagnostics.pipe( getSdkHttpParameters(context, httpOperation, methodParameters, responsesWithBodies[0]), ); + filterOutUselessPathParameters(context, httpOperation, methodParameters); return diagnostics.wrap({ __raw: httpOperation, kind: "http", @@ -408,12 +409,12 @@ function getSdkHttpResponseAndExceptions( context: TCGCContext, httpOperation: HttpOperation, ): [ - { - responses: SdkHttpResponse[]; - exceptions: SdkHttpErrorResponse[]; - }, - readonly Diagnostic[], -] { + { + responses: SdkHttpResponse[]; + exceptions: SdkHttpErrorResponse[]; + }, + readonly Diagnostic[], + ] { const diagnostics = createDiagnosticCollector(); const responses: SdkHttpResponse[] = []; const exceptions: SdkHttpErrorResponse[] = []; @@ -628,6 +629,19 @@ function findMapping( return undefined; } +function filterOutUselessPathParameters(context: TCGCContext, httpOperation: HttpOperation, methodParameters: SdkMethodParameter[]) { + // for autoroute with constant or singleton arm resource operation, + // some path parameters will be added to route directly with the constant value, + // so we will remove the method parameter for consistent + for (let i = 0; i < methodParameters.length; i++) { + const param = methodParameters[i]; + if (param.__raw && isPathParam(context.program, param.__raw) && httpOperation.parameters.parameters.filter(p => p.type === "path" && p.name === getWireName(context, param.__raw!)).length === 0) { + methodParameters.splice(i, 1); + i--; + } + } +} + function findRootSourceProperty(property: ModelProperty): ModelProperty { while (property.sourceProperty) { property = property.sourceProperty; 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 c2e64e935e..c7881bbdad 100644 --- a/packages/typespec-client-generator-core/test/packages/parameters.test.ts +++ b/packages/typespec-client-generator-core/test/packages/parameters.test.ts @@ -10,6 +10,8 @@ import { } from "../../src/interfaces.js"; import { SdkTestRunner, createSdkTestRunner } from "../test-host.js"; import { getServiceMethodOfClient, getServiceWithDefaultApiVersion } from "./utils.js"; +import { AzureResourceManagerTestLibrary } from "@azure-tools/typespec-azure-resource-manager/testing"; +import { OpenAPITestLibrary } from "@typespec/openapi/testing"; describe("typespec-client-generator-core: parameters", () => { let runner: SdkTestRunner; @@ -1087,4 +1089,46 @@ describe("typespec-client-generator-core: parameters", () => { strictEqual(method.operation.bodyParam?.serializedName, "body"); }); }); + + describe("method parameter not used in operation", () => { + it("autoroute with constant", async () => { + await runner.compileWithBuiltInService(` + @autoRoute + op test(@path param: "test"): void; + `); + const sdkPackage = runner.context.sdkPackage; + const method = getServiceMethodOfClient(sdkPackage); + strictEqual(method.parameters.length, 0); + strictEqual(method.operation.parameters.length, 0); + strictEqual(method.operation.uriTemplate, "/test"); + }); + + it("singleton resource", async () => { + const runnerWithArm = await createSdkTestRunner({ + librariesToAdd: [AzureResourceManagerTestLibrary, AzureCoreTestLibrary, OpenAPITestLibrary], + autoUsings: ["Azure.ResourceManager", "Azure.Core"], + emitterName: "@azure-tools/typespec-java", + }); + await runnerWithArm.compileWithBuiltInAzureResourceManagerService(` + @singleton("default") + model SingletonTrackedResource is TrackedResource { + ...ResourceNameParameter; + } + + model SingletonTrackedResourceProperties { + description?: string; + } + + @armResourceOperations + interface Singleton { + createOrUpdate is ArmResourceCreateOrReplaceAsync; + } + `); + + const sdkPackage = runnerWithArm.context.sdkPackage; + const method = getServiceMethodOfClient(sdkPackage); + deepStrictEqual(method.parameters.map(p => p.name), ["resourceGroupName", "resource", "contentType", "accept"]); + deepStrictEqual(method.operation.parameters.map(p => p.name), ["apiVersion", "subscriptionId", "resourceGroupName", "contentType", "accept"]); + }); + }); }); From bfab50e6ba99d4e8cc097eb20324a48f8fc7a482 Mon Sep 17 00:00:00 2001 From: tadelesh Date: Thu, 31 Oct 2024 18:26:45 +0800 Subject: [PATCH 2/5] format --- .../src/http.ts | 20 +++++++++++++++---- .../test/package.test.ts | 2 +- .../test/packages/parameters.test.ts | 14 +++++++++---- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index a49bae7992..1eff281ae5 100644 --- a/packages/typespec-client-generator-core/src/http.ts +++ b/packages/typespec-client-generator-core/src/http.ts @@ -629,13 +629,25 @@ function findMapping( return undefined; } -function filterOutUselessPathParameters(context: TCGCContext, httpOperation: HttpOperation, methodParameters: SdkMethodParameter[]) { - // for autoroute with constant or singleton arm resource operation, - // some path parameters will be added to route directly with the constant value, +function filterOutUselessPathParameters( + context: TCGCContext, + httpOperation: HttpOperation, + methodParameters: SdkMethodParameter[], +) { + // there are some cases that method path parameter is not in operation: + // 1. autoroute with constant parameter + // 2. singleton arm resource name + // 3. visibility mis-match // so we will remove the method parameter for consistent for (let i = 0; i < methodParameters.length; i++) { const param = methodParameters[i]; - if (param.__raw && isPathParam(context.program, param.__raw) && httpOperation.parameters.parameters.filter(p => p.type === "path" && p.name === getWireName(context, param.__raw!)).length === 0) { + if ( + param.__raw && + isPathParam(context.program, param.__raw) && + httpOperation.parameters.parameters.filter( + (p) => p.type === "path" && p.name === getWireName(context, param.__raw!), + ).length === 0 + ) { methodParameters.splice(i, 1); i--; } diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index 7d36eb30b7..92f63a6d71 100644 --- a/packages/typespec-client-generator-core/test/package.test.ts +++ b/packages/typespec-client-generator-core/test/package.test.ts @@ -349,7 +349,7 @@ describe("typespec-client-generator-core: package", () => { const method = getServiceMethodOfClient(sdkPackage); strictEqual(method.name, "create"); strictEqual(method.kind, "basic"); - strictEqual(method.parameters.length, 5); + strictEqual(method.parameters.length, 4); deepStrictEqual( method.parameters.map((x) => x.name), ["id", "weight", "color", "contentType", "accept"], 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 c7881bbdad..7ce538e6b0 100644 --- a/packages/typespec-client-generator-core/test/packages/parameters.test.ts +++ b/packages/typespec-client-generator-core/test/packages/parameters.test.ts @@ -1,4 +1,6 @@ import { AzureCoreTestLibrary } from "@azure-tools/typespec-azure-core/testing"; +import { AzureResourceManagerTestLibrary } from "@azure-tools/typespec-azure-resource-manager/testing"; +import { OpenAPITestLibrary } from "@typespec/openapi/testing"; import { deepStrictEqual, ok, strictEqual } from "assert"; import { beforeEach, describe, it } from "vitest"; import { @@ -10,8 +12,6 @@ import { } from "../../src/interfaces.js"; import { SdkTestRunner, createSdkTestRunner } from "../test-host.js"; import { getServiceMethodOfClient, getServiceWithDefaultApiVersion } from "./utils.js"; -import { AzureResourceManagerTestLibrary } from "@azure-tools/typespec-azure-resource-manager/testing"; -import { OpenAPITestLibrary } from "@typespec/openapi/testing"; describe("typespec-client-generator-core: parameters", () => { let runner: SdkTestRunner; @@ -1127,8 +1127,14 @@ describe("typespec-client-generator-core: parameters", () => { const sdkPackage = runnerWithArm.context.sdkPackage; const method = getServiceMethodOfClient(sdkPackage); - deepStrictEqual(method.parameters.map(p => p.name), ["resourceGroupName", "resource", "contentType", "accept"]); - deepStrictEqual(method.operation.parameters.map(p => p.name), ["apiVersion", "subscriptionId", "resourceGroupName", "contentType", "accept"]); + deepStrictEqual( + method.parameters.map((p) => p.name), + ["resourceGroupName", "resource", "contentType", "accept"], + ); + deepStrictEqual( + method.operation.parameters.map((p) => p.name), + ["apiVersion", "subscriptionId", "resourceGroupName", "contentType", "accept"], + ); }); }); }); From 29debd5a85bf6149a1ff3310f8b4e7f7b23494b7 Mon Sep 17 00:00:00 2001 From: tadelesh Date: Thu, 31 Oct 2024 18:39:40 +0800 Subject: [PATCH 3/5] changeset --- .chronus/changes/fix_route_param-2024-9-31-18-1-39.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chronus/changes/fix_route_param-2024-9-31-18-1-39.md b/.chronus/changes/fix_route_param-2024-9-31-18-1-39.md index d4ff2a14ef..95a617fe4b 100644 --- a/.chronus/changes/fix_route_param-2024-9-31-18-1-39.md +++ b/.chronus/changes/fix_route_param-2024-9-31-18-1-39.md @@ -4,4 +4,4 @@ packages: - "@azure-tools/typespec-client-generator-core" --- -remove useless path parameter from method \ No newline at end of file +remove unused path parameter from method From 683de0b54eb5a46b594b5a1200cec81b443ba4dc Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Thu, 31 Oct 2024 11:08:11 -0400 Subject: [PATCH 4/5] fix test --- packages/typespec-client-generator-core/test/package.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index 92f63a6d71..3235098db8 100644 --- a/packages/typespec-client-generator-core/test/package.test.ts +++ b/packages/typespec-client-generator-core/test/package.test.ts @@ -352,7 +352,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(method.parameters.length, 4); deepStrictEqual( method.parameters.map((x) => x.name), - ["id", "weight", "color", "contentType", "accept"], + ["weight", "color", "contentType", "accept"], ); const bodyParameter = method.operation.bodyParam; From a24ce208d8f80129e8090c2fb16fbc8439a0c601 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Thu, 31 Oct 2024 11:13:58 -0400 Subject: [PATCH 5/5] format --- packages/typespec-client-generator-core/src/http.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index 1eff281ae5..ba5002b7b3 100644 --- a/packages/typespec-client-generator-core/src/http.ts +++ b/packages/typespec-client-generator-core/src/http.ts @@ -409,12 +409,12 @@ function getSdkHttpResponseAndExceptions( context: TCGCContext, httpOperation: HttpOperation, ): [ - { - responses: SdkHttpResponse[]; - exceptions: SdkHttpErrorResponse[]; - }, - readonly Diagnostic[], - ] { + { + responses: SdkHttpResponse[]; + exceptions: SdkHttpErrorResponse[]; + }, + readonly Diagnostic[], +] { const diagnostics = createDiagnosticCollector(); const responses: SdkHttpResponse[] = []; const exceptions: SdkHttpErrorResponse[] = [];