From 3e7122b3e5b2e4213c801d0a90dfaae97f03b6df Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Sat, 10 Oct 2026 14:43:08 +0800 Subject: [PATCH 01/17] Add ARM use-update-for-patch lint rule Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bf3420c-d859-4ac1-81e8-2081d1fded84 --- .chronus/changes/arm-use-update-for-patch.md | 7 ++ .../changes/register-use-update-for-patch.md | 7 ++ .../typespec-azure-resource-manager/README.md | 1 + .../src/linter.ts | 2 + .../src/rules/use-update-for-patch.md | 81 +++++++++++++++++ .../src/rules/use-update-for-patch.ts | 32 +++++++ .../test/rules/use-update-for-patch.test.ts | 86 +++++++++++++++++++ .../src/rulesets/resource-manager.ts | 1 + .../content/docs/docs/howtos/ARM/arm-rules.md | 1 + .../reference/linter.md | 1 + 10 files changed, 219 insertions(+) create mode 100644 .chronus/changes/arm-use-update-for-patch.md create mode 100644 .chronus/changes/register-use-update-for-patch.md create mode 100644 packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.md create mode 100644 packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.ts create mode 100644 packages/typespec-azure-resource-manager/test/rules/use-update-for-patch.test.ts diff --git a/.chronus/changes/arm-use-update-for-patch.md b/.chronus/changes/arm-use-update-for-patch.md new file mode 100644 index 0000000000..8924fa54ef --- /dev/null +++ b/.chronus/changes/arm-use-update-for-patch.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-azure-resource-manager" +--- + +Add the `use-update-for-patch` lint rule to require authored ARM PATCH operation names to start with `update`. diff --git a/.chronus/changes/register-use-update-for-patch.md b/.chronus/changes/register-use-update-for-patch.md new file mode 100644 index 0000000000..e4b187829f --- /dev/null +++ b/.chronus/changes/register-use-update-for-patch.md @@ -0,0 +1,7 @@ +--- +changeKind: internal +packages: + - "@azure-tools/typespec-azure-rulesets" +--- + +Register the ARM `use-update-for-patch` lint rule as disabled in the resource manager ruleset. diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index 45af72fa73..5143700db5 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -76,6 +76,7 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/retry-after`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/retry-after) | Check if retry-after header appears in response body. | | [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/unsupported-type) | Check for unsupported ARM types. | | [`@azure-tools/typespec-azure-resource-manager/use-model-request-body`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-model-request-body) | Request bodies must use plain models. | +| [`@azure-tools/typespec-azure-resource-manager/use-update-for-patch`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-update-for-patch) | ARM PATCH operation names should start with 'update'. | | [`@azure-tools/typespec-azure-resource-manager/secret-prop`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/secret-prop) | RPC-v1-13: Check that property with names indicating sensitive information(e.g. contains auth, password, token, secret, etc.) are marked with @secret decorator. | | [`@azure-tools/typespec-azure-resource-manager/no-empty-model`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-empty-model) | ARM Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience. | | [`@azure-tools/typespec-azure-resource-manager/no-reserved-resource-property`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-reserved-resource-property) | Reserved property names (for example 'billingData') must not be present in a resource's property bag. The property name is matched case-insensitively. | diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index 4333e5357a..486bfd9dde 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -51,6 +51,7 @@ import { useInterfaceRule } from "./rules/use-interface.js"; import { useModelRequestBodyRule } from "./rules/use-model-request-body.js"; import { useOperationDecoratorRule } from "./rules/use-operation-decorator.js"; import { useRelationshipRequiredPropertiesRule } from "./rules/use-relationship-required-properties.js"; +import { useUpdateForPatchRule } from "./rules/use-update-for-patch.js"; import { versionProgressionRule } from "./rules/version-progression.js"; const rules = [ @@ -102,6 +103,7 @@ const rules = [ retryAfterRule, unsupportedTypeRule, useModelRequestBodyRule, + useUpdateForPatchRule, secretProprule, noEmptyModel, noReservedResourcePropertyRule, diff --git a/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.md b/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.md new file mode 100644 index 0000000000..bbb53048f6 --- /dev/null +++ b/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.md @@ -0,0 +1,81 @@ +ARM HTTP PATCH operation names should start with `update`, ignoring case. Use names such as +`update`, `updateTags`, or `updateWidget` instead of `patch`, `modify`, or `create`. +This check applies to concrete HTTP service endpoints regardless of their response status codes. + +Consistent partial-update names make ARM APIs easier to discover and give generated SDKs a +predictable operation naming convention. The check uses the authored operation name, not an SDK +name override or the containing interface name. + +## Incorrect + +```tsp +@armProviderNamespace +@service +@armCommonTypesVersion(CommonTypes.Versions.v5) +namespace Microsoft.Contoso; + +model Widget is TrackedResource { + @key("widgetName") + @segment("widgets") + name: string; +} + +model WidgetProperties { + provisioningState?: ResourceProvisioningState; +} + +@armResourceOperations +interface Widgets { + modify is ArmResourcePatchSync; +} +``` + +## Correct + +```tsp +@armProviderNamespace +@service +@armCommonTypesVersion(CommonTypes.Versions.v5) +namespace Microsoft.Contoso; + +model Widget is TrackedResource { + @key("widgetName") + @segment("widgets") + name: string; +} + +model WidgetProperties { + provisioningState?: ResourceProvisioningState; +} + +@armResourceOperations +interface Widgets { + update is ArmResourcePatchSync; +} +``` + +The examples assume the usual ARM library imports and `using` declarations. + +## Impact + +**API, SDK.** Inconsistent partial-update operation names make APIs harder to discover. Renaming +an already shipped operation may break existing SDK callers. + +## Suppression + +Suppress this warning when compatibility requires preserving a shipped operation name. Prefer +an `update` prefix for new operations. + +```tsp +@armResourceOperations +interface Widgets { + #suppress "@azure-tools/typespec-azure-resource-manager/use-update-for-patch" "Preserve the shipped SDK operation name." + modify is ArmResourcePatchSync; +} +``` + +## LintDiff Equivalent + +This rule corresponds to +[`PatchInOperationName`](https://github.com/Azure/azure-openapi-validator/blob/main/docs/patch-in-operation-name.md). +It enforces authored ARM endpoint names rather than reproducing emitted-operation-ID exemptions. diff --git a/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.ts b/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.ts new file mode 100644 index 0000000000..6fad958872 --- /dev/null +++ b/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.ts @@ -0,0 +1,32 @@ +import { createRule, fileRef, paramMessage } from "@typespec/compiler"; +import { getAllHttpServices } from "@typespec/http"; + +export const useUpdateForPatchRule = createRule({ + name: "use-update-for-patch", + docs: fileRef.fromPackageRoot("src/rules/use-update-for-patch.md"), + description: "ARM PATCH operation names should start with 'update'.", + url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-update-for-patch", + severity: "warning", + messages: { + default: paramMessage`ARM PATCH operation '${"operationName"}' should start with 'update'.`, + }, + create(context) { + return { + root: () => { + const [services] = getAllHttpServices(context.program); + for (const service of services) { + for (const { operation, verb } of service.operations) { + if (verb !== "patch" || operation.name.toLowerCase().startsWith("update")) { + continue; + } + + context.reportDiagnostic({ + target: operation, + format: { operationName: operation.name }, + }); + } + } + }, + }; + }, +}); diff --git a/packages/typespec-azure-resource-manager/test/rules/use-update-for-patch.test.ts b/packages/typespec-azure-resource-manager/test/rules/use-update-for-patch.test.ts new file mode 100644 index 0000000000..a749eee115 --- /dev/null +++ b/packages/typespec-azure-resource-manager/test/rules/use-update-for-patch.test.ts @@ -0,0 +1,86 @@ +import { Tester } from "#test/tester.js"; +import { createLinterRuleTester, type LinterRuleTester } from "@typespec/compiler/testing"; +import { readFile } from "node:fs/promises"; +import { beforeEach, it } from "vitest"; +import { useUpdateForPatchRule } from "../../src/rules/use-update-for-patch.js"; + +let tester: LinterRuleTester; + +beforeEach(async () => { + tester = createLinterRuleTester( + await Tester.createInstance(), + useUpdateForPatchRule, + "@azure-tools/typespec-azure-resource-manager", + ); +}); + +const header = "@service namespace Microsoft.Contoso;"; + +function diagnostic(name: string) { + return { + code: "@azure-tools/typespec-azure-resource-manager/use-update-for-patch", + message: `ARM PATCH operation '${name}' should start with 'update'.`, + }; +} + +it.each(["patch", "modify", "create", "patch_Update"])("rejects PATCH name %s", async (name) => { + await tester + .expect(`${header} @patch @route("/item") op ${name}(): string;`) + .toEmitDiagnostics([diagnostic(name)]); +}); + +it.each(["update", "updateWidget", "UpdateTags"])("accepts update prefix %s", async (name) => { + await tester.expect(`${header} @patch @route("/item") op ${name}(): string;`).toBeValid(); +}); + +it.each([200, 201, 202, 204])("checks PATCH names for response %s", async (status) => { + await tester + .expect(`${header} @patch @route("/item") op modify(): { @statusCode code: ${status}; };`) + .toEmitDiagnostics([diagnostic("modify")]); +}); + +it("ignores non-PATCH verbs", async () => { + await tester.expect(`${header} @get @route("/item") op modify(): string;`).toBeValid(); +}); + +it("does not let an interface name mask the operation prefix", async () => { + await tester + .expect(`${header} @route("/item") interface UpdateWidgets { @patch op modify(): string; }`) + .toEmitDiagnostics([diagnostic("modify")]); +}); + +it("reports distinct offending operation names without a provider decorator", async () => { + await tester + .expect( + ` + ${header} + @patch @route("/first") op modifyFirst(): string; + @patch @route("/second") op modifySecond(): string; + `, + ) + .toEmitDiagnostics([diagnostic("modifyFirst"), diagnostic("modifySecond")]); +}); + +it("validates the exact incorrect documentation example", async () => { + const source = await readFile( + new URL("../../src/rules/use-update-for-patch.md", import.meta.url), + "utf8", + ); + const snippet = source + .replace(/\r\n/g, "\n") + .split("## Incorrect")[1] + .match(/```tsp\n([\s\S]*?)```/)![1]; + await tester.expect(snippet).toEmitDiagnostics([diagnostic("modify")]); +}); + +it("validates the exact correct documentation example", async () => { + const source = await readFile( + new URL("../../src/rules/use-update-for-patch.md", import.meta.url), + "utf8", + ); + const snippet = source + .replace(/\r\n/g, "\n") + .split("## Correct")[1] + .match(/```tsp\n([\s\S]*?)```/)![1]; + await tester.expect(snippet).toBeValid(); +}); diff --git a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts index b04bddedb8..c55f8bca64 100644 --- a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts +++ b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts @@ -109,6 +109,7 @@ export default { "@azure-tools/typespec-azure-resource-manager/unsupported-type": true, "@azure-tools/typespec-azure-resource-manager/no-query-in-point-op": false, "@azure-tools/typespec-azure-resource-manager/use-model-request-body": false, + "@azure-tools/typespec-azure-resource-manager/use-update-for-patch": false, // TCGC rules "@azure-tools/typespec-client-generator-core/require-client-suffix": true, diff --git a/website/src/content/docs/docs/howtos/ARM/arm-rules.md b/website/src/content/docs/docs/howtos/ARM/arm-rules.md index 0f8253d4d2..bff583dc3a 100644 --- a/website/src/content/docs/docs/howtos/ARM/arm-rules.md +++ b/website/src/content/docs/docs/howtos/ARM/arm-rules.md @@ -145,6 +145,7 @@ The tables below provide guidance to rule authors and ARM reviewers on how to ev | [`use-interface`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-interface/) | — | **API, SDK, Emitters.** Resource operations outside interfaces can break ARM resource-operation modeling and downstream tooling assumptions. | | [`use-model-request-body`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-model-request-body/) | [`ParametersSchemaAsTypeObject`](https://github.com/Azure/azure-openapi-validator/blob/main/docs/parameters-schema-as-type-object.md) | **API, SDK.** Non-model, array, and record request bodies cannot evolve with additional properties without a breaking change. | | [`use-operation-decorator`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-operation-decorator/) | — | **API, SDK, Emitters.** Missing or mismatched decorators prevent resource operations from being associated with the correct resource, which can break SDK generation and resource-aware tooling. | +| [`use-update-for-patch`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-update-for-patch/) | [`PatchInOperationName`](https://github.com/Azure/azure-openapi-validator/blob/main/docs/patch-in-operation-name.md) | **API, SDK.** Inconsistent authored PATCH names make partial-update operations harder to discover; renaming shipped operations may break SDK callers. | | [`version-progression`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/version-progression/) | — | **API, SDK, Tooling.** Out-of-order versions make specs unmaintainable, and versions with matching dates cause SDK problems. | ### Rules in @azure-tools/typespec-client-generator-core diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md b/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md index 7371c440d3..b2a866492c 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md @@ -70,6 +70,7 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/retry-after`](../rules/retry-after.md) | Check if retry-after header appears in response body. | | [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](../rules/unsupported-type.md) | Check for unsupported ARM types. | | [`@azure-tools/typespec-azure-resource-manager/use-model-request-body`](../rules/use-model-request-body.md) | Request bodies must use plain models. | +| [`@azure-tools/typespec-azure-resource-manager/use-update-for-patch`](../rules/use-update-for-patch.md) | ARM PATCH operation names should start with 'update'. | | [`@azure-tools/typespec-azure-resource-manager/secret-prop`](../rules/secret-prop.md) | RPC-v1-13: Check that property with names indicating sensitive information(e.g. contains auth, password, token, secret, etc.) are marked with @secret decorator. | | [`@azure-tools/typespec-azure-resource-manager/no-empty-model`](../rules/no-empty-model.md) | ARM Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience. | | [`@azure-tools/typespec-azure-resource-manager/no-reserved-resource-property`](../rules/no-reserved-resource-property.md) | Reserved property names (for example 'billingData') must not be present in a resource's property bag. The property name is matched case-insensitively. | From 5d131d487ec0e1dd8f1bd0b6ff1050fe4df2f747 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Sat, 10 Oct 2026 15:30:23 +0800 Subject: [PATCH 02/17] Move PATCH SDK naming guidance to TCGC common names Follow the user-approved use-create-for-put pattern, keep client-sdk registration disabled, and remove the prior ARM introduction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bf3420c-d859-4ac1-81e8-2081d1fded84 --- .chronus/changes/arm-use-update-for-patch.md | 7 - .../changes/register-use-update-for-patch.md | 2 +- .chronus/changes/tcgc-use-update-for-patch.md | 7 + .../typespec-azure-resource-manager/README.md | 1 - .../src/linter.ts | 2 - .../src/rules/use-update-for-patch.md | 81 ----- .../src/rules/use-update-for-patch.ts | 32 -- .../test/rules/use-update-for-patch.test.ts | 86 ------ .../src/rulesets/client-sdk.ts | 1 + .../src/rulesets/resource-manager.ts | 1 - .../test/validate-rules-defined.test.ts | 14 + .../typespec-client-generator-core/README.md | 1 + .../src/linter.ts | 2 + .../src/rules/use-update-for-patch.md | 82 +++++ .../src/rules/use-update-for-patch.ts | 47 +++ .../test/rules/use-update-for-patch.test.ts | 290 ++++++++++++++++++ .../content/docs/docs/howtos/ARM/arm-rules.md | 2 +- .../reference/linter.md | 1 - .../reference/linter.md | 1 + 19 files changed, 447 insertions(+), 213 deletions(-) delete mode 100644 .chronus/changes/arm-use-update-for-patch.md create mode 100644 .chronus/changes/tcgc-use-update-for-patch.md delete mode 100644 packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.md delete mode 100644 packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.ts delete mode 100644 packages/typespec-azure-resource-manager/test/rules/use-update-for-patch.test.ts create mode 100644 packages/typespec-client-generator-core/src/rules/use-update-for-patch.md create mode 100644 packages/typespec-client-generator-core/src/rules/use-update-for-patch.ts create mode 100644 packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts diff --git a/.chronus/changes/arm-use-update-for-patch.md b/.chronus/changes/arm-use-update-for-patch.md deleted file mode 100644 index 8924fa54ef..0000000000 --- a/.chronus/changes/arm-use-update-for-patch.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -changeKind: feature -packages: - - "@azure-tools/typespec-azure-resource-manager" ---- - -Add the `use-update-for-patch` lint rule to require authored ARM PATCH operation names to start with `update`. diff --git a/.chronus/changes/register-use-update-for-patch.md b/.chronus/changes/register-use-update-for-patch.md index e4b187829f..02e638d907 100644 --- a/.chronus/changes/register-use-update-for-patch.md +++ b/.chronus/changes/register-use-update-for-patch.md @@ -4,4 +4,4 @@ packages: - "@azure-tools/typespec-azure-rulesets" --- -Register the ARM `use-update-for-patch` lint rule as disabled in the resource manager ruleset. +Register the TCGC `use-update-for-patch` lint rule as disabled in the client SDK ruleset. diff --git a/.chronus/changes/tcgc-use-update-for-patch.md b/.chronus/changes/tcgc-use-update-for-patch.md new file mode 100644 index 0000000000..9184a3032d --- /dev/null +++ b/.chronus/changes/tcgc-use-update-for-patch.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +Add the opt-in `use-update-for-patch` lint rule to require common SDK method names for ARM PATCH operations to start with `update`, honoring common client name overrides. diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index 5143700db5..45af72fa73 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -76,7 +76,6 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/retry-after`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/retry-after) | Check if retry-after header appears in response body. | | [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/unsupported-type) | Check for unsupported ARM types. | | [`@azure-tools/typespec-azure-resource-manager/use-model-request-body`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-model-request-body) | Request bodies must use plain models. | -| [`@azure-tools/typespec-azure-resource-manager/use-update-for-patch`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-update-for-patch) | ARM PATCH operation names should start with 'update'. | | [`@azure-tools/typespec-azure-resource-manager/secret-prop`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/secret-prop) | RPC-v1-13: Check that property with names indicating sensitive information(e.g. contains auth, password, token, secret, etc.) are marked with @secret decorator. | | [`@azure-tools/typespec-azure-resource-manager/no-empty-model`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-empty-model) | ARM Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience. | | [`@azure-tools/typespec-azure-resource-manager/no-reserved-resource-property`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-reserved-resource-property) | Reserved property names (for example 'billingData') must not be present in a resource's property bag. The property name is matched case-insensitively. | diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index 486bfd9dde..4333e5357a 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -51,7 +51,6 @@ import { useInterfaceRule } from "./rules/use-interface.js"; import { useModelRequestBodyRule } from "./rules/use-model-request-body.js"; import { useOperationDecoratorRule } from "./rules/use-operation-decorator.js"; import { useRelationshipRequiredPropertiesRule } from "./rules/use-relationship-required-properties.js"; -import { useUpdateForPatchRule } from "./rules/use-update-for-patch.js"; import { versionProgressionRule } from "./rules/version-progression.js"; const rules = [ @@ -103,7 +102,6 @@ const rules = [ retryAfterRule, unsupportedTypeRule, useModelRequestBodyRule, - useUpdateForPatchRule, secretProprule, noEmptyModel, noReservedResourcePropertyRule, diff --git a/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.md b/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.md deleted file mode 100644 index bbb53048f6..0000000000 --- a/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.md +++ /dev/null @@ -1,81 +0,0 @@ -ARM HTTP PATCH operation names should start with `update`, ignoring case. Use names such as -`update`, `updateTags`, or `updateWidget` instead of `patch`, `modify`, or `create`. -This check applies to concrete HTTP service endpoints regardless of their response status codes. - -Consistent partial-update names make ARM APIs easier to discover and give generated SDKs a -predictable operation naming convention. The check uses the authored operation name, not an SDK -name override or the containing interface name. - -## Incorrect - -```tsp -@armProviderNamespace -@service -@armCommonTypesVersion(CommonTypes.Versions.v5) -namespace Microsoft.Contoso; - -model Widget is TrackedResource { - @key("widgetName") - @segment("widgets") - name: string; -} - -model WidgetProperties { - provisioningState?: ResourceProvisioningState; -} - -@armResourceOperations -interface Widgets { - modify is ArmResourcePatchSync; -} -``` - -## Correct - -```tsp -@armProviderNamespace -@service -@armCommonTypesVersion(CommonTypes.Versions.v5) -namespace Microsoft.Contoso; - -model Widget is TrackedResource { - @key("widgetName") - @segment("widgets") - name: string; -} - -model WidgetProperties { - provisioningState?: ResourceProvisioningState; -} - -@armResourceOperations -interface Widgets { - update is ArmResourcePatchSync; -} -``` - -The examples assume the usual ARM library imports and `using` declarations. - -## Impact - -**API, SDK.** Inconsistent partial-update operation names make APIs harder to discover. Renaming -an already shipped operation may break existing SDK callers. - -## Suppression - -Suppress this warning when compatibility requires preserving a shipped operation name. Prefer -an `update` prefix for new operations. - -```tsp -@armResourceOperations -interface Widgets { - #suppress "@azure-tools/typespec-azure-resource-manager/use-update-for-patch" "Preserve the shipped SDK operation name." - modify is ArmResourcePatchSync; -} -``` - -## LintDiff Equivalent - -This rule corresponds to -[`PatchInOperationName`](https://github.com/Azure/azure-openapi-validator/blob/main/docs/patch-in-operation-name.md). -It enforces authored ARM endpoint names rather than reproducing emitted-operation-ID exemptions. diff --git a/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.ts b/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.ts deleted file mode 100644 index 6fad958872..0000000000 --- a/packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { createRule, fileRef, paramMessage } from "@typespec/compiler"; -import { getAllHttpServices } from "@typespec/http"; - -export const useUpdateForPatchRule = createRule({ - name: "use-update-for-patch", - docs: fileRef.fromPackageRoot("src/rules/use-update-for-patch.md"), - description: "ARM PATCH operation names should start with 'update'.", - url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-update-for-patch", - severity: "warning", - messages: { - default: paramMessage`ARM PATCH operation '${"operationName"}' should start with 'update'.`, - }, - create(context) { - return { - root: () => { - const [services] = getAllHttpServices(context.program); - for (const service of services) { - for (const { operation, verb } of service.operations) { - if (verb !== "patch" || operation.name.toLowerCase().startsWith("update")) { - continue; - } - - context.reportDiagnostic({ - target: operation, - format: { operationName: operation.name }, - }); - } - } - }, - }; - }, -}); diff --git a/packages/typespec-azure-resource-manager/test/rules/use-update-for-patch.test.ts b/packages/typespec-azure-resource-manager/test/rules/use-update-for-patch.test.ts deleted file mode 100644 index a749eee115..0000000000 --- a/packages/typespec-azure-resource-manager/test/rules/use-update-for-patch.test.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { Tester } from "#test/tester.js"; -import { createLinterRuleTester, type LinterRuleTester } from "@typespec/compiler/testing"; -import { readFile } from "node:fs/promises"; -import { beforeEach, it } from "vitest"; -import { useUpdateForPatchRule } from "../../src/rules/use-update-for-patch.js"; - -let tester: LinterRuleTester; - -beforeEach(async () => { - tester = createLinterRuleTester( - await Tester.createInstance(), - useUpdateForPatchRule, - "@azure-tools/typespec-azure-resource-manager", - ); -}); - -const header = "@service namespace Microsoft.Contoso;"; - -function diagnostic(name: string) { - return { - code: "@azure-tools/typespec-azure-resource-manager/use-update-for-patch", - message: `ARM PATCH operation '${name}' should start with 'update'.`, - }; -} - -it.each(["patch", "modify", "create", "patch_Update"])("rejects PATCH name %s", async (name) => { - await tester - .expect(`${header} @patch @route("/item") op ${name}(): string;`) - .toEmitDiagnostics([diagnostic(name)]); -}); - -it.each(["update", "updateWidget", "UpdateTags"])("accepts update prefix %s", async (name) => { - await tester.expect(`${header} @patch @route("/item") op ${name}(): string;`).toBeValid(); -}); - -it.each([200, 201, 202, 204])("checks PATCH names for response %s", async (status) => { - await tester - .expect(`${header} @patch @route("/item") op modify(): { @statusCode code: ${status}; };`) - .toEmitDiagnostics([diagnostic("modify")]); -}); - -it("ignores non-PATCH verbs", async () => { - await tester.expect(`${header} @get @route("/item") op modify(): string;`).toBeValid(); -}); - -it("does not let an interface name mask the operation prefix", async () => { - await tester - .expect(`${header} @route("/item") interface UpdateWidgets { @patch op modify(): string; }`) - .toEmitDiagnostics([diagnostic("modify")]); -}); - -it("reports distinct offending operation names without a provider decorator", async () => { - await tester - .expect( - ` - ${header} - @patch @route("/first") op modifyFirst(): string; - @patch @route("/second") op modifySecond(): string; - `, - ) - .toEmitDiagnostics([diagnostic("modifyFirst"), diagnostic("modifySecond")]); -}); - -it("validates the exact incorrect documentation example", async () => { - const source = await readFile( - new URL("../../src/rules/use-update-for-patch.md", import.meta.url), - "utf8", - ); - const snippet = source - .replace(/\r\n/g, "\n") - .split("## Incorrect")[1] - .match(/```tsp\n([\s\S]*?)```/)![1]; - await tester.expect(snippet).toEmitDiagnostics([diagnostic("modify")]); -}); - -it("validates the exact correct documentation example", async () => { - const source = await readFile( - new URL("../../src/rules/use-update-for-patch.md", import.meta.url), - "utf8", - ); - const snippet = source - .replace(/\r\n/g, "\n") - .split("## Correct")[1] - .match(/```tsp\n([\s\S]*?)```/)![1]; - await tester.expect(snippet).toBeValid(); -}); diff --git a/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts b/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts index 2cfd0b40bd..f7040184f8 100644 --- a/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts +++ b/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts @@ -8,6 +8,7 @@ export default { "@azure-tools/typespec-client-generator-core/csharp-model-suffix": true, "@azure-tools/typespec-client-generator-core/use-union-hierarchy": true, "@azure-tools/typespec-client-generator-core/get-operation-name": false, + "@azure-tools/typespec-client-generator-core/use-update-for-patch": false, }, disable: { "@azure-tools/typespec-client-generator-core/use-create-for-put": diff --git a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts index c55f8bca64..b04bddedb8 100644 --- a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts +++ b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts @@ -109,7 +109,6 @@ export default { "@azure-tools/typespec-azure-resource-manager/unsupported-type": true, "@azure-tools/typespec-azure-resource-manager/no-query-in-point-op": false, "@azure-tools/typespec-azure-resource-manager/use-model-request-body": false, - "@azure-tools/typespec-azure-resource-manager/use-update-for-patch": false, // TCGC rules "@azure-tools/typespec-client-generator-core/require-client-suffix": true, diff --git a/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts b/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts index ab829cd683..07f07991a4 100644 --- a/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts +++ b/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts @@ -72,4 +72,18 @@ describe("expect all rules to be defined", () => { ok(!$linter.ruleSets?.[rulesetName].enable?.[ruleName]); } }); + + it("keeps PATCH SDK naming guidance opt-in", () => { + const ruleName = "@azure-tools/typespec-client-generator-core/use-update-for-patch"; + strictEqual($linter.ruleSets?.["client-sdk"].enable?.[ruleName], false); + for (const rulesetName of ["data-plane", "resource-manager"]) { + strictEqual($linter.ruleSets?.[rulesetName].enable?.[ruleName], undefined); + strictEqual( + $linter.ruleSets?.[rulesetName].enable?.[ + "@azure-tools/typespec-azure-resource-manager/use-update-for-patch" + ], + undefined, + ); + } + }); }); diff --git a/packages/typespec-client-generator-core/README.md b/packages/typespec-client-generator-core/README.md index 9292331adf..e0ecba68e9 100644 --- a/packages/typespec-client-generator-core/README.md +++ b/packages/typespec-client-generator-core/README.md @@ -121,6 +121,7 @@ Available ruleSets: | Name | Description | | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | [`@azure-tools/typespec-client-generator-core/use-create-for-put`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-create-for-put) | ARM PUT SDK method names should use 'create' as the verb prefix. | +| [`@azure-tools/typespec-client-generator-core/use-update-for-patch`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-update-for-patch) | ARM PATCH SDK method names should use 'update' as the verb prefix. | | [`@azure-tools/typespec-client-generator-core/require-client-suffix`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/require-client-suffix) | Client names should end with 'Client'. | | [`@azure-tools/typespec-client-generator-core/property-name-conflict`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/property-name-conflict) | Avoid naming conflicts between a property and a model of the same name. | | [`@azure-tools/typespec-client-generator-core/csharp-no-url-suffix`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/csharp-no-url-suffix) | Properties ending with 'Url' should use 'Uri' suffix instead to follow .NET naming conventions. | diff --git a/packages/typespec-client-generator-core/src/linter.ts b/packages/typespec-client-generator-core/src/linter.ts index fb02138678..b615c582d3 100644 --- a/packages/typespec-client-generator-core/src/linter.ts +++ b/packages/typespec-client-generator-core/src/linter.ts @@ -7,9 +7,11 @@ import { propertyNameConflictRule } from "./rules/property-name-conflict.rule.js import { requireClientSuffixRule } from "./rules/require-client-suffix.rule.js"; import { useCreateForPutRule } from "./rules/use-create-for-put.js"; import { useUnionHierarchyRule } from "./rules/use-union-hierarchy.js"; +import { useUpdateForPatchRule } from "./rules/use-update-for-patch.js"; const rules = [ useCreateForPutRule, + useUpdateForPatchRule, requireClientSuffixRule, propertyNameConflictRule, csharpNoUrlSuffixRule, diff --git a/packages/typespec-client-generator-core/src/rules/use-update-for-patch.md b/packages/typespec-client-generator-core/src/rules/use-update-for-patch.md new file mode 100644 index 0000000000..5f05aa2be3 --- /dev/null +++ b/packages/typespec-client-generator-core/src/rules/use-update-for-patch.md @@ -0,0 +1,82 @@ +Use an SDK method name beginning with `update` for ARM PATCH operations, such as +`update`, `updateTags`, or `updateWidget`. This applies even when the operation uses +a standard ARM patch template, independently of its response status codes. + +The rule checks the common TCGC SDK method name case-insensitively on concrete HTTP +endpoints. When enabled, it does not require `@armProviderNamespace`; select it for +ARM APIs, not data-plane APIs with different naming guidance. It is disabled by +default in the `client-sdk` ruleset. + +Name resolution uses TCGC's `getLibraryName`, including an unscoped `@clientName` +override, `exact` client names, and the usual `@friendlyName` fallback. +Emitter-scoped overrides and OpenAPI `@operationId` values do not define the common +SDK method name and are ignored. + +The source program is checked, including operations removed in later versions. +Historical names supplied through `@renamedFrom` are not reconstructed. + +#### ❌ Incorrect + +```typespec +@armResourceOperations +interface Widgets { + modify is ArmResourcePatchSync; +} +``` + +#### ✅ Correct + +```typespec +@armResourceOperations +interface Widgets { + update is ArmResourcePatchSync; +} +``` + +These examples assume an ARM service, a tracked resource named `Widget`, and its +property model `WidgetProperties`. + +## Impact + +- **Area:** SDK + +Consistent update-method naming makes partial updates easier to discover in +generated clients. Renaming a method after an SDK has shipped may introduce a +breaking change to its public API. Review compatibility before changing an +existing SDK method name. + +An unscoped `@clientName` can provide the correct SDK name without renaming the +TypeSpec operation: + +```typespec +@armResourceOperations +interface Widgets { + @Azure.ClientGenerator.Core.clientName("updateWidget") + modify is ArmResourcePatchSync; +} +``` + +Import `@azure-tools/typespec-client-generator-core` to use `@clientName`. + +## Suppression + +Suppression is acceptable when preserving an already-shipped SDK method name +requires an exception approved by the API reviewer. For new operations, fix the +SDK method name instead, either by renaming the operation or using `@clientName`. + +```typespec +@armResourceOperations +interface Widgets { + #suppress "@azure-tools/typespec-client-generator-core/use-update-for-patch" "Preserve the already-shipped SDK method name." + modify is ArmResourcePatchSync; +} +``` + +## LintDiff Equivalent + +This rule enforces the ARM SDK method-naming intent of +[`PatchInOperationName`](https://github.com/Azure/azure-openapi-validator/blob/aec54e95e52338d0afa911a92958541d5624c105/docs/patch-in-operation-name.md). +It follows TCGC's `use-create-for-put` common SDK naming pattern rather than +reproducing Swagger `operationId` group-name exemptions or checking only authored +TypeSpec names. Common SDK name overrides intentionally affect this rule; no +emitter or version projection is used. diff --git a/packages/typespec-client-generator-core/src/rules/use-update-for-patch.ts b/packages/typespec-client-generator-core/src/rules/use-update-for-patch.ts new file mode 100644 index 0000000000..9ec31904f3 --- /dev/null +++ b/packages/typespec-client-generator-core/src/rules/use-update-for-patch.ts @@ -0,0 +1,47 @@ +import { createRule, fileRef, paramMessage } from "@typespec/compiler"; +import { getAllHttpServices } from "@typespec/http"; +import { createTCGCContext } from "../context.js"; +import { AllScopes } from "../internal-utils.js"; +import { getLibraryName } from "../public-utils.js"; + +export const useUpdateForPatchRule = createRule({ + name: "use-update-for-patch", + description: "ARM PATCH SDK method names should use 'update' as the verb prefix.", + severity: "warning", + url: "https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-update-for-patch", + docs: fileRef.fromPackageRoot("src/rules/use-update-for-patch.md"), + messages: { + default: paramMessage`PATCH SDK method name '${"operationName"}' should start with 'update'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.`, + }, + create(context) { + const tcgcContext = createTCGCContext( + context.program, + "@azure-tools/typespec-client-generator-core", + { mutateNamespace: false }, + ); + + return { + root: () => { + const [services] = getAllHttpServices(context.program); + for (const service of services) { + for (const { operation, verb } of service.operations) { + if (verb !== "patch") { + continue; + } + + // Check the common SDK name, leaving emitter-specific overrides to emitter rules. + const operationName = getLibraryName(tcgcContext, operation, AllScopes); + if (operationName.toLowerCase().startsWith("update")) { + continue; + } + + context.reportDiagnostic({ + target: operation, + format: { operationName }, + }); + } + } + }, + }; + }, +}); diff --git a/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts b/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts new file mode 100644 index 0000000000..b696bba5d2 --- /dev/null +++ b/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts @@ -0,0 +1,290 @@ +import { + createLinterRuleTester, + type LinterRuleTester, + type TesterInstance, +} from "@typespec/compiler/testing"; +import { readFile } from "node:fs/promises"; +import { beforeEach, describe, it, vi } from "vitest"; +import { useUpdateForPatchRule } from "../../src/rules/use-update-for-patch.js"; +import { ArmTester } from "../tester.js"; + +vi.mock("@azure-tools/typespec-autorest", () => { + throw new Error("Native lint must not load the AutoRest emitter."); +}); + +let tester: LinterRuleTester; +let runner: TesterInstance; + +beforeEach(async () => { + runner = await ArmTester.import("@typespec/openapi").createInstance(); + tester = createLinterRuleTester( + runner, + useUpdateForPatchRule, + "@azure-tools/typespec-client-generator-core", + ); +}); + +const header = ` + @armProviderNamespace @service + @armCommonTypesVersion(CommonTypes.Versions.v5) + namespace Arm; +`; + +const widget = ` + model Widget is TrackedResource { + @key("widgetName") @segment("widgets") name: string; + } + model WidgetProperties { provisioningState?: ResourceProvisioningState; } +`; + +function diagnostic(operationName: string, target = operationName) { + return { + code: "@azure-tools/typespec-client-generator-core/use-update-for-patch", + severity: "warning" as const, + message: `PATCH SDK method name '${operationName}' should start with 'update'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.`, + target, + }; +} + +async function documentationSnippet(section: string) { + const source = await readFile( + new URL("../../src/rules/use-update-for-patch.md", import.meta.url), + "utf8", + ); + const snippet = source + .replace(/\r\n/g, "\n") + .split(section)[1] + ?.match(/```typespec\n([\s\S]*?)```/)?.[1]; + if (!snippet) { + throw new Error(`Missing documentation snippet: ${section}`); + } + return `${header} ${widget} ${snippet}`; +} + +describe("use-update-for-patch", () => { + it("accepts an unscoped client name override", async () => { + await tester + .expect(`${header} @clientName("updateWidget") @patch @route("/item") op modify(): string;`) + .toBeValid(); + }); + + it("reports an invalid unscoped client name on the authored operation", async () => { + await tester + .expect(`${header} @clientName("modifyWidget") @patch @route("/item") op update(): string;`) + .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); + }); + + it("ignores emitter-scoped overrides of a compliant default name", async () => { + await tester + .expect( + `${header} @clientName("modifyWidget", "csharp") @patch @route("/item") op update(): string;`, + ) + .toBeValid(); + }); + + it("does not let an emitter-scoped override mask an invalid default name", async () => { + await tester + .expect( + `${header} @clientName("updateWidget", "python") @patch @route("/item") op modify(): string;`, + ) + .toEmitDiagnostics([diagnostic("modify")]); + }); + + it("uses the unscoped name when scoped and unscoped overrides coexist", async () => { + await tester + .expect( + `${header} + @clientName("updateWidget") + @clientName("modifyWidget", "csharp") + @patch @route("/item") op modify(): string;`, + ) + .toBeValid(); + }); + + it("does not let a scoped name mask an invalid unscoped override", async () => { + await tester + .expect( + `${header} + @clientName("modifyWidget") + @clientName("updateWidget", "python") + @patch @route("/item") op update(): string;`, + ) + .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); + }); + + it("honors client names on standard ARM patch template aliases", async () => { + await tester + .expect( + `${header} ${widget} + @armResourceOperations interface Widgets { + @clientName("updateWidget") + modify is ArmResourcePatchSync; + }`, + ) + .toBeValid(); + }); + + it("reports invalid common client names on standard ARM patch template aliases", async () => { + await tester + .expect( + `${header} ${widget} + @armResourceOperations interface Widgets { + @clientName("modifyWidget") + update is ArmResourcePatchSync; + }`, + ) + .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); + }); + + it("normalizes exact client names", async () => { + await tester + .expect( + `${header} @clientName(exact("UpdateWidget")) @patch @route("/item") op modify(): string;`, + ) + .toBeValid(); + }); + + it("reports invalid exact client names on the authored operation", async () => { + await tester + .expect( + `${header} @clientName(exact("ModifyWidget")) @patch @route("/item") op update(): string;`, + ) + .toEmitDiagnostics([diagnostic("ModifyWidget", "update")]); + }); + + it("uses the friendly name fallback", async () => { + await tester + .expect(`${header} @friendlyName("updateWidget") @patch @route("/item") op modify(): string;`) + .toBeValid(); + }); + + it("reports an invalid friendly name on the authored operation", async () => { + await tester + .expect(`${header} @friendlyName("modifyWidget") @patch @route("/item") op update(): string;`) + .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); + }); + + it("prefers a common client name over a friendly name", async () => { + await tester + .expect( + `${header} + @clientName("updateWidget") @friendlyName("modifyWidget") + @patch @route("/item") op modify(): string;`, + ) + .toBeValid(); + }); + + // cspell:ignore updat + it.each(["patch", "modify", "create", "patch_Update", "u", "updat"])( + "rejects PATCH name %s", + async (name) => { + await tester + .expect(`${header} @patch @route("/item") op ${name}(): string;`) + .toEmitDiagnostics([diagnostic(name)]); + }, + ); + + it.each(["update", "updateWidget", "updateTags", "UpdateTags", "UPDATE", "uPdAtEWidget"])( + "accepts update prefix %s", + async (name) => { + await tester.expect(`${header} @patch @route("/item") op ${name}(): string;`).toBeValid(); + }, + ); + + it.each(["get", "put", "post", "delete", "head"])("ignores HTTP verb %s", async (verb) => { + await tester + .expect(`${header} @${verb} @route("/item") op modify(): { @statusCode code: 204 };`) + .toBeValid(); + }); + + it.each([200, 201, 202, 204])( + "checks PATCH names independently of response code %s", + async (code) => { + await tester + .expect(`${header} @patch @route("/item") op modify(): { @statusCode code: ${code} };`) + .toEmitDiagnostics([diagnostic("modify")]); + }, + ); + + it("does not let an interface name mask the operation prefix", async () => { + await tester + .expect(`${header} @route("/item") interface UpdateWidgets { @patch op modify(): string; }`) + .toEmitDiagnostics([diagnostic("modify")]); + }); + + it("checks ordinary service namespaces without provider metadata", async () => { + await tester + .expect(`@service namespace Contoso { @patch @route("/item") op modify(): string; }`) + .toEmitDiagnostics([diagnostic("modify")]); + }); + + it("reports distinct offending endpoints on their authored operations", async () => { + await tester + .expect( + `${header} + @patch @route("/first") op modifyFirst(): string; + @patch @route("/second") op modifySecond(): string;`, + ) + .toEmitDiagnostics([diagnostic("modifyFirst"), diagnostic("modifySecond")]); + }); + + it("ignores an operationId that would mask an invalid common SDK name", async () => { + await tester + .expect( + `${header} + @TypeSpec.OpenAPI.operationId("Widgets_Update") + @patch @route("/item") op modify(): string;`, + ) + .toEmitDiagnostics([diagnostic("modify")]); + }); + + it("ignores an operationId that would invalidate a compliant common SDK name", async () => { + await tester + .expect( + `${header} + @TypeSpec.OpenAPI.operationId("Widgets_Modify") + @patch @route("/item") op update(): string;`, + ) + .toBeValid(); + }); + + it("retains historical operations but does not simulate historical names", async () => { + await tester + .expect( + ` + @service @armProviderNamespace @versioned(Versions) + @armCommonTypesVersion(CommonTypes.Versions.v5) + namespace Arm; + enum Versions { v1, v2 } + @removed(Versions.v2) @route("/old") @patch op modify(): string; + @renamedFrom(Versions.v2, "modify") @route("/renamed") @patch op update(): string; + `, + ) + .toEmitDiagnostics([diagnostic("modify")]); + }); + + it("validates the exact incorrect documentation example", async () => { + await tester + .expect(await documentationSnippet("#### ❌ Incorrect")) + .toEmitDiagnostics([diagnostic("modify")]); + }); + + it("validates the exact correct documentation example", async () => { + await tester.expect(await documentationSnippet("#### ✅ Correct")).toBeValid(); + }); + + it("validates the exact common client name documentation example", async () => { + await tester.expect(await documentationSnippet("## Impact")).toBeValid(); + }); + + it("validates the exact suppression documentation example", async () => { + // The direct rule tester collects diagnostics before compiler suppression is applied. + await runner.compile(await documentationSnippet("## Suppression"), { + compilerOptions: { + linterRuleSet: { + enable: { "@azure-tools/typespec-client-generator-core/use-update-for-patch": true }, + }, + }, + }); + }); +}); diff --git a/website/src/content/docs/docs/howtos/ARM/arm-rules.md b/website/src/content/docs/docs/howtos/ARM/arm-rules.md index bff583dc3a..bb68345799 100644 --- a/website/src/content/docs/docs/howtos/ARM/arm-rules.md +++ b/website/src/content/docs/docs/howtos/ARM/arm-rules.md @@ -145,7 +145,7 @@ The tables below provide guidance to rule authors and ARM reviewers on how to ev | [`use-interface`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-interface/) | — | **API, SDK, Emitters.** Resource operations outside interfaces can break ARM resource-operation modeling and downstream tooling assumptions. | | [`use-model-request-body`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-model-request-body/) | [`ParametersSchemaAsTypeObject`](https://github.com/Azure/azure-openapi-validator/blob/main/docs/parameters-schema-as-type-object.md) | **API, SDK.** Non-model, array, and record request bodies cannot evolve with additional properties without a breaking change. | | [`use-operation-decorator`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-operation-decorator/) | — | **API, SDK, Emitters.** Missing or mismatched decorators prevent resource operations from being associated with the correct resource, which can break SDK generation and resource-aware tooling. | -| [`use-update-for-patch`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-update-for-patch/) | [`PatchInOperationName`](https://github.com/Azure/azure-openapi-validator/blob/main/docs/patch-in-operation-name.md) | **API, SDK.** Inconsistent authored PATCH names make partial-update operations harder to discover; renaming shipped operations may break SDK callers. | +| [`use-update-for-patch`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-update-for-patch/) | [`PatchInOperationName`](https://github.com/Azure/azure-openapi-validator/blob/main/docs/patch-in-operation-name.md) | **SDK.** TCGC checks the common SDK method name, including unscoped `@clientName` overrides; renaming a shipped method may break SDK compatibility. | | [`version-progression`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/version-progression/) | — | **API, SDK, Tooling.** Out-of-order versions make specs unmaintainable, and versions with matching dates cause SDK problems. | ### Rules in @azure-tools/typespec-client-generator-core diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md b/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md index b2a866492c..7371c440d3 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md @@ -70,7 +70,6 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/retry-after`](../rules/retry-after.md) | Check if retry-after header appears in response body. | | [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](../rules/unsupported-type.md) | Check for unsupported ARM types. | | [`@azure-tools/typespec-azure-resource-manager/use-model-request-body`](../rules/use-model-request-body.md) | Request bodies must use plain models. | -| [`@azure-tools/typespec-azure-resource-manager/use-update-for-patch`](../rules/use-update-for-patch.md) | ARM PATCH operation names should start with 'update'. | | [`@azure-tools/typespec-azure-resource-manager/secret-prop`](../rules/secret-prop.md) | RPC-v1-13: Check that property with names indicating sensitive information(e.g. contains auth, password, token, secret, etc.) are marked with @secret decorator. | | [`@azure-tools/typespec-azure-resource-manager/no-empty-model`](../rules/no-empty-model.md) | ARM Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience. | | [`@azure-tools/typespec-azure-resource-manager/no-reserved-resource-property`](../rules/no-reserved-resource-property.md) | Reserved property names (for example 'billingData') must not be present in a resource's property bag. The property name is matched case-insensitively. | diff --git a/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md b/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md index c7bd311955..6279939788 100644 --- a/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md +++ b/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md @@ -24,6 +24,7 @@ Available ruleSets: | Name | Description | | ---------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | [`@azure-tools/typespec-client-generator-core/use-create-for-put`](../rules/use-create-for-put.md) | ARM PUT SDK method names should use 'create' as the verb prefix. | +| [`@azure-tools/typespec-client-generator-core/use-update-for-patch`](../rules/use-update-for-patch.md) | ARM PATCH SDK method names should use 'update' as the verb prefix. | | [`@azure-tools/typespec-client-generator-core/require-client-suffix`](../rules/require-client-suffix.md) | Client names should end with 'Client'. | | [`@azure-tools/typespec-client-generator-core/property-name-conflict`](../rules/property-name-conflict.md) | Avoid naming conflicts between a property and a model of the same name. | | [`@azure-tools/typespec-client-generator-core/csharp-no-url-suffix`](../rules/csharp-no-url-suffix.md) | Properties ending with 'Url' should use 'Uri' suffix instead to follow .NET naming conventions. | From abeb615adf3914b7f13547b2f1c0b5796e27ab72 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Mon, 12 Oct 2026 15:13:32 +0800 Subject: [PATCH 03/17] feat(tcgc): add opt-in DELETE SDK naming rule Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 832854b7-de67-4f1f-8fe1-6a3e9897aeee --- .../changes/register-use-delete-for-delete.md | 7 + .chronus/changes/use-delete-for-delete.md | 7 + .../src/rulesets/client-sdk.ts | 1 + .../typespec-client-generator-core/README.md | 1 + .../src/linter.ts | 2 + .../src/rules/use-delete-for-delete.md | 76 ++++ .../src/rules/use-delete-for-delete.ts | 46 +++ .../test/rules/use-delete-for-delete.test.ts | 332 ++++++++++++++++++ .../reference/linter.md | 1 + 9 files changed, 473 insertions(+) create mode 100644 .chronus/changes/register-use-delete-for-delete.md create mode 100644 .chronus/changes/use-delete-for-delete.md create mode 100644 packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md create mode 100644 packages/typespec-client-generator-core/src/rules/use-delete-for-delete.ts create mode 100644 packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts diff --git a/.chronus/changes/register-use-delete-for-delete.md b/.chronus/changes/register-use-delete-for-delete.md new file mode 100644 index 0000000000..a1292ac12f --- /dev/null +++ b/.chronus/changes/register-use-delete-for-delete.md @@ -0,0 +1,7 @@ +--- +changeKind: internal +packages: + - "@azure-tools/typespec-azure-rulesets" +--- + +Register the `@azure-tools/typespec-client-generator-core/use-delete-for-delete` lint rule as disabled in the `client-sdk` ruleset. diff --git a/.chronus/changes/use-delete-for-delete.md b/.chronus/changes/use-delete-for-delete.md new file mode 100644 index 0000000000..4ea7e9b809 --- /dev/null +++ b/.chronus/changes/use-delete-for-delete.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +Add the opt-in `use-delete-for-delete` lint rule requiring common SDK method names of concrete HTTP DELETE endpoints to start with `delete`, honoring unscoped client names and the supported friendly-name fallback. diff --git a/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts b/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts index 2cfd0b40bd..a4972841cb 100644 --- a/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts +++ b/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts @@ -8,6 +8,7 @@ export default { "@azure-tools/typespec-client-generator-core/csharp-model-suffix": true, "@azure-tools/typespec-client-generator-core/use-union-hierarchy": true, "@azure-tools/typespec-client-generator-core/get-operation-name": false, + "@azure-tools/typespec-client-generator-core/use-delete-for-delete": false, }, disable: { "@azure-tools/typespec-client-generator-core/use-create-for-put": diff --git a/packages/typespec-client-generator-core/README.md b/packages/typespec-client-generator-core/README.md index 9292331adf..17d622c859 100644 --- a/packages/typespec-client-generator-core/README.md +++ b/packages/typespec-client-generator-core/README.md @@ -121,6 +121,7 @@ Available ruleSets: | Name | Description | | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | [`@azure-tools/typespec-client-generator-core/use-create-for-put`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-create-for-put) | ARM PUT SDK method names should use 'create' as the verb prefix. | +| [`@azure-tools/typespec-client-generator-core/use-delete-for-delete`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-delete-for-delete) | DELETE SDK method names should use 'delete' as the verb prefix. | | [`@azure-tools/typespec-client-generator-core/require-client-suffix`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/require-client-suffix) | Client names should end with 'Client'. | | [`@azure-tools/typespec-client-generator-core/property-name-conflict`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/property-name-conflict) | Avoid naming conflicts between a property and a model of the same name. | | [`@azure-tools/typespec-client-generator-core/csharp-no-url-suffix`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/csharp-no-url-suffix) | Properties ending with 'Url' should use 'Uri' suffix instead to follow .NET naming conventions. | diff --git a/packages/typespec-client-generator-core/src/linter.ts b/packages/typespec-client-generator-core/src/linter.ts index fb02138678..72e806e0f1 100644 --- a/packages/typespec-client-generator-core/src/linter.ts +++ b/packages/typespec-client-generator-core/src/linter.ts @@ -6,10 +6,12 @@ import { getOperationNameRule } from "./rules/get-operation-name.rule.js"; import { propertyNameConflictRule } from "./rules/property-name-conflict.rule.js"; import { requireClientSuffixRule } from "./rules/require-client-suffix.rule.js"; import { useCreateForPutRule } from "./rules/use-create-for-put.js"; +import { useDeleteForDeleteRule } from "./rules/use-delete-for-delete.js"; import { useUnionHierarchyRule } from "./rules/use-union-hierarchy.js"; const rules = [ useCreateForPutRule, + useDeleteForDeleteRule, requireClientSuffixRule, propertyNameConflictRule, csharpNoUrlSuffixRule, diff --git a/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md b/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md new file mode 100644 index 0000000000..103a7828d9 --- /dev/null +++ b/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md @@ -0,0 +1,76 @@ +Use an SDK method name beginning with `delete` for HTTP DELETE operations, such as +`delete` or `deleteWidget`. Consistent names make deletion methods easier to +discover in generated clients. + +The rule checks the common TCGC SDK method name case-insensitively on concrete +HTTP service endpoints. It applies to ARM and data-plane services without requiring +`@armProviderNamespace`, and is disabled by default in the `client-sdk` ruleset. +Client-only declarations that are not service endpoints are not checked. + +Name resolution uses TCGC's `getLibraryName` with `AllScopes`, including an +unscoped `@clientName`, exact names, and the supported `@friendlyName` fallback. +Overrides applying only to a particular language, such as +`@clientName("remove", "csharp")`, do not replace the common name. + +The source program is checked without selecting an emitter or an API-version +projection. Historical names supplied through `@renamedFrom` are not reconstructed. + +The following examples use generic HTTP authoring: import `@typespec/http`, use +`TypeSpec.Http`, and place the operation in a `@service` namespace. For ARM +resources, use the standard +[resource delete operation templates](https://azure.github.io/typespec-azure/docs/howtos/ARM/resource-operations#resource-delete-operations-delete), +with a name beginning with `delete`, such as +`delete is ArmResourceDeleteSync;`. + +#### ❌ Incorrect + +```typespec +@delete +@route("/widgets/{id}") +op remove(@path id: string): void; +``` + +#### ✅ Correct + +```typespec +@delete +@route("/widgets/{id}") +op deleteWidget(@path id: string): void; +``` + +## Impact + +- **Area:** SDK + +Renaming a method in an already-shipped SDK may introduce a breaking change. +Review compatibility before changing existing names. + +An unscoped `@clientName` can provide the correct SDK name without renaming the +TypeSpec operation: + +```typespec +@Azure.ClientGenerator.Core.clientName("deleteWidget") +@delete +@route("/widgets/{id}") +op remove(@path id: string): void; +``` + +Import `@azure-tools/typespec-client-generator-core` to use `@clientName`. + +## Suppression + +Suppression is appropriate when preserving an already-shipped SDK method name +requires an exception approved by the API reviewer. For new methods, fix the +name instead, either by renaming the operation or using an unscoped `@clientName`. +Use `#suppress "@azure-tools/typespec-client-generator-core/use-delete-for-delete"` +on the operation with a justification for the compatibility exception. + +## LintDiff Equivalent + +This rule enforces the SDK naming intent of +[`DeleteInOperationName`](https://github.com/Azure/azure-openapi-validator/blob/main/docs/delete-in-operation-name.md). +It does not reproduce serialized `operationId` underscore or group-prefix +exemptions. Explicit OpenAPI operation IDs do not change the common SDK method +name checked by this rule. The lintdiff source checks authored operation names; +the promoted rule intentionally checks common SDK names, including unscoped +SDK name overrides. diff --git a/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.ts b/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.ts new file mode 100644 index 0000000000..79be740a25 --- /dev/null +++ b/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.ts @@ -0,0 +1,46 @@ +import { createRule, fileRef, paramMessage } from "@typespec/compiler"; +import { getAllHttpServices } from "@typespec/http"; +import { createTCGCContext } from "../context.js"; +import { AllScopes } from "../internal-utils.js"; +import { getLibraryName } from "../public-utils.js"; + +export const useDeleteForDeleteRule = createRule({ + name: "use-delete-for-delete", + description: "DELETE SDK method names should use 'delete' as the verb prefix.", + severity: "warning", + url: "https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-delete-for-delete", + docs: fileRef.fromPackageRoot("src/rules/use-delete-for-delete.md"), + messages: { + default: paramMessage`DELETE SDK method name '${"operationName"}' should start with 'delete'.`, + }, + create(context) { + const tcgcContext = createTCGCContext( + context.program, + "@azure-tools/typespec-client-generator-core", + { mutateNamespace: false }, + ); + + return { + root: () => { + const [services] = getAllHttpServices(context.program); + for (const service of services) { + for (const { operation, verb } of service.operations) { + if (verb !== "delete") { + continue; + } + + const operationName = getLibraryName(tcgcContext, operation, AllScopes); + if (operationName.toLowerCase().startsWith("delete")) { + continue; + } + + context.reportDiagnostic({ + target: operation, + format: { operationName }, + }); + } + } + }, + }; + }, +}); diff --git a/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts new file mode 100644 index 0000000000..8dffb2e255 --- /dev/null +++ b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts @@ -0,0 +1,332 @@ +import { LinterRuleTester, createLinterRuleTester } from "@typespec/compiler/testing"; +import { beforeEach, describe, it, vi } from "vitest"; +import { useDeleteForDeleteRule } from "../../src/rules/use-delete-for-delete.js"; +import { ArmTester, SimpleTester } from "../tester.js"; + +vi.mock("@azure-tools/typespec-autorest", () => { + throw new Error("Native lint must not load the AutoRest emitter."); +}); + +function diagnostic(operationName: string, target = operationName) { + return { + code: "@azure-tools/typespec-client-generator-core/use-delete-for-delete", + severity: "warning" as const, + message: `DELETE SDK method name '${operationName}' should start with 'delete'.`, + target, + }; +} + +describe("use-delete-for-delete", () => { + let tester: LinterRuleTester; + const header = "@service namespace Test;"; + + beforeEach(async () => { + const runner = await SimpleTester.import("@typespec/openapi").createInstance(); + tester = createLinterRuleTester( + runner, + useDeleteForDeleteRule, + "@azure-tools/typespec-client-generator-core", + ); + }); + + it("reports the documented incorrect HTTP SDK name", async () => { + await tester + .expect( + `${header} + @delete + @route("/widgets/{id}") + op remove(@path id: string): void; + `, + ) + .toEmitDiagnostics([diagnostic("remove")]); + }); + + it("accepts the documented correct HTTP SDK name", async () => { + await tester + .expect( + `${header} + @delete + @route("/widgets/{id}") + op deleteWidget(@path id: string): void; + `, + ) + .toBeValid(); + }); + + it("accepts the documented common SDK name override", async () => { + await tester + .expect( + `${header} + @Azure.ClientGenerator.Core.clientName("deleteWidget") + @delete + @route("/widgets/{id}") + op remove(@path id: string): void; + `, + ) + .toBeValid(); + }); + + it("reports an invalid unscoped client name on the authored operation", async () => { + await tester + .expect( + `${header} + @clientName("remove") + @delete @route("/widgets") op deleteWidget(): void; + `, + ) + .toEmitDiagnostics([diagnostic("remove", "deleteWidget")]); + }); + + it("ignores a language-specific override of a compliant common name", async () => { + await tester + .expect( + `${header} + @clientName("remove", "csharp") + @delete @route("/widgets") op deleteWidget(): void; + `, + ) + .toBeValid(); + }); + + it("does not let a language-specific override fix an invalid common name", async () => { + await tester + .expect( + `${header} + @clientName("deleteWidget", "python") + @delete @route("/widgets") op remove(): void; + `, + ) + .toEmitDiagnostics([diagnostic("remove")]); + }); + + it("uses a valid common override when common and scoped names coexist", async () => { + await tester + .expect( + `${header} + @clientName("deleteWidget") + @clientName("remove", "csharp") + @delete @route("/widgets") op remove(): void; + `, + ) + .toBeValid(); + }); + + it("reports an invalid common override despite a valid scoped name", async () => { + await tester + .expect( + `${header} + @clientName("remove") + @clientName("deleteWidget", "python") + @delete @route("/widgets") op deleteWidget(): void; + `, + ) + .toEmitDiagnostics([diagnostic("remove", "deleteWidget")]); + }); + + it("honors an exact common SDK name", async () => { + await tester + .expect( + `${header} + @clientName(exact("DeleteWidget")) + @delete @route("/widgets") op remove(): void; + `, + ) + .toBeValid(); + }); + + it("reports an invalid exact common SDK name on the authored operation", async () => { + await tester + .expect( + `${header} + @clientName(exact("RemoveWidget")) + @delete @route("/widgets") op deleteWidget(): void; + `, + ) + .toEmitDiagnostics([diagnostic("RemoveWidget", "deleteWidget")]); + }); + + it("uses the supported friendly name fallback", async () => { + await tester + .expect( + `${header} + @friendlyName("deleteWidget") + @delete @route("/widgets") op remove(): void; + `, + ) + .toBeValid(); + }); + + it("reports an invalid friendly name on the authored operation", async () => { + await tester + .expect( + `${header} + @friendlyName("remove") + @delete @route("/widgets") op deleteWidget(): void; + `, + ) + .toEmitDiagnostics([diagnostic("remove", "deleteWidget")]); + }); + + it("prefers a common client name over the friendly name fallback", async () => { + await tester + .expect( + `${header} + @friendlyName("remove") + @clientName("deleteWidget") + @delete @route("/widgets") op erase(): void; + `, + ) + .toBeValid(); + }); + + it("does not let an OpenAPI operation ID fix an invalid SDK name", async () => { + await tester + .expect( + `${header} + @TypeSpec.OpenAPI.operationId("Widgets_Delete") + @delete @route("/widgets") op remove(): void; + `, + ) + .toEmitDiagnostics([diagnostic("remove")]); + }); + + it("ignores an invalid OpenAPI operation ID for a compliant SDK name", async () => { + await tester + .expect( + `${header} + @TypeSpec.OpenAPI.operationId("Widgets_Remove") + @delete @route("/widgets") op deleteWidget(): void; + `, + ) + .toBeValid(); + }); + + it("does not let a Delete-prefixed group mask an invalid method name", async () => { + await tester + .expect( + `${header} + interface DeleteWidgets { + @delete @route("/widgets") remove(): void; + } + `, + ) + .toEmitDiagnostics([diagnostic("remove")]); + }); + + it("reports each offending concrete endpoint once", async () => { + await tester + .expect( + `${header} + @delete @route("/widgets") op removeWidget(): void; + @delete @route("/gadgets") op eraseGadget(): void; + @delete @route("/valid") op deleteItem(): void; + `, + ) + .toEmitDiagnostics([diagnostic("removeWidget"), diagnostic("eraseGadget")]); + }); + + it("checks service endpoints rather than client-only declarations", async () => { + await tester + .expect( + ` + @service namespace Test { + @delete @route("/widgets") op remove(): void; + } + namespace Customizations { + @delete @route("/customizations") op removeForClient(): void; + } + `, + ) + .toEmitDiagnostics([diagnostic("remove")]); + }); + + it("accepts delete prefixes case-insensitively", async () => { + await tester + .expect( + `${header} + @delete @route("/one") op delete(): void; + @delete @route("/two") op DeleteWidget(): void; + @delete @route("/three") op DELETEAll(): void; + `, + ) + .toBeValid(); + }); + + it("requires delete at the beginning of the SDK name", async () => { + await tester + .expect(`${header} @delete @route("/widgets") op softDeleteWidget(): void;`) + .toEmitDiagnostics([diagnostic("softDeleteWidget")]); + }); + + it("does not apply to other HTTP verbs", async () => { + await tester + .expect( + `${header} + @get @route("/one") op read(): void; + @post @route("/two") op create(): void; + @put @route("/three") op replace(): void; + @patch @route("/four") op update(): void; + `, + ) + .toBeValid(); + }); +}); + +describe("use-delete-for-delete with standard ARM operations", () => { + let tester: LinterRuleTester; + const resource = ` + @armProviderNamespace @service + @armCommonTypesVersion(CommonTypes.Versions.v5) + namespace Arm; + model Widget is ProxyResource { + @key("widgetName") @segment("widgets") name: string; + } + model WidgetProperties {} + `; + + beforeEach(async () => { + const runner = await ArmTester.createInstance(); + tester = createLinterRuleTester( + runner, + useDeleteForDeleteRule, + "@azure-tools/typespec-client-generator-core", + ); + }); + + it("reports a customized standard ARM delete endpoint", async () => { + await tester + .expect( + `${resource} + @armResourceOperations interface Widgets { + remove is ArmResourceDeleteSync; + } + `, + ) + .toEmitDiagnostics([diagnostic("remove")]); + }); + + it("accepts a standard ARM delete endpoint", async () => { + await tester + .expect( + `${resource} + @armResourceOperations interface Widgets { + delete is ArmResourceDeleteSync; + } + `, + ) + .toBeValid(); + }); + + it("honors a common client name on a standard ARM delete endpoint", async () => { + await tester + .expect( + `${resource} + @armResourceOperations interface Widgets { + @clientName("deleteWidget") + remove is ArmResourceDeleteSync; + } + `, + ) + .toBeValid(); + }); +}); diff --git a/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md b/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md index c7bd311955..0f3dfb81bf 100644 --- a/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md +++ b/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md @@ -24,6 +24,7 @@ Available ruleSets: | Name | Description | | ---------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | [`@azure-tools/typespec-client-generator-core/use-create-for-put`](../rules/use-create-for-put.md) | ARM PUT SDK method names should use 'create' as the verb prefix. | +| [`@azure-tools/typespec-client-generator-core/use-delete-for-delete`](../rules/use-delete-for-delete.md) | DELETE SDK method names should use 'delete' as the verb prefix. | | [`@azure-tools/typespec-client-generator-core/require-client-suffix`](../rules/require-client-suffix.md) | Client names should end with 'Client'. | | [`@azure-tools/typespec-client-generator-core/property-name-conflict`](../rules/property-name-conflict.md) | Avoid naming conflicts between a property and a model of the same name. | | [`@azure-tools/typespec-client-generator-core/csharp-no-url-suffix`](../rules/csharp-no-url-suffix.md) | Properties ending with 'Url' should use 'Uri' suffix instead to follow .NET naming conventions. | From ef1e5891cd5f968defcaee0b1501382e062fce2e Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Mon, 12 Oct 2026 15:58:55 +0800 Subject: [PATCH 04/17] Address promotion review feedback Add regression coverage for the disabled SDK naming rule registration and complete the suppression example. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 832854b7-de67-4f1f-8fe1-6a3e9897aeee --- .../test/validate-rules-defined.test.ts | 4 ++++ .../src/rules/use-delete-for-delete.md | 11 +++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts b/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts index ab829cd683..f75c4de9fa 100644 --- a/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts +++ b/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts @@ -60,6 +60,10 @@ describe("expect all rules to be defined", () => { ruleset.enable?.["@azure-tools/typespec-client-generator-core/get-operation-name"], false, ); + strictEqual( + ruleset.enable?.["@azure-tools/typespec-client-generator-core/use-delete-for-delete"], + false, + ); ok( ruleset.disable?.["@azure-tools/typespec-client-generator-core/csharp-use-standard-acronyms"], ); diff --git a/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md b/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md index 103a7828d9..f0593dcd09 100644 --- a/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md +++ b/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md @@ -62,8 +62,15 @@ Import `@azure-tools/typespec-client-generator-core` to use `@clientName`. Suppression is appropriate when preserving an already-shipped SDK method name requires an exception approved by the API reviewer. For new methods, fix the name instead, either by renaming the operation or using an unscoped `@clientName`. -Use `#suppress "@azure-tools/typespec-client-generator-core/use-delete-for-delete"` -on the operation with a justification for the compatibility exception. +Use `#suppress` on the operation with a justification for the compatibility +exception: + +```typespec +#suppress "@azure-tools/typespec-client-generator-core/use-delete-for-delete" "Preserve the already-shipped SDK method name." +@delete +@route("/widgets/{id}") +op remove(@path id: string): void; +``` ## LintDiff Equivalent From 6475b75fc58016feb14592ff537a9cce3d132788 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Tue, 13 Oct 2026 11:26:27 +0800 Subject: [PATCH 05/17] Use ARM templates in DELETE naming rule documentation Replace raw routes with ArmResourceDeleteSync aliases and validate the actual published examples, including compiler-enabled suppression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 832854b7-de67-4f1f-8fe1-6a3e9897aeee --- .../src/rules/use-delete-for-delete.md | 70 +++++++++++----- .../test/rules/use-delete-for-delete.test.ts | 81 +++++++++++++++++-- 2 files changed, 125 insertions(+), 26 deletions(-) diff --git a/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md b/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md index f0593dcd09..afa0815435 100644 --- a/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md +++ b/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.md @@ -15,27 +15,52 @@ Overrides applying only to a particular language, such as The source program is checked without selecting an emitter or an API-version projection. Historical names supplied through `@renamedFrom` are not reconstructed. -The following examples use generic HTTP authoring: import `@typespec/http`, use -`TypeSpec.Http`, and place the operation in a `@service` namespace. For ARM -resources, use the standard -[resource delete operation templates](https://azure.github.io/typespec-azure/docs/howtos/ARM/resource-operations#resource-delete-operations-delete), -with a name beginning with `delete`, such as -`delete is ArmResourceDeleteSync;`. +The following ARM examples use the standard +[resource delete operation template](https://azure.github.io/typespec-azure/docs/howtos/ARM/resource-operations#resource-delete-operations-delete). +The template supplies the DELETE route and response; the operation alias supplies +the SDK method name. The same naming requirement applies to data-plane services. + +Use this shared setup before each example: + +```typespec +import "@typespec/http"; +import "@typespec/rest"; +import "@azure-tools/typespec-azure-resource-manager"; +import "@azure-tools/typespec-client-generator-core"; + +using TypeSpec.Http; +using TypeSpec.Rest; +using Azure.ResourceManager; +using Azure.ClientGenerator.Core; + +@armProviderNamespace +@service +@armCommonTypesVersion(CommonTypes.Versions.v5) +namespace Example; + +model Widget is ProxyResource { + @key("widgetName") @segment("widgets") name: string; +} + +model WidgetProperties {} +``` #### ❌ Incorrect ```typespec -@delete -@route("/widgets/{id}") -op remove(@path id: string): void; +@armResourceOperations +interface Widgets { + remove is ArmResourceDeleteSync; +} ``` #### ✅ Correct ```typespec -@delete -@route("/widgets/{id}") -op deleteWidget(@path id: string): void; +@armResourceOperations +interface Widgets { + delete is ArmResourceDeleteSync; +} ``` ## Impact @@ -49,13 +74,15 @@ An unscoped `@clientName` can provide the correct SDK name without renaming the TypeSpec operation: ```typespec -@Azure.ClientGenerator.Core.clientName("deleteWidget") -@delete -@route("/widgets/{id}") -op remove(@path id: string): void; +@armResourceOperations +interface Widgets { + @clientName("deleteWidget") + remove is ArmResourceDeleteSync; +} ``` -Import `@azure-tools/typespec-client-generator-core` to use `@clientName`. +The shared setup imports `@azure-tools/typespec-client-generator-core` and uses +`Azure.ClientGenerator.Core` to make `@clientName` available. ## Suppression @@ -66,10 +93,11 @@ Use `#suppress` on the operation with a justification for the compatibility exception: ```typespec -#suppress "@azure-tools/typespec-client-generator-core/use-delete-for-delete" "Preserve the already-shipped SDK method name." -@delete -@route("/widgets/{id}") -op remove(@path id: string): void; +@armResourceOperations +interface Widgets { + #suppress "@azure-tools/typespec-client-generator-core/use-delete-for-delete" "Preserve the already-shipped SDK method name." + remove is ArmResourceDeleteSync; +} ``` ## LintDiff Equivalent diff --git a/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts index 8dffb2e255..e601dec5d3 100644 --- a/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts @@ -1,5 +1,13 @@ -import { LinterRuleTester, createLinterRuleTester } from "@typespec/compiler/testing"; -import { beforeEach, describe, it, vi } from "vitest"; +import { resolvePath } from "@typespec/compiler"; +import { + LinterRuleTester, + TesterInstance, + createLinterRuleTester, + createTester, + expectDiagnosticEmpty, +} from "@typespec/compiler/testing"; +import { readFileSync } from "node:fs"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { useDeleteForDeleteRule } from "../../src/rules/use-delete-for-delete.js"; import { ArmTester, SimpleTester } from "../tester.js"; @@ -29,7 +37,7 @@ describe("use-delete-for-delete", () => { ); }); - it("reports the documented incorrect HTTP SDK name", async () => { + it("reports an invalid HTTP SDK name", async () => { await tester .expect( `${header} @@ -41,7 +49,7 @@ describe("use-delete-for-delete", () => { .toEmitDiagnostics([diagnostic("remove")]); }); - it("accepts the documented correct HTTP SDK name", async () => { + it("accepts a valid HTTP SDK name", async () => { await tester .expect( `${header} @@ -53,7 +61,7 @@ describe("use-delete-for-delete", () => { .toBeValid(); }); - it("accepts the documented common SDK name override", async () => { + it("accepts a common SDK name override", async () => { await tester .expect( `${header} @@ -329,4 +337,67 @@ describe("use-delete-for-delete with standard ARM operations", () => { ) .toBeValid(); }); + + describe("published template documentation examples", () => { + let documentationRunner: TesterInstance; + let setup: string; + let examples: string[]; + + beforeEach(async () => { + const runner = await createTester(resolvePath(import.meta.dirname, "..", ".."), { + libraries: [ + "@typespec/http", + "@typespec/rest", + "@typespec/versioning", + "@typespec/openapi", + "@azure-tools/typespec-azure-core", + "@azure-tools/typespec-azure-resource-manager", + "@azure-tools/typespec-client-generator-core", + ], + }).createInstance(); + documentationRunner = runner; + tester = createLinterRuleTester( + runner, + useDeleteForDeleteRule, + "@azure-tools/typespec-client-generator-core", + ); + const documentation = readFileSync( + new URL("../../src/rules/use-delete-for-delete.md", import.meta.url), + "utf8", + ); + const snippets = [...documentation.matchAll(/```typespec\r?\n([\s\S]*?)```/g)].map( + (match) => match[1], + ); + expect(snippets).toHaveLength(5); + [setup, ...examples] = snippets; + }); + + it("reports the published incorrect template alias", async () => { + await tester.expect(`${setup}\n${examples[0]}`).toEmitDiagnostics([diagnostic("remove")]); + }); + + it("accepts the published correct template alias", async () => { + await tester.expect(`${setup}\n${examples[1]}`).toBeValid(); + }); + + it("accepts the published template client name override", async () => { + await tester.expect(`${setup}\n${examples[2]}`).toBeValid(); + }); + + it("accepts the published template suppression", async () => { + const [, diagnostics] = await documentationRunner.compileAndDiagnose( + `${setup}\n${examples[3]}`, + { + compilerOptions: { + linterRuleSet: { + enable: { + "@azure-tools/typespec-client-generator-core/use-delete-for-delete": true, + }, + }, + }, + }, + ); + expectDiagnosticEmpty(diagnostics); + }); + }); }); From 2af91d5fc97ead1c31a6999a339e24419839fbd4 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 14 Oct 2026 14:14:18 +0800 Subject: [PATCH 06/17] refactor(tcgc): share PATCH and PUT SDK verb-prefix checks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../changes/review-patch-sdk-verb-prefix.md | 7 +++ .../src/rules/use-create-for-put.ts | 31 ++++------ .../src/rules/use-update-for-patch.ts | 31 ++++------ .../src/rules/verb-prefix.ts | 22 +++++++ .../test/rules/use-update-for-patch.test.ts | 59 ++++++++++++------- 5 files changed, 86 insertions(+), 64 deletions(-) create mode 100644 .chronus/changes/review-patch-sdk-verb-prefix.md create mode 100644 packages/typespec-client-generator-core/src/rules/verb-prefix.ts diff --git a/.chronus/changes/review-patch-sdk-verb-prefix.md b/.chronus/changes/review-patch-sdk-verb-prefix.md new file mode 100644 index 0000000000..11f0ac0edd --- /dev/null +++ b/.chronus/changes/review-patch-sdk-verb-prefix.md @@ -0,0 +1,7 @@ +--- +changeKind: internal +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +Share package-private SDK verb-prefix traversal between PATCH and PUT naming rules. diff --git a/packages/typespec-client-generator-core/src/rules/use-create-for-put.ts b/packages/typespec-client-generator-core/src/rules/use-create-for-put.ts index 979aba880a..947ae7525c 100644 --- a/packages/typespec-client-generator-core/src/rules/use-create-for-put.ts +++ b/packages/typespec-client-generator-core/src/rules/use-create-for-put.ts @@ -1,8 +1,6 @@ import { createRule, fileRef, paramMessage } from "@typespec/compiler"; -import { getAllHttpServices } from "@typespec/http"; import { createTCGCContext } from "../context.js"; -import { AllScopes } from "../internal-utils.js"; -import { getLibraryName } from "../public-utils.js"; +import { getOperationsWithIncorrectVerbPrefix } from "./verb-prefix.js"; export const useCreateForPutRule = createRule({ name: "use-create-for-put", @@ -22,24 +20,15 @@ export const useCreateForPutRule = createRule({ return { root: () => { - const [services] = getAllHttpServices(context.program); - for (const service of services) { - for (const { operation, verb } of service.operations) { - if (verb !== "put") { - continue; - } - - // Check the common SDK name, leaving emitter-specific overrides to emitter rules. - const operationName = getLibraryName(tcgcContext, operation, AllScopes); - if (operationName.toLowerCase().startsWith("create")) { - continue; - } - - context.reportDiagnostic({ - target: operation, - format: { operationName }, - }); - } + for (const { operation, operationName } of getOperationsWithIncorrectVerbPrefix( + tcgcContext, + "put", + "create", + )) { + context.reportDiagnostic({ + target: operation, + format: { operationName }, + }); } }, }; diff --git a/packages/typespec-client-generator-core/src/rules/use-update-for-patch.ts b/packages/typespec-client-generator-core/src/rules/use-update-for-patch.ts index 9ec31904f3..68733a54f0 100644 --- a/packages/typespec-client-generator-core/src/rules/use-update-for-patch.ts +++ b/packages/typespec-client-generator-core/src/rules/use-update-for-patch.ts @@ -1,8 +1,6 @@ import { createRule, fileRef, paramMessage } from "@typespec/compiler"; -import { getAllHttpServices } from "@typespec/http"; import { createTCGCContext } from "../context.js"; -import { AllScopes } from "../internal-utils.js"; -import { getLibraryName } from "../public-utils.js"; +import { getOperationsWithIncorrectVerbPrefix } from "./verb-prefix.js"; export const useUpdateForPatchRule = createRule({ name: "use-update-for-patch", @@ -22,24 +20,15 @@ export const useUpdateForPatchRule = createRule({ return { root: () => { - const [services] = getAllHttpServices(context.program); - for (const service of services) { - for (const { operation, verb } of service.operations) { - if (verb !== "patch") { - continue; - } - - // Check the common SDK name, leaving emitter-specific overrides to emitter rules. - const operationName = getLibraryName(tcgcContext, operation, AllScopes); - if (operationName.toLowerCase().startsWith("update")) { - continue; - } - - context.reportDiagnostic({ - target: operation, - format: { operationName }, - }); - } + for (const { operation, operationName } of getOperationsWithIncorrectVerbPrefix( + tcgcContext, + "patch", + "update", + )) { + context.reportDiagnostic({ + target: operation, + format: { operationName }, + }); } }, }; diff --git a/packages/typespec-client-generator-core/src/rules/verb-prefix.ts b/packages/typespec-client-generator-core/src/rules/verb-prefix.ts new file mode 100644 index 0000000000..cbb3530fd0 --- /dev/null +++ b/packages/typespec-client-generator-core/src/rules/verb-prefix.ts @@ -0,0 +1,22 @@ +import type { Operation } from "@typespec/compiler"; +import { getAllHttpServices, type HttpVerb } from "@typespec/http"; +import type { TCGCContext } from "../interfaces.js"; +import { AllScopes } from "../internal-utils.js"; +import { getLibraryName } from "../public-utils.js"; + +export function* getOperationsWithIncorrectVerbPrefix( + context: TCGCContext, + verb: HttpVerb, + prefix: string, +): Iterable<{ operation: Operation; operationName: string }> { + const [services] = getAllHttpServices(context.program); + for (const service of services) { + for (const endpoint of service.operations) { + if (endpoint.verb !== verb) continue; + const operationName = getLibraryName(context, endpoint.operation, AllScopes); + if (!operationName.toLowerCase().startsWith(prefix)) { + yield { operation: endpoint.operation, operationName }; + } + } + } +} diff --git a/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts b/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts index b696bba5d2..247eec510f 100644 --- a/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts @@ -3,7 +3,7 @@ import { type LinterRuleTester, type TesterInstance, } from "@typespec/compiler/testing"; -import { readFile } from "node:fs/promises"; + import { beforeEach, describe, it, vi } from "vitest"; import { useUpdateForPatchRule } from "../../src/rules/use-update-for-patch.js"; import { ArmTester } from "../tester.js"; @@ -46,19 +46,34 @@ function diagnostic(operationName: string, target = operationName) { }; } -async function documentationSnippet(section: string) { - const source = await readFile( - new URL("../../src/rules/use-update-for-patch.md", import.meta.url), - "utf8", - ); - const snippet = source - .replace(/\r\n/g, "\n") - .split(section)[1] - ?.match(/```typespec\n([\s\S]*?)```/)?.[1]; - if (!snippet) { - throw new Error(`Missing documentation snippet: ${section}`); - } - return `${header} ${widget} ${snippet}`; +function patchTemplateFixture( + section: "#### ❌ Incorrect" | "#### ✅ Correct" | "## Impact" | "## Suppression", +) { + const fixtures = { + "#### ❌ Incorrect": `@armResourceOperations +interface Widgets { + modify is ArmResourcePatchSync; +} +`, + "#### ✅ Correct": `@armResourceOperations +interface Widgets { + update is ArmResourcePatchSync; +} +`, + "## Impact": `@armResourceOperations +interface Widgets { + @Azure.ClientGenerator.Core.clientName("updateWidget") + modify is ArmResourcePatchSync; +} +`, + "## Suppression": `@armResourceOperations +interface Widgets { + #suppress "@azure-tools/typespec-client-generator-core/use-update-for-patch" "Preserve the already-shipped SDK method name." + modify is ArmResourcePatchSync; +} +`, + }; + return `${header} ${widget} ${fixtures[section]}`; } describe("use-update-for-patch", () => { @@ -263,23 +278,23 @@ describe("use-update-for-patch", () => { .toEmitDiagnostics([diagnostic("modify")]); }); - it("validates the exact incorrect documentation example", async () => { + it("validates the exact incorrect template fixture", async () => { await tester - .expect(await documentationSnippet("#### ❌ Incorrect")) + .expect(await patchTemplateFixture("#### ❌ Incorrect")) .toEmitDiagnostics([diagnostic("modify")]); }); - it("validates the exact correct documentation example", async () => { - await tester.expect(await documentationSnippet("#### ✅ Correct")).toBeValid(); + it("validates the exact correct template fixture", async () => { + await tester.expect(await patchTemplateFixture("#### ✅ Correct")).toBeValid(); }); - it("validates the exact common client name documentation example", async () => { - await tester.expect(await documentationSnippet("## Impact")).toBeValid(); + it("validates the exact common client name template fixture", async () => { + await tester.expect(await patchTemplateFixture("## Impact")).toBeValid(); }); - it("validates the exact suppression documentation example", async () => { + it("validates the exact suppression template fixture", async () => { // The direct rule tester collects diagnostics before compiler suppression is applied. - await runner.compile(await documentationSnippet("## Suppression"), { + await runner.compile(await patchTemplateFixture("## Suppression"), { compilerOptions: { linterRuleSet: { enable: { "@azure-tools/typespec-client-generator-core/use-update-for-patch": true }, From 6f0eb98cf42dc0060a5ed1bbb747717696432c25 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 14 Oct 2026 14:17:40 +0800 Subject: [PATCH 07/17] refactor(tcgc): share DELETE and PUT SDK verb-prefix checks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../changes/review-delete-sdk-verb-prefix.md | 7 ++ .../src/rules/use-create-for-put.ts | 31 +++------ .../src/rules/use-delete-for-delete.ts | 30 +++----- .../src/rules/verb-prefix.ts | 22 ++++++ .../test/rules/use-delete-for-delete.test.ts | 69 +++++++++++++++---- 5 files changed, 103 insertions(+), 56 deletions(-) create mode 100644 .chronus/changes/review-delete-sdk-verb-prefix.md create mode 100644 packages/typespec-client-generator-core/src/rules/verb-prefix.ts diff --git a/.chronus/changes/review-delete-sdk-verb-prefix.md b/.chronus/changes/review-delete-sdk-verb-prefix.md new file mode 100644 index 0000000000..76b1fafeb8 --- /dev/null +++ b/.chronus/changes/review-delete-sdk-verb-prefix.md @@ -0,0 +1,7 @@ +--- +changeKind: internal +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +Share package-private SDK verb-prefix traversal between DELETE and PUT naming rules. diff --git a/packages/typespec-client-generator-core/src/rules/use-create-for-put.ts b/packages/typespec-client-generator-core/src/rules/use-create-for-put.ts index 979aba880a..947ae7525c 100644 --- a/packages/typespec-client-generator-core/src/rules/use-create-for-put.ts +++ b/packages/typespec-client-generator-core/src/rules/use-create-for-put.ts @@ -1,8 +1,6 @@ import { createRule, fileRef, paramMessage } from "@typespec/compiler"; -import { getAllHttpServices } from "@typespec/http"; import { createTCGCContext } from "../context.js"; -import { AllScopes } from "../internal-utils.js"; -import { getLibraryName } from "../public-utils.js"; +import { getOperationsWithIncorrectVerbPrefix } from "./verb-prefix.js"; export const useCreateForPutRule = createRule({ name: "use-create-for-put", @@ -22,24 +20,15 @@ export const useCreateForPutRule = createRule({ return { root: () => { - const [services] = getAllHttpServices(context.program); - for (const service of services) { - for (const { operation, verb } of service.operations) { - if (verb !== "put") { - continue; - } - - // Check the common SDK name, leaving emitter-specific overrides to emitter rules. - const operationName = getLibraryName(tcgcContext, operation, AllScopes); - if (operationName.toLowerCase().startsWith("create")) { - continue; - } - - context.reportDiagnostic({ - target: operation, - format: { operationName }, - }); - } + for (const { operation, operationName } of getOperationsWithIncorrectVerbPrefix( + tcgcContext, + "put", + "create", + )) { + context.reportDiagnostic({ + target: operation, + format: { operationName }, + }); } }, }; diff --git a/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.ts b/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.ts index 79be740a25..5aae3c09ae 100644 --- a/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.ts +++ b/packages/typespec-client-generator-core/src/rules/use-delete-for-delete.ts @@ -1,8 +1,6 @@ import { createRule, fileRef, paramMessage } from "@typespec/compiler"; -import { getAllHttpServices } from "@typespec/http"; import { createTCGCContext } from "../context.js"; -import { AllScopes } from "../internal-utils.js"; -import { getLibraryName } from "../public-utils.js"; +import { getOperationsWithIncorrectVerbPrefix } from "./verb-prefix.js"; export const useDeleteForDeleteRule = createRule({ name: "use-delete-for-delete", @@ -22,23 +20,15 @@ export const useDeleteForDeleteRule = createRule({ return { root: () => { - const [services] = getAllHttpServices(context.program); - for (const service of services) { - for (const { operation, verb } of service.operations) { - if (verb !== "delete") { - continue; - } - - const operationName = getLibraryName(tcgcContext, operation, AllScopes); - if (operationName.toLowerCase().startsWith("delete")) { - continue; - } - - context.reportDiagnostic({ - target: operation, - format: { operationName }, - }); - } + for (const { operation, operationName } of getOperationsWithIncorrectVerbPrefix( + tcgcContext, + "delete", + "delete", + )) { + context.reportDiagnostic({ + target: operation, + format: { operationName }, + }); } }, }; diff --git a/packages/typespec-client-generator-core/src/rules/verb-prefix.ts b/packages/typespec-client-generator-core/src/rules/verb-prefix.ts new file mode 100644 index 0000000000..cbb3530fd0 --- /dev/null +++ b/packages/typespec-client-generator-core/src/rules/verb-prefix.ts @@ -0,0 +1,22 @@ +import type { Operation } from "@typespec/compiler"; +import { getAllHttpServices, type HttpVerb } from "@typespec/http"; +import type { TCGCContext } from "../interfaces.js"; +import { AllScopes } from "../internal-utils.js"; +import { getLibraryName } from "../public-utils.js"; + +export function* getOperationsWithIncorrectVerbPrefix( + context: TCGCContext, + verb: HttpVerb, + prefix: string, +): Iterable<{ operation: Operation; operationName: string }> { + const [services] = getAllHttpServices(context.program); + for (const service of services) { + for (const endpoint of service.operations) { + if (endpoint.verb !== verb) continue; + const operationName = getLibraryName(context, endpoint.operation, AllScopes); + if (!operationName.toLowerCase().startsWith(prefix)) { + yield { operation: endpoint.operation, operationName }; + } + } + } +} diff --git a/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts index e601dec5d3..2a9518957f 100644 --- a/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts @@ -6,8 +6,8 @@ import { createTester, expectDiagnosticEmpty, } from "@typespec/compiler/testing"; -import { readFileSync } from "node:fs"; -import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { beforeEach, describe, it, vi } from "vitest"; import { useDeleteForDeleteRule } from "../../src/rules/use-delete-for-delete.js"; import { ArmTester, SimpleTester } from "../tester.js"; @@ -338,7 +338,7 @@ describe("use-delete-for-delete with standard ARM operations", () => { .toBeValid(); }); - describe("published template documentation examples", () => { + describe("standard template template fixtures", () => { let documentationRunner: TesterInstance; let setup: string; let examples: string[]; @@ -361,30 +361,69 @@ describe("use-delete-for-delete with standard ARM operations", () => { useDeleteForDeleteRule, "@azure-tools/typespec-client-generator-core", ); - const documentation = readFileSync( - new URL("../../src/rules/use-delete-for-delete.md", import.meta.url), - "utf8", - ); - const snippets = [...documentation.matchAll(/```typespec\r?\n([\s\S]*?)```/g)].map( - (match) => match[1], - ); - expect(snippets).toHaveLength(5); + + const snippets = [ + `import "@typespec/http"; +import "@typespec/rest"; +import "@azure-tools/typespec-azure-resource-manager"; +import "@azure-tools/typespec-client-generator-core"; + +using TypeSpec.Http; +using TypeSpec.Rest; +using Azure.ResourceManager; +using Azure.ClientGenerator.Core; + +@armProviderNamespace +@service +@armCommonTypesVersion(CommonTypes.Versions.v5) +namespace Example; + +model Widget is ProxyResource { + @key("widgetName") @segment("widgets") name: string; +} + +model WidgetProperties {} +`, + `@armResourceOperations +interface Widgets { + remove is ArmResourceDeleteSync; +} +`, + `@armResourceOperations +interface Widgets { + delete is ArmResourceDeleteSync; +} +`, + `@armResourceOperations +interface Widgets { + @clientName("deleteWidget") + remove is ArmResourceDeleteSync; +} +`, + `@armResourceOperations +interface Widgets { + #suppress "@azure-tools/typespec-client-generator-core/use-delete-for-delete" "Preserve the already-shipped SDK method name." + remove is ArmResourceDeleteSync; +} +`, + ]; + [setup, ...examples] = snippets; }); - it("reports the published incorrect template alias", async () => { + it("reports the standard incorrect template alias", async () => { await tester.expect(`${setup}\n${examples[0]}`).toEmitDiagnostics([diagnostic("remove")]); }); - it("accepts the published correct template alias", async () => { + it("accepts the standard correct template alias", async () => { await tester.expect(`${setup}\n${examples[1]}`).toBeValid(); }); - it("accepts the published template client name override", async () => { + it("accepts the standard template client name override", async () => { await tester.expect(`${setup}\n${examples[2]}`).toBeValid(); }); - it("accepts the published template suppression", async () => { + it("accepts the standard template suppression", async () => { const [, diagnostics] = await documentationRunner.compileAndDiagnose( `${setup}\n${examples[3]}`, { From 5a9015bbb3560e16579f21479a273a60e34f6025 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 14 Oct 2026 17:30:19 +0800 Subject: [PATCH 08/17] test(tcgc): remove PATCH version and suppression-only fixtures Preserve direct valid and invalid names and common or scoped client-name resolution coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/rules/use-update-for-patch.test.ts | 36 +------------------ 1 file changed, 1 insertion(+), 35 deletions(-) diff --git a/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts b/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts index 247eec510f..cdbcefc053 100644 --- a/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts @@ -46,9 +46,7 @@ function diagnostic(operationName: string, target = operationName) { }; } -function patchTemplateFixture( - section: "#### ❌ Incorrect" | "#### ✅ Correct" | "## Impact" | "## Suppression", -) { +function patchTemplateFixture(section: "#### ❌ Incorrect" | "#### ✅ Correct" | "## Impact") { const fixtures = { "#### ❌ Incorrect": `@armResourceOperations interface Widgets { @@ -65,12 +63,6 @@ interface Widgets { @Azure.ClientGenerator.Core.clientName("updateWidget") modify is ArmResourcePatchSync; } -`, - "## Suppression": `@armResourceOperations -interface Widgets { - #suppress "@azure-tools/typespec-client-generator-core/use-update-for-patch" "Preserve the already-shipped SDK method name." - modify is ArmResourcePatchSync; -} `, }; return `${header} ${widget} ${fixtures[section]}`; @@ -263,21 +255,6 @@ describe("use-update-for-patch", () => { .toBeValid(); }); - it("retains historical operations but does not simulate historical names", async () => { - await tester - .expect( - ` - @service @armProviderNamespace @versioned(Versions) - @armCommonTypesVersion(CommonTypes.Versions.v5) - namespace Arm; - enum Versions { v1, v2 } - @removed(Versions.v2) @route("/old") @patch op modify(): string; - @renamedFrom(Versions.v2, "modify") @route("/renamed") @patch op update(): string; - `, - ) - .toEmitDiagnostics([diagnostic("modify")]); - }); - it("validates the exact incorrect template fixture", async () => { await tester .expect(await patchTemplateFixture("#### ❌ Incorrect")) @@ -291,15 +268,4 @@ describe("use-update-for-patch", () => { it("validates the exact common client name template fixture", async () => { await tester.expect(await patchTemplateFixture("## Impact")).toBeValid(); }); - - it("validates the exact suppression template fixture", async () => { - // The direct rule tester collects diagnostics before compiler suppression is applied. - await runner.compile(await patchTemplateFixture("## Suppression"), { - compilerOptions: { - linterRuleSet: { - enable: { "@azure-tools/typespec-client-generator-core/use-update-for-patch": true }, - }, - }, - }); - }); }); From bdb8b3fd8efae3dcb41b5fe05c9d96ac02a311d1 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 14 Oct 2026 17:30:19 +0800 Subject: [PATCH 09/17] test(tcgc): remove DELETE suppression-only fixture Keep native naming and ARM template client-name assertions; remove unused suppression runner plumbing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/rules/use-delete-for-delete.test.ts | 28 +------------------ 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts index 2a9518957f..49b2c484b9 100644 --- a/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts @@ -1,10 +1,8 @@ import { resolvePath } from "@typespec/compiler"; import { - LinterRuleTester, - TesterInstance, createLinterRuleTester, createTester, - expectDiagnosticEmpty, + type LinterRuleTester, } from "@typespec/compiler/testing"; import { beforeEach, describe, it, vi } from "vitest"; @@ -339,7 +337,6 @@ describe("use-delete-for-delete with standard ARM operations", () => { }); describe("standard template template fixtures", () => { - let documentationRunner: TesterInstance; let setup: string; let examples: string[]; @@ -355,7 +352,6 @@ describe("use-delete-for-delete with standard ARM operations", () => { "@azure-tools/typespec-client-generator-core", ], }).createInstance(); - documentationRunner = runner; tester = createLinterRuleTester( runner, useDeleteForDeleteRule, @@ -399,12 +395,6 @@ interface Widgets { @clientName("deleteWidget") remove is ArmResourceDeleteSync; } -`, - `@armResourceOperations -interface Widgets { - #suppress "@azure-tools/typespec-client-generator-core/use-delete-for-delete" "Preserve the already-shipped SDK method name." - remove is ArmResourceDeleteSync; -} `, ]; @@ -422,21 +412,5 @@ interface Widgets { it("accepts the standard template client name override", async () => { await tester.expect(`${setup}\n${examples[2]}`).toBeValid(); }); - - it("accepts the standard template suppression", async () => { - const [, diagnostics] = await documentationRunner.compileAndDiagnose( - `${setup}\n${examples[3]}`, - { - compilerOptions: { - linterRuleSet: { - enable: { - "@azure-tools/typespec-client-generator-core/use-delete-for-delete": true, - }, - }, - }, - }, - ); - expectDiagnosticEmpty(diagnostics); - }); }); }); From 6fcf466d3d487ec62638ab0647cd6d4d98258492 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Thu, 15 Oct 2026 10:29:11 +0800 Subject: [PATCH 10/17] Share standalone ARM fixtures and isolate DELETE dependency proof Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...ete-in-operation-name-2026-9-15-10-24-4.md | 7 ++++ .../test/rules/use-delete-for-delete.test.ts | 31 +++----------- .../test/tester.ts | 42 +++++++++---------- ...te-for-delete-dependency-isolation.test.ts | 25 +++++++++++ .../content/docs/docs/howtos/ARM/arm-rules.md | 1 + 5 files changed, 60 insertions(+), 46 deletions(-) create mode 100644 .chronus/changes/promote-lintdiff-delete-in-operation-name-2026-9-15-10-24-4.md create mode 100644 packages/typespec-client-generator-core/test/use-delete-for-delete-dependency-isolation.test.ts diff --git a/.chronus/changes/promote-lintdiff-delete-in-operation-name-2026-9-15-10-24-4.md b/.chronus/changes/promote-lintdiff-delete-in-operation-name-2026-9-15-10-24-4.md new file mode 100644 index 0000000000..00042cb642 --- /dev/null +++ b/.chronus/changes/promote-lintdiff-delete-in-operation-name-2026-9-15-10-24-4.md @@ -0,0 +1,7 @@ +--- +changeKind: internal +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +Reuse unwrapped ARM test setup and retain emitter-isolation coverage in a dedicated integration test. \ No newline at end of file diff --git a/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts index 49b2c484b9..76edecf870 100644 --- a/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts @@ -1,17 +1,8 @@ -import { resolvePath } from "@typespec/compiler"; -import { - createLinterRuleTester, - createTester, - type LinterRuleTester, -} from "@typespec/compiler/testing"; - -import { beforeEach, describe, it, vi } from "vitest"; -import { useDeleteForDeleteRule } from "../../src/rules/use-delete-for-delete.js"; -import { ArmTester, SimpleTester } from "../tester.js"; +import { createLinterRuleTester, type LinterRuleTester } from "@typespec/compiler/testing"; -vi.mock("@azure-tools/typespec-autorest", () => { - throw new Error("Native lint must not load the AutoRest emitter."); -}); +import { beforeEach, describe, it } from "vitest"; +import { useDeleteForDeleteRule } from "../../src/rules/use-delete-for-delete.js"; +import { ArmBaseTester, ArmTester, SimpleTester } from "../tester.js"; function diagnostic(operationName: string, target = operationName) { return { @@ -336,22 +327,12 @@ describe("use-delete-for-delete with standard ARM operations", () => { .toBeValid(); }); - describe("standard template template fixtures", () => { + describe("standalone standard template fixtures", () => { let setup: string; let examples: string[]; beforeEach(async () => { - const runner = await createTester(resolvePath(import.meta.dirname, "..", ".."), { - libraries: [ - "@typespec/http", - "@typespec/rest", - "@typespec/versioning", - "@typespec/openapi", - "@azure-tools/typespec-azure-core", - "@azure-tools/typespec-azure-resource-manager", - "@azure-tools/typespec-client-generator-core", - ], - }).createInstance(); + const runner = await ArmBaseTester.createInstance(); tester = createLinterRuleTester( runner, useDeleteForDeleteRule, diff --git a/packages/typespec-client-generator-core/test/tester.ts b/packages/typespec-client-generator-core/test/tester.ts index 9bf184a683..5cf8340009 100644 --- a/packages/typespec-client-generator-core/test/tester.ts +++ b/packages/typespec-client-generator-core/test/tester.ts @@ -161,10 +161,7 @@ ${x} `, ); -/** - * Tester for TCGC tests with Azure Resource Manager library support. - */ -export const ArmTester = createTester(resolvePath(import.meta.dirname, ".."), { +export const ArmBaseTester = createTester(resolvePath(import.meta.dirname, ".."), { libraries: [ "@typespec/http", "@typespec/rest", @@ -174,23 +171,26 @@ export const ArmTester = createTester(resolvePath(import.meta.dirname, ".."), { "@azure-tools/typespec-azure-resource-manager", "@azure-tools/typespec-client-generator-core", ], -}) - .import( - "@typespec/http", - "@typespec/rest", - "@typespec/versioning", - "@azure-tools/typespec-azure-core", - "@azure-tools/typespec-azure-resource-manager", - "@azure-tools/typespec-client-generator-core", - ) - .using( - "Http", - "Rest", - "Versioning", - "Azure.Core", - "Azure.ResourceManager", - "Azure.ClientGenerator.Core", - ); +}); + +/** + * Tester for TCGC tests with Azure Resource Manager library support. + */ +export const ArmTester = ArmBaseTester.import( + "@typespec/http", + "@typespec/rest", + "@typespec/versioning", + "@azure-tools/typespec-azure-core", + "@azure-tools/typespec-azure-resource-manager", + "@azure-tools/typespec-client-generator-core", +).using( + "Http", + "Rest", + "Versioning", + "Azure.Core", + "Azure.ResourceManager", + "Azure.ClientGenerator.Core", +); /** * Tester with a built-in Azure Resource Manager service namespace. diff --git a/packages/typespec-client-generator-core/test/use-delete-for-delete-dependency-isolation.test.ts b/packages/typespec-client-generator-core/test/use-delete-for-delete-dependency-isolation.test.ts new file mode 100644 index 0000000000..16ea3fa6c7 --- /dev/null +++ b/packages/typespec-client-generator-core/test/use-delete-for-delete-dependency-isolation.test.ts @@ -0,0 +1,25 @@ +import { createLinterRuleTester } from "@typespec/compiler/testing"; +import { it, vi } from "vitest"; +import { useDeleteForDeleteRule } from "../src/rules/use-delete-for-delete.js"; +import { SimpleTester } from "./tester.js"; + +vi.mock("@azure-tools/typespec-autorest", () => { + throw new Error("Native lint must not load the AutoRest emitter."); +}); + +it("runs native DELETE naming without loading the AutoRest emitter", async () => { + const tester = createLinterRuleTester( + await SimpleTester.createInstance(), + useDeleteForDeleteRule, + "@azure-tools/typespec-client-generator-core", + ); + await tester + .expect( + ` + @service namespace Example; + @delete @route("/widgets/{id}") + op deleteWidget(@path id: string): void; + `, + ) + .toBeValid(); +}); diff --git a/website/src/content/docs/docs/howtos/ARM/arm-rules.md b/website/src/content/docs/docs/howtos/ARM/arm-rules.md index a8863c6d41..37fc9bb878 100644 --- a/website/src/content/docs/docs/howtos/ARM/arm-rules.md +++ b/website/src/content/docs/docs/howtos/ARM/arm-rules.md @@ -143,6 +143,7 @@ The tables below provide guidance to rule authors and ARM reviewers on how to ev | [`use-api-version`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-api-version/) | [`ApiVersionParameterRequired`](https://github.com/Azure/azure-openapi-validator/blob/main/docs/api-version-parameter-required.md) | **API, SDK.** An operation without the standard API version parameter cannot evolve safely and is difficult for SDKs to represent consistently. | | [`use-application-json-content-type`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-application-json-content-type/) | [`NonApplicationJsonType`](https://github.com/Azure/azure-openapi-validator/blob/main/packages/rulesets/src/spectral/az-arm.ts) | **API, SDK.** Non-JSON request or response bodies are incompatible with the ARM contract and produce inconsistent serialization. | | [`use-create-for-put`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-create-for-put/) | [PutInOperationName (R1006)](https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/openapi-authoring-automated-guidelines.md#r1006) | **SDK.** TCGC checks the common SDK method name, including unscoped `@clientName` overrides; renaming a shipped method may break SDK compatibility. | +| [`use-delete-for-delete`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-delete-for-delete/) | [`DeleteInOperationName`](https://github.com/Azure/azure-openapi-validator/blob/main/docs/delete-in-operation-name.md) | **SDK.** TCGC checks that common SDK names for DELETE operations start with `delete`, including unscoped `@clientName` overrides. Renaming a shipped method may break SDK compatibility. | | [`use-interface`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-interface/) | — | **API, SDK, Emitters.** Resource operations outside interfaces can break ARM resource-operation modeling and downstream tooling assumptions. | | [`use-model-request-body`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-model-request-body/) | [`ParametersSchemaAsTypeObject`](https://github.com/Azure/azure-openapi-validator/blob/main/docs/parameters-schema-as-type-object.md) | **API, SDK.** Non-model, array, and record request bodies cannot evolve with additional properties without a breaking change. | | [`use-operation-decorator`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-operation-decorator/) | — | **API, SDK, Emitters.** Missing or mismatched decorators prevent resource operations from being associated with the correct resource, which can break SDK generation and resource-aware tooling. | From 90e1e7d6e5f752e38d290bdf11fa1995821c0179 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Thu, 15 Oct 2026 10:33:03 +0800 Subject: [PATCH 11/17] Move native SDK linter emitter isolation to package integration tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../package/native-linter-isolation.test.ts | 41 +++++++++++++++++++ .../test/rules/use-create-for-put.test.ts | 6 +-- .../test/rules/use-update-for-patch.test.ts | 6 +-- 3 files changed, 43 insertions(+), 10 deletions(-) create mode 100644 packages/typespec-client-generator-core/test/package/native-linter-isolation.test.ts diff --git a/packages/typespec-client-generator-core/test/package/native-linter-isolation.test.ts b/packages/typespec-client-generator-core/test/package/native-linter-isolation.test.ts new file mode 100644 index 0000000000..eb2646f1be --- /dev/null +++ b/packages/typespec-client-generator-core/test/package/native-linter-isolation.test.ts @@ -0,0 +1,41 @@ +import { createLinterRuleTester } from "@typespec/compiler/testing"; +import { describe, it, vi } from "vitest"; +import { useCreateForPutRule } from "../../src/rules/use-create-for-put.js"; +import { useUpdateForPatchRule } from "../../src/rules/use-update-for-patch.js"; +import { ArmTester } from "../tester.js"; + +vi.mock("@azure-tools/typespec-autorest", () => { + throw new Error("Native lint must not load the AutoRest emitter."); +}); + +describe("native SDK naming linter package integration", () => { + for (const [rule, verb, prefix] of [ + [useCreateForPutRule, "put", "create"], + [useUpdateForPatchRule, "patch", "update"], + ] as const) { + it(`runs ${rule.name} without loading the AutoRest emitter`, async () => { + const tester = createLinterRuleTester( + await ArmTester.createInstance(), + rule, + "@azure-tools/typespec-client-generator-core", + ); + await tester + .expect( + ` + @service namespace Service; + @${verb} @route("/invalid") op modify(): string; + @clientName("${prefix}Widget") + @${verb} @route("/valid") op change(): string; + `, + ) + .toEmitDiagnostics([ + { + code: `@azure-tools/typespec-client-generator-core/${rule.name}`, + severity: "warning", + target: "modify", + message: `${verb.toUpperCase()} SDK method name 'modify' should start with '${prefix}'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.`, + }, + ]); + }); + } +}); diff --git a/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts b/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts index 4b27ceda2b..a6ac8bcbd9 100644 --- a/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts @@ -1,12 +1,8 @@ import { LinterRuleTester, createLinterRuleTester } from "@typespec/compiler/testing"; -import { beforeEach, describe, it, vi } from "vitest"; +import { beforeEach, describe, it } from "vitest"; import { useCreateForPutRule } from "../../src/rules/use-create-for-put.js"; import { ArmTester } from "../tester.js"; -vi.mock("@azure-tools/typespec-autorest", () => { - throw new Error("Native lint must not load the AutoRest emitter."); -}); - let tester: LinterRuleTester; beforeEach(async () => { diff --git a/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts b/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts index cdbcefc053..ab1410c41f 100644 --- a/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts @@ -4,14 +4,10 @@ import { type TesterInstance, } from "@typespec/compiler/testing"; -import { beforeEach, describe, it, vi } from "vitest"; +import { beforeEach, describe, it } from "vitest"; import { useUpdateForPatchRule } from "../../src/rules/use-update-for-patch.js"; import { ArmTester } from "../tester.js"; -vi.mock("@azure-tools/typespec-autorest", () => { - throw new Error("Native lint must not load the AutoRest emitter."); -}); - let tester: LinterRuleTester; let runner: TesterInstance; From 6b9be2357ced1385b7a0a953b7f87c7a56c75637 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Mon, 19 Oct 2026 12:18:02 +0800 Subject: [PATCH 12/17] test(tcgc): simplify naming suites and package isolation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../changes/review-patch-sdk-verb-prefix.md | 7 - .../package/native-linter-isolation.test.ts | 39 +-- .../test/rules/use-create-for-put.test.ts | 330 +++++++++--------- .../test/rules/use-update-for-patch.test.ts | 308 ++++++++-------- 4 files changed, 321 insertions(+), 363 deletions(-) delete mode 100644 .chronus/changes/review-patch-sdk-verb-prefix.md diff --git a/.chronus/changes/review-patch-sdk-verb-prefix.md b/.chronus/changes/review-patch-sdk-verb-prefix.md deleted file mode 100644 index 11f0ac0edd..0000000000 --- a/.chronus/changes/review-patch-sdk-verb-prefix.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -changeKind: internal -packages: - - "@azure-tools/typespec-client-generator-core" ---- - -Share package-private SDK verb-prefix traversal between PATCH and PUT naming rules. diff --git a/packages/typespec-client-generator-core/test/package/native-linter-isolation.test.ts b/packages/typespec-client-generator-core/test/package/native-linter-isolation.test.ts index eb2646f1be..c7b7064de6 100644 --- a/packages/typespec-client-generator-core/test/package/native-linter-isolation.test.ts +++ b/packages/typespec-client-generator-core/test/package/native-linter-isolation.test.ts @@ -1,41 +1,10 @@ -import { createLinterRuleTester } from "@typespec/compiler/testing"; -import { describe, it, vi } from "vitest"; -import { useCreateForPutRule } from "../../src/rules/use-create-for-put.js"; -import { useUpdateForPatchRule } from "../../src/rules/use-update-for-patch.js"; -import { ArmTester } from "../tester.js"; +import { expect, it, vi } from "vitest"; +import * as library from "../../src/index.js"; vi.mock("@azure-tools/typespec-autorest", () => { throw new Error("Native lint must not load the AutoRest emitter."); }); -describe("native SDK naming linter package integration", () => { - for (const [rule, verb, prefix] of [ - [useCreateForPutRule, "put", "create"], - [useUpdateForPatchRule, "patch", "update"], - ] as const) { - it(`runs ${rule.name} without loading the AutoRest emitter`, async () => { - const tester = createLinterRuleTester( - await ArmTester.createInstance(), - rule, - "@azure-tools/typespec-client-generator-core", - ); - await tester - .expect( - ` - @service namespace Service; - @${verb} @route("/invalid") op modify(): string; - @clientName("${prefix}Widget") - @${verb} @route("/valid") op change(): string; - `, - ) - .toEmitDiagnostics([ - { - code: `@azure-tools/typespec-client-generator-core/${rule.name}`, - severity: "warning", - target: "modify", - message: `${verb.toUpperCase()} SDK method name 'modify' should start with '${prefix}'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.`, - }, - ]); - }); - } +it("loads the SDK public entry point without the AutoRest emitter", () => { + expect(library.$lib.name).toBe("@azure-tools/typespec-client-generator-core"); }); diff --git a/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts b/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts index a6ac8bcbd9..8b8d5c7ba3 100644 --- a/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts @@ -1,5 +1,5 @@ import { LinterRuleTester, createLinterRuleTester } from "@typespec/compiler/testing"; -import { beforeEach, describe, it } from "vitest"; +import { beforeEach, it } from "vitest"; import { useCreateForPutRule } from "../../src/rules/use-create-for-put.js"; import { ArmTester } from "../tester.js"; @@ -36,165 +36,164 @@ function diagnostic(operationName: string, target = operationName) { }; } -describe("use-create-for-put", () => { - it("accepts an unscoped client name override", async () => { - await tester - .expect(`${header} @clientName("createWidget") @put @route("/item") op set(): string;`) - .toBeValid(); - }); +it("accepts an unscoped client name override", async () => { + await tester + .expect(`${header} @clientName("createWidget") @put @route("/item") op set(): string;`) + .toBeValid(); +}); - it("reports an invalid unscoped client name on the authored operation", async () => { - await tester - .expect(`${header} @clientName("replaceWidget") @put @route("/item") op create(): string;`) - .toEmitDiagnostics([diagnostic("replaceWidget", "create")]); - }); +it("reports an invalid unscoped client name on the authored operation", async () => { + await tester + .expect(`${header} @clientName("replaceWidget") @put @route("/item") op create(): string;`) + .toEmitDiagnostics([diagnostic("replaceWidget", "create")]); +}); - it("ignores emitter-scoped overrides of a compliant default name", async () => { - await tester - .expect( - `${header} @clientName("setWidget", "csharp") @put @route("/item") op create(): string;`, - ) - .toBeValid(); - }); +it("ignores emitter-scoped overrides of a compliant default name", async () => { + await tester + .expect( + `${header} @clientName("setWidget", "csharp") @put @route("/item") op create(): string;`, + ) + .toBeValid(); +}); - it("does not let an emitter-scoped override mask an invalid default name", async () => { - await tester - .expect( - `${header} @clientName("createWidget", "python") @put @route("/item") op set(): string;`, - ) - .toEmitDiagnostics([diagnostic("set")]); - }); +it("does not let an emitter-scoped override mask an invalid default name", async () => { + await tester + .expect( + `${header} @clientName("createWidget", "python") @put @route("/item") op set(): string;`, + ) + .toEmitDiagnostics([diagnostic("set")]); +}); - it("uses the unscoped name when scoped and unscoped overrides coexist", async () => { - await tester - .expect( - `${header} +it("uses the unscoped name when scoped and unscoped overrides coexist", async () => { + await tester + .expect( + `${header} @clientName("createWidget") @clientName("setWidget", "csharp") @put @route("/item") op set(): string;`, - ) - .toBeValid(); - }); + ) + .toBeValid(); +}); - it("honors client names on standard ARM template aliases", async () => { - await tester - .expect( - `${header} +it("honors client names on standard ARM template aliases", async () => { + await tester + .expect( + `${header} ${widget} @armResourceOperations interface Widgets { @clientName("createOrUpdate") set is ArmResourceCreateOrReplaceAsync; }`, - ) - .toBeValid(); - }); + ) + .toBeValid(); +}); - it("normalizes exact client names", async () => { - await tester - .expect( - `${header} +it("normalizes exact client names", async () => { + await tester + .expect( + `${header} @clientName(exact("CreateWidget")) @put @route("/item") op set(): string;`, - ) - .toBeValid(); - }); + ) + .toBeValid(); +}); - it("uses the friendly name fallback", async () => { - await tester - .expect( - `${header} +it("uses the friendly name fallback", async () => { + await tester + .expect( + `${header} @friendlyName("createWidget") @put @route("/item") op set(): string;`, - ) - .toBeValid(); - }); - - // cspell:ignore creat - it.each(["set", "put", "recreate", "replace", "c", "creat"])( - "rejects PUT name %s", - async (name) => { - await tester - .expect(`${header} @put @route("/item") op ${name}(): string;`) - .toEmitDiagnostics([diagnostic(name)]); - }, - ); - - it.each(["create", "createOrUpdate", "createOrReplace", "createWidget", "CreateWidget"])( - "accepts native create prefix %s", - async (name) => { - await tester.expect(`${header} @put @route("/item") op ${name}(): string;`).toBeValid(); - }, - ); + ) + .toBeValid(); +}); - it.each(["get", "post", "patch", "delete", "head"])("ignores HTTP verb %s", async (verb) => { +// cspell:ignore creat +it.each(["set", "put", "recreate", "replace", "c", "creat"])( + "rejects PUT name %s", + async (name) => { await tester - .expect(`${header} @${verb} @route("/item") op set(): { @statusCode code: 204 };`) - .toBeValid(); - }); + .expect(`${header} @put @route("/item") op ${name}(): string;`) + .toEmitDiagnostics([diagnostic(name)]); + }, +); - it.each([200, 201, 202, 204])( - "checks PUT names independently of response code %s", - async (code) => { - await tester - .expect(`${header} @put @route("/item") op replace(): { @statusCode code: ${code} };`) - .toEmitDiagnostics([diagnostic("replace")]); - }, - ); +it.each(["create", "createOrUpdate", "createOrReplace", "createWidget", "CreateWidget"])( + "accepts native create prefix %s", + async (name) => { + await tester.expect(`${header} @put @route("/item") op ${name}(): string;`).toBeValid(); + }, +); - it("does not diagnose generic operation declarations or instantiations", async () => { +it.each(["get", "post", "patch", "delete", "head"])("ignores HTTP verb %s", async (verb) => { + await tester + .expect(`${header} @${verb} @route("/item") op set(): { @statusCode code: 204 };`) + .toBeValid(); +}); + +it.each([200, 201, 202, 204])( + "checks PUT names independently of response code %s", + async (code) => { await tester - .expect( - `${header} + .expect(`${header} @put @route("/item") op replace(): { @statusCode code: ${code} };`) + .toEmitDiagnostics([diagnostic("replace")]); + }, +); + +it("does not diagnose generic operation declarations or instantiations", async () => { + await tester + .expect( + `${header} @put op PutTemplate(@body body: T): T; @route("/item") op create is PutTemplate; interface Templates { @put op put(@body body: T): T; } `, - ) - .toBeValid(); - }); + ) + .toBeValid(); +}); - it("checks concrete template aliases and inherited interface endpoints once each", async () => { - await tester - .expect( - `${header} +it("checks concrete template aliases and inherited interface endpoints once each", async () => { + await tester + .expect( + `${header} @put op PutTemplate(@body body: T): T; interface Templates { @put op set(@body body: T): T; } @route("/alias") op replace is PutTemplate; @route("/inherited") interface Items extends Templates {} `, - ) - .toEmitDiagnostics([diagnostic("replace"), diagnostic("set")]); - }); + ) + .toEmitDiagnostics([diagnostic("replace"), diagnostic("set")]); +}); - it("checks endpoints inside nested ARM namespaces", async () => { - await tester - .expect( - `${header} +it("checks endpoints inside nested ARM namespaces", async () => { + await tester + .expect( + `${header} namespace Nested { @route("/item") interface Items { @put op set(): string; } } `, - ) - .toEmitDiagnostics([diagnostic("set")]); - }); + ) + .toEmitDiagnostics([diagnostic("set")]); +}); - it("does not let the interface name or underscores mask a non-create operation", async () => { - await tester - .expect( - `${header} +it("does not let the interface name or underscores mask a non-create operation", async () => { + await tester + .expect( + `${header} @route("/item") interface CreateWidgets { @put op set(): string; } @route("/other") interface Widgets { @put op set_Create(): string; } `, - ) - .toEmitDiagnostics([diagnostic("set"), diagnostic("set_Create")]); - }); + ) + .toEmitDiagnostics([diagnostic("set"), diagnostic("set_Create")]); +}); - it("retains historical operations but does not simulate historical names", async () => { - await tester - .expect( - ` +it("retains historical operations but does not simulate historical names", async () => { + await tester + .expect( + ` @service @armProviderNamespace @versioned(Versions) @armCommonTypesVersion(CommonTypes.Versions.v5) namespace Arm; @@ -202,26 +201,26 @@ describe("use-create-for-put", () => { @removed(Versions.v2) @route("/old") @put op set(): string; @renamedFrom(Versions.v2, "replace") @route("/renamed") @put op create(): string; `, - ) - .toEmitDiagnostics([diagnostic("set")]); - }); + ) + .toEmitDiagnostics([diagnostic("set")]); +}); - it("checks ordinary service namespaces without provider metadata", async () => { - await tester - .expect( - ` +it("checks ordinary service namespaces without provider metadata", async () => { + await tester + .expect( + ` @service namespace Contoso { @put @route("/item") op set(): string; } `, - ) - .toEmitDiagnostics([diagnostic("set")]); - }); + ) + .toEmitDiagnostics([diagnostic("set")]); +}); - it("checks nested service namespaces without provider metadata", async () => { - await tester - .expect( - ` +it("checks nested service namespaces without provider metadata", async () => { + await tester + .expect( + ` namespace Contoso { @service namespace Management { namespace Nested { @@ -230,14 +229,14 @@ describe("use-create-for-put", () => { } } `, - ) - .toEmitDiagnostics([diagnostic("replace")]); - }); + ) + .toEmitDiagnostics([diagnostic("replace")]); +}); - it("ignores non-service helpers and Azure library declarations", async () => { - await tester - .expect( - ` +it("ignores non-service helpers and Azure library declarations", async () => { + await tester + .expect( + ` @put @route("/outside") op set(): string; namespace Customizations { @put op replace(): string; } namespace Azure.Core { @put op PutTemplate(@body body: T): T; } @@ -246,14 +245,14 @@ describe("use-create-for-put", () => { @put @route("/item") op create(): string; } `, - ) - .toBeValid(); - }); + ) + .toBeValid(); +}); - it("reports the authored setWidgetConfig name despite an explicit operationId", async () => { - await tester - .expect( - ` +it("reports the authored setWidgetConfig name despite an explicit operationId", async () => { + await tester + .expect( + ` ${header} @TypeSpec.OpenAPI.operationId("Widgets_Create") @route("/config") @put @@ -262,45 +261,44 @@ describe("use-create-for-put", () => { @body result: { config: string }; }; `, - ) - .toEmitDiagnostics([diagnostic("setWidgetConfig")]); - }); + ) + .toEmitDiagnostics([diagnostic("setWidgetConfig")]); +}); - it("reports a standard ARM create template aliased as set", async () => { - await tester - .expect( - `${header} +it("reports a standard ARM create template aliased as set", async () => { + await tester + .expect( + `${header} ${widget} @armResourceOperations interface Widgets { set is ArmResourceCreateOrReplaceAsync; } `, - ) - .toEmitDiagnostics([diagnostic("set")]); - }); + ) + .toEmitDiagnostics([diagnostic("set")]); +}); - it("allows a standard ARM create template aliased as createOrUpdate", async () => { - await tester - .expect( - `${header} +it("allows a standard ARM create template aliased as createOrUpdate", async () => { + await tester + .expect( + `${header} ${widget} @armResourceOperations interface Widgets { createOrUpdate is ArmResourceCreateOrReplaceAsync; } `, - ) - .toBeValid(); - }); + ) + .toBeValid(); +}); - it("ignores explicit operationId overrides of a compliant authored name", async () => { - await tester - .expect( - ` +it("ignores explicit operationId overrides of a compliant authored name", async () => { + await tester + .expect( + ` ${header} @TypeSpec.OpenAPI.operationId("Widgets_Set") @put @route("/item") op create(): string; `, - ) - .toBeValid(); - }); + ) + .toBeValid(); }); diff --git a/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts b/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts index ab1410c41f..34c6ce9837 100644 --- a/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-update-for-patch.test.ts @@ -4,7 +4,7 @@ import { type TesterInstance, } from "@typespec/compiler/testing"; -import { beforeEach, describe, it } from "vitest"; +import { beforeEach, it } from "vitest"; import { useUpdateForPatchRule } from "../../src/rules/use-update-for-patch.js"; import { ArmTester } from "../tester.js"; @@ -64,204 +64,202 @@ interface Widgets { return `${header} ${widget} ${fixtures[section]}`; } -describe("use-update-for-patch", () => { - it("accepts an unscoped client name override", async () => { - await tester - .expect(`${header} @clientName("updateWidget") @patch @route("/item") op modify(): string;`) - .toBeValid(); - }); +it("accepts an unscoped client name override", async () => { + await tester + .expect(`${header} @clientName("updateWidget") @patch @route("/item") op modify(): string;`) + .toBeValid(); +}); - it("reports an invalid unscoped client name on the authored operation", async () => { - await tester - .expect(`${header} @clientName("modifyWidget") @patch @route("/item") op update(): string;`) - .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); - }); +it("reports an invalid unscoped client name on the authored operation", async () => { + await tester + .expect(`${header} @clientName("modifyWidget") @patch @route("/item") op update(): string;`) + .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); +}); - it("ignores emitter-scoped overrides of a compliant default name", async () => { - await tester - .expect( - `${header} @clientName("modifyWidget", "csharp") @patch @route("/item") op update(): string;`, - ) - .toBeValid(); - }); +it("ignores emitter-scoped overrides of a compliant default name", async () => { + await tester + .expect( + `${header} @clientName("modifyWidget", "csharp") @patch @route("/item") op update(): string;`, + ) + .toBeValid(); +}); - it("does not let an emitter-scoped override mask an invalid default name", async () => { - await tester - .expect( - `${header} @clientName("updateWidget", "python") @patch @route("/item") op modify(): string;`, - ) - .toEmitDiagnostics([diagnostic("modify")]); - }); +it("does not let an emitter-scoped override mask an invalid default name", async () => { + await tester + .expect( + `${header} @clientName("updateWidget", "python") @patch @route("/item") op modify(): string;`, + ) + .toEmitDiagnostics([diagnostic("modify")]); +}); - it("uses the unscoped name when scoped and unscoped overrides coexist", async () => { - await tester - .expect( - `${header} +it("uses the unscoped name when scoped and unscoped overrides coexist", async () => { + await tester + .expect( + `${header} @clientName("updateWidget") @clientName("modifyWidget", "csharp") @patch @route("/item") op modify(): string;`, - ) - .toBeValid(); - }); + ) + .toBeValid(); +}); - it("does not let a scoped name mask an invalid unscoped override", async () => { - await tester - .expect( - `${header} +it("does not let a scoped name mask an invalid unscoped override", async () => { + await tester + .expect( + `${header} @clientName("modifyWidget") @clientName("updateWidget", "python") @patch @route("/item") op update(): string;`, - ) - .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); - }); + ) + .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); +}); - it("honors client names on standard ARM patch template aliases", async () => { - await tester - .expect( - `${header} ${widget} +it("honors client names on standard ARM patch template aliases", async () => { + await tester + .expect( + `${header} ${widget} @armResourceOperations interface Widgets { @clientName("updateWidget") modify is ArmResourcePatchSync; }`, - ) - .toBeValid(); - }); + ) + .toBeValid(); +}); - it("reports invalid common client names on standard ARM patch template aliases", async () => { - await tester - .expect( - `${header} ${widget} +it("reports invalid common client names on standard ARM patch template aliases", async () => { + await tester + .expect( + `${header} ${widget} @armResourceOperations interface Widgets { @clientName("modifyWidget") update is ArmResourcePatchSync; }`, - ) - .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); - }); + ) + .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); +}); - it("normalizes exact client names", async () => { - await tester - .expect( - `${header} @clientName(exact("UpdateWidget")) @patch @route("/item") op modify(): string;`, - ) - .toBeValid(); - }); +it("normalizes exact client names", async () => { + await tester + .expect( + `${header} @clientName(exact("UpdateWidget")) @patch @route("/item") op modify(): string;`, + ) + .toBeValid(); +}); - it("reports invalid exact client names on the authored operation", async () => { - await tester - .expect( - `${header} @clientName(exact("ModifyWidget")) @patch @route("/item") op update(): string;`, - ) - .toEmitDiagnostics([diagnostic("ModifyWidget", "update")]); - }); +it("reports invalid exact client names on the authored operation", async () => { + await tester + .expect( + `${header} @clientName(exact("ModifyWidget")) @patch @route("/item") op update(): string;`, + ) + .toEmitDiagnostics([diagnostic("ModifyWidget", "update")]); +}); - it("uses the friendly name fallback", async () => { - await tester - .expect(`${header} @friendlyName("updateWidget") @patch @route("/item") op modify(): string;`) - .toBeValid(); - }); +it("uses the friendly name fallback", async () => { + await tester + .expect(`${header} @friendlyName("updateWidget") @patch @route("/item") op modify(): string;`) + .toBeValid(); +}); - it("reports an invalid friendly name on the authored operation", async () => { - await tester - .expect(`${header} @friendlyName("modifyWidget") @patch @route("/item") op update(): string;`) - .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); - }); +it("reports an invalid friendly name on the authored operation", async () => { + await tester + .expect(`${header} @friendlyName("modifyWidget") @patch @route("/item") op update(): string;`) + .toEmitDiagnostics([diagnostic("modifyWidget", "update")]); +}); - it("prefers a common client name over a friendly name", async () => { - await tester - .expect( - `${header} +it("prefers a common client name over a friendly name", async () => { + await tester + .expect( + `${header} @clientName("updateWidget") @friendlyName("modifyWidget") @patch @route("/item") op modify(): string;`, - ) - .toBeValid(); - }); - - // cspell:ignore updat - it.each(["patch", "modify", "create", "patch_Update", "u", "updat"])( - "rejects PATCH name %s", - async (name) => { - await tester - .expect(`${header} @patch @route("/item") op ${name}(): string;`) - .toEmitDiagnostics([diagnostic(name)]); - }, - ); - - it.each(["update", "updateWidget", "updateTags", "UpdateTags", "UPDATE", "uPdAtEWidget"])( - "accepts update prefix %s", - async (name) => { - await tester.expect(`${header} @patch @route("/item") op ${name}(): string;`).toBeValid(); - }, - ); + ) + .toBeValid(); +}); - it.each(["get", "put", "post", "delete", "head"])("ignores HTTP verb %s", async (verb) => { +// cspell:ignore updat +it.each(["patch", "modify", "create", "patch_Update", "u", "updat"])( + "rejects PATCH name %s", + async (name) => { await tester - .expect(`${header} @${verb} @route("/item") op modify(): { @statusCode code: 204 };`) - .toBeValid(); - }); - - it.each([200, 201, 202, 204])( - "checks PATCH names independently of response code %s", - async (code) => { - await tester - .expect(`${header} @patch @route("/item") op modify(): { @statusCode code: ${code} };`) - .toEmitDiagnostics([diagnostic("modify")]); - }, - ); + .expect(`${header} @patch @route("/item") op ${name}(): string;`) + .toEmitDiagnostics([diagnostic(name)]); + }, +); + +it.each(["update", "updateWidget", "updateTags", "UpdateTags", "UPDATE", "uPdAtEWidget"])( + "accepts update prefix %s", + async (name) => { + await tester.expect(`${header} @patch @route("/item") op ${name}(): string;`).toBeValid(); + }, +); + +it.each(["get", "put", "post", "delete", "head"])("ignores HTTP verb %s", async (verb) => { + await tester + .expect(`${header} @${verb} @route("/item") op modify(): { @statusCode code: 204 };`) + .toBeValid(); +}); - it("does not let an interface name mask the operation prefix", async () => { +it.each([200, 201, 202, 204])( + "checks PATCH names independently of response code %s", + async (code) => { await tester - .expect(`${header} @route("/item") interface UpdateWidgets { @patch op modify(): string; }`) + .expect(`${header} @patch @route("/item") op modify(): { @statusCode code: ${code} };`) .toEmitDiagnostics([diagnostic("modify")]); - }); + }, +); - it("checks ordinary service namespaces without provider metadata", async () => { - await tester - .expect(`@service namespace Contoso { @patch @route("/item") op modify(): string; }`) - .toEmitDiagnostics([diagnostic("modify")]); - }); +it("does not let an interface name mask the operation prefix", async () => { + await tester + .expect(`${header} @route("/item") interface UpdateWidgets { @patch op modify(): string; }`) + .toEmitDiagnostics([diagnostic("modify")]); +}); - it("reports distinct offending endpoints on their authored operations", async () => { - await tester - .expect( - `${header} +it("checks ordinary service namespaces without provider metadata", async () => { + await tester + .expect(`@service namespace Contoso { @patch @route("/item") op modify(): string; }`) + .toEmitDiagnostics([diagnostic("modify")]); +}); + +it("reports distinct offending endpoints on their authored operations", async () => { + await tester + .expect( + `${header} @patch @route("/first") op modifyFirst(): string; @patch @route("/second") op modifySecond(): string;`, - ) - .toEmitDiagnostics([diagnostic("modifyFirst"), diagnostic("modifySecond")]); - }); + ) + .toEmitDiagnostics([diagnostic("modifyFirst"), diagnostic("modifySecond")]); +}); - it("ignores an operationId that would mask an invalid common SDK name", async () => { - await tester - .expect( - `${header} +it("ignores an operationId that would mask an invalid common SDK name", async () => { + await tester + .expect( + `${header} @TypeSpec.OpenAPI.operationId("Widgets_Update") @patch @route("/item") op modify(): string;`, - ) - .toEmitDiagnostics([diagnostic("modify")]); - }); + ) + .toEmitDiagnostics([diagnostic("modify")]); +}); - it("ignores an operationId that would invalidate a compliant common SDK name", async () => { - await tester - .expect( - `${header} +it("ignores an operationId that would invalidate a compliant common SDK name", async () => { + await tester + .expect( + `${header} @TypeSpec.OpenAPI.operationId("Widgets_Modify") @patch @route("/item") op update(): string;`, - ) - .toBeValid(); - }); + ) + .toBeValid(); +}); - it("validates the exact incorrect template fixture", async () => { - await tester - .expect(await patchTemplateFixture("#### ❌ Incorrect")) - .toEmitDiagnostics([diagnostic("modify")]); - }); +it("validates the exact incorrect template fixture", async () => { + await tester + .expect(await patchTemplateFixture("#### ❌ Incorrect")) + .toEmitDiagnostics([diagnostic("modify")]); +}); - it("validates the exact correct template fixture", async () => { - await tester.expect(await patchTemplateFixture("#### ✅ Correct")).toBeValid(); - }); +it("validates the exact correct template fixture", async () => { + await tester.expect(await patchTemplateFixture("#### ✅ Correct")).toBeValid(); +}); - it("validates the exact common client name template fixture", async () => { - await tester.expect(await patchTemplateFixture("## Impact")).toBeValid(); - }); +it("validates the exact common client name template fixture", async () => { + await tester.expect(await patchTemplateFixture("## Impact")).toBeValid(); }); From 50c4b74939e6ad0c849ab5b536ed9d8e245d345f Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Mon, 19 Oct 2026 12:18:23 +0800 Subject: [PATCH 13/17] test(tcgc): simplify DELETE groups and package isolation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...ete-in-operation-name-2026-9-15-10-24-4.md | 7 ------ .../changes/review-delete-sdk-verb-prefix.md | 7 ------ .../test/package-isolation.test.ts | 10 ++++++++ .../test/rules/use-create-for-put.test.ts | 6 +---- .../test/rules/use-delete-for-delete.test.ts | 4 +-- ...te-for-delete-dependency-isolation.test.ts | 25 ------------------- 6 files changed, 13 insertions(+), 46 deletions(-) delete mode 100644 .chronus/changes/promote-lintdiff-delete-in-operation-name-2026-9-15-10-24-4.md delete mode 100644 .chronus/changes/review-delete-sdk-verb-prefix.md create mode 100644 packages/typespec-client-generator-core/test/package-isolation.test.ts delete mode 100644 packages/typespec-client-generator-core/test/use-delete-for-delete-dependency-isolation.test.ts diff --git a/.chronus/changes/promote-lintdiff-delete-in-operation-name-2026-9-15-10-24-4.md b/.chronus/changes/promote-lintdiff-delete-in-operation-name-2026-9-15-10-24-4.md deleted file mode 100644 index 00042cb642..0000000000 --- a/.chronus/changes/promote-lintdiff-delete-in-operation-name-2026-9-15-10-24-4.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -changeKind: internal -packages: - - "@azure-tools/typespec-client-generator-core" ---- - -Reuse unwrapped ARM test setup and retain emitter-isolation coverage in a dedicated integration test. \ No newline at end of file diff --git a/.chronus/changes/review-delete-sdk-verb-prefix.md b/.chronus/changes/review-delete-sdk-verb-prefix.md deleted file mode 100644 index 76b1fafeb8..0000000000 --- a/.chronus/changes/review-delete-sdk-verb-prefix.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -changeKind: internal -packages: - - "@azure-tools/typespec-client-generator-core" ---- - -Share package-private SDK verb-prefix traversal between DELETE and PUT naming rules. diff --git a/packages/typespec-client-generator-core/test/package-isolation.test.ts b/packages/typespec-client-generator-core/test/package-isolation.test.ts new file mode 100644 index 0000000000..3bac8e124c --- /dev/null +++ b/packages/typespec-client-generator-core/test/package-isolation.test.ts @@ -0,0 +1,10 @@ +import { expect, it, vi } from "vitest"; +import * as library from "../src/index.js"; + +vi.mock("@azure-tools/typespec-autorest", () => { + throw new Error("Native lint must not load the AutoRest emitter."); +}); + +it("loads the SDK public entry point without the AutoRest emitter", () => { + expect(library.$lib.name).toBe("@azure-tools/typespec-client-generator-core"); +}); diff --git a/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts b/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts index 4b27ceda2b..a6ac8bcbd9 100644 --- a/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts @@ -1,12 +1,8 @@ import { LinterRuleTester, createLinterRuleTester } from "@typespec/compiler/testing"; -import { beforeEach, describe, it, vi } from "vitest"; +import { beforeEach, describe, it } from "vitest"; import { useCreateForPutRule } from "../../src/rules/use-create-for-put.js"; import { ArmTester } from "../tester.js"; -vi.mock("@azure-tools/typespec-autorest", () => { - throw new Error("Native lint must not load the AutoRest emitter."); -}); - let tester: LinterRuleTester; beforeEach(async () => { diff --git a/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts index 76edecf870..5d2dac046a 100644 --- a/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-delete-for-delete.test.ts @@ -13,7 +13,7 @@ function diagnostic(operationName: string, target = operationName) { }; } -describe("use-delete-for-delete", () => { +describe("HTTP SDK names", () => { let tester: LinterRuleTester; const header = "@service namespace Test;"; @@ -269,7 +269,7 @@ describe("use-delete-for-delete", () => { }); }); -describe("use-delete-for-delete with standard ARM operations", () => { +describe("standard ARM operations", () => { let tester: LinterRuleTester; const resource = ` @armProviderNamespace @service diff --git a/packages/typespec-client-generator-core/test/use-delete-for-delete-dependency-isolation.test.ts b/packages/typespec-client-generator-core/test/use-delete-for-delete-dependency-isolation.test.ts deleted file mode 100644 index 16ea3fa6c7..0000000000 --- a/packages/typespec-client-generator-core/test/use-delete-for-delete-dependency-isolation.test.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { createLinterRuleTester } from "@typespec/compiler/testing"; -import { it, vi } from "vitest"; -import { useDeleteForDeleteRule } from "../src/rules/use-delete-for-delete.js"; -import { SimpleTester } from "./tester.js"; - -vi.mock("@azure-tools/typespec-autorest", () => { - throw new Error("Native lint must not load the AutoRest emitter."); -}); - -it("runs native DELETE naming without loading the AutoRest emitter", async () => { - const tester = createLinterRuleTester( - await SimpleTester.createInstance(), - useDeleteForDeleteRule, - "@azure-tools/typespec-client-generator-core", - ); - await tester - .expect( - ` - @service namespace Example; - @delete @route("/widgets/{id}") - op deleteWidget(@path id: string): void; - `, - ) - .toBeValid(); -}); From a93d6956b5e87c000b99573d08dfa878190176ef Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Tue, 20 Oct 2026 11:30:15 +0800 Subject: [PATCH 14/17] test(tcgc): remove redundant package isolation guard Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/package-isolation.test.ts | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 packages/typespec-client-generator-core/test/package-isolation.test.ts diff --git a/packages/typespec-client-generator-core/test/package-isolation.test.ts b/packages/typespec-client-generator-core/test/package-isolation.test.ts deleted file mode 100644 index 3bac8e124c..0000000000 --- a/packages/typespec-client-generator-core/test/package-isolation.test.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { expect, it, vi } from "vitest"; -import * as library from "../src/index.js"; - -vi.mock("@azure-tools/typespec-autorest", () => { - throw new Error("Native lint must not load the AutoRest emitter."); -}); - -it("loads the SDK public entry point without the AutoRest emitter", () => { - expect(library.$lib.name).toBe("@azure-tools/typespec-client-generator-core"); -}); From 937f5bc66386901c998097f22a10f91b44aa5593 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 21 Oct 2026 11:56:27 +0800 Subject: [PATCH 15/17] fix(tcgc): simplify verb naming checks and service traversal Address selected review findings 37-41 with native endpoint traversal, focused tests, registration coverage, and concise ARM examples. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/rules/use-correct-verb.md | 31 +---- .../src/rules/use-correct-verb.ts | 66 +++++++-- .../test/linter-registration.test.ts | 20 +++ .../test/rules/use-correct-verb.test.ts | 131 +++++++++++------- 4 files changed, 150 insertions(+), 98 deletions(-) create mode 100644 packages/typespec-client-generator-core/test/linter-registration.test.ts diff --git a/packages/typespec-client-generator-core/src/rules/use-correct-verb.md b/packages/typespec-client-generator-core/src/rules/use-correct-verb.md index 36cabac0ae..abf691a418 100644 --- a/packages/typespec-client-generator-core/src/rules/use-correct-verb.md +++ b/packages/typespec-client-generator-core/src/rules/use-correct-verb.md @@ -12,32 +12,7 @@ the common name. Diagnostics target the original operation. The rule is disabled by default in the `client-sdk` ruleset. It does not select an emitter or API-version projection, or reconstruct historical renamed names. -Use this shared setup before each ARM example: - -```typespec -import "@typespec/http"; -import "@typespec/rest"; -import "@azure-tools/typespec-azure-resource-manager"; -import "@azure-tools/typespec-client-generator-core"; - -using TypeSpec.Http; -using TypeSpec.Rest; -using Azure.ResourceManager; -using Azure.ClientGenerator.Core; - -@armProviderNamespace -@service -@armCommonTypesVersion(CommonTypes.Versions.v5) -namespace Example; - -model Widget is TrackedResource { - @key("widgetName") @segment("widgets") name: string; -} - -model WidgetProperties { - provisioningState?: ResourceProvisioningState; -} -``` +These examples assume `Widget` and `WidgetProperties` are defined ARM resource types. #### ❌ Incorrect @@ -72,10 +47,6 @@ the common SDK name without renaming the TypeSpec operation: ```typespec @armResourceOperations interface Widgets { - @clientName("createOrUpdate") - replace is ArmResourceCreateOrReplaceAsync; - @clientName("updateWidget") - modify is ArmResourcePatchSync; @clientName("deleteWidget") remove is ArmResourceDeleteSync; } diff --git a/packages/typespec-client-generator-core/src/rules/use-correct-verb.ts b/packages/typespec-client-generator-core/src/rules/use-correct-verb.ts index 2708156023..99084e5a2d 100644 --- a/packages/typespec-client-generator-core/src/rules/use-correct-verb.ts +++ b/packages/typespec-client-generator-core/src/rules/use-correct-verb.ts @@ -1,5 +1,13 @@ -import { createRule, fileRef, paramMessage } from "@typespec/compiler"; -import { getAllHttpServices } from "@typespec/http"; +import { + createRule, + fileRef, + isTemplateDeclaration, + isTemplateDeclarationOrInstance, + listServices, + paramMessage, + type Namespace, +} from "@typespec/compiler"; +import { getHttpOperation } from "@typespec/http"; import { createTCGCContext } from "../context.js"; import { AllScopes } from "../internal-utils.js"; import { getLibraryName } from "../public-utils.js"; @@ -22,23 +30,51 @@ export const useCorrectVerbRule = createRule({ "@azure-tools/typespec-client-generator-core", { mutateNamespace: false }, ); + const serviceNamespaces = new Set(listServices(context.program).map((service) => service.type)); + const globalNamespace = context.program.getGlobalNamespaceType(); return { - root: () => { - const [services] = getAllHttpServices(context.program); - for (const service of services) { - for (const { operation, verb } of service.operations) { - if (verb !== "put" && verb !== "patch" && verb !== "delete") continue; - const prefix = verbPrefixes[verb]; - const operationName = getLibraryName(tcgcContext, operation, AllScopes); - if (!operationName.toLowerCase().startsWith(prefix)) { - context.reportDiagnostic({ - target: operation, - format: { verb: verb.toUpperCase(), operationName, prefix }, - }); - } + operation: (operation) => { + if (isTemplateDeclarationOrInstance(operation)) return; + const container = operation.interface ?? operation.namespace; + const namespace = operation.interface?.namespace ?? operation.namespace; + if (!container || !namespace || container.operations.get(operation.name) !== operation) { + return; + } + if ( + operation.interface && + (isTemplateDeclaration(operation.interface) || + namespace.interfaces.get(operation.interface.name) !== operation.interface) + ) { + return; + } + + // HTTP's implicit global service is nonrecursive, including only direct interfaces. + let eligible = + namespace === globalNamespace && + (serviceNamespaces.size === 0 || serviceNamespaces.has(globalNamespace)); + for ( + let ancestor: Namespace | undefined = namespace; + ancestor && ancestor !== globalNamespace; + ancestor = ancestor.namespace + ) { + if (serviceNamespaces.has(ancestor)) { + eligible = true; + break; } } + if (!eligible) return; + + const [{ verb }] = getHttpOperation(context.program, operation); + if (verb !== "put" && verb !== "patch" && verb !== "delete") return; + const prefix = verbPrefixes[verb]; + const operationName = getLibraryName(tcgcContext, operation, AllScopes); + if (!operationName.toLowerCase().startsWith(prefix)) { + context.reportDiagnostic({ + target: operation, + format: { verb: verb.toUpperCase(), operationName, prefix }, + }); + } }, }; }, diff --git a/packages/typespec-client-generator-core/test/linter-registration.test.ts b/packages/typespec-client-generator-core/test/linter-registration.test.ts new file mode 100644 index 0000000000..42f794abb1 --- /dev/null +++ b/packages/typespec-client-generator-core/test/linter-registration.test.ts @@ -0,0 +1,20 @@ +import { expect, it } from "vitest"; +import { $linter } from "../src/index.js"; + +it("registers only the combined verb rule without enabling it for C#", () => { + const names = $linter.rules.map((rule) => rule.name); + expect(names.filter((name) => name === "use-correct-verb")).toHaveLength(1); + for (const oldName of ["use-create-for-put", "use-update-for-patch", "use-delete-for-delete"]) { + expect(names).not.toContain(oldName); + expect( + $linter.ruleSets?.["best-practices:csharp"].enable?.[ + `@azure-tools/typespec-client-generator-core/${oldName}` + ], + ).toBeUndefined(); + } + expect( + $linter.ruleSets?.["best-practices:csharp"].enable?.[ + "@azure-tools/typespec-client-generator-core/use-correct-verb" + ], + ).toBeUndefined(); +}); diff --git a/packages/typespec-client-generator-core/test/rules/use-correct-verb.test.ts b/packages/typespec-client-generator-core/test/rules/use-correct-verb.test.ts index 723115ef64..615451540a 100644 --- a/packages/typespec-client-generator-core/test/rules/use-correct-verb.test.ts +++ b/packages/typespec-client-generator-core/test/rules/use-correct-verb.test.ts @@ -1,9 +1,7 @@ import { createLinterRuleTester, type LinterRuleTester } from "@typespec/compiler/testing"; -import { readFile } from "node:fs/promises"; -import { beforeEach, describe, expect, it } from "vitest"; -import { $linter } from "../../src/linter.js"; +import { beforeEach, describe, it } from "vitest"; import { useCorrectVerbRule } from "../../src/rules/use-correct-verb.js"; -import { ArmBaseTester, ArmTester, SimpleTester } from "../tester.js"; +import { ArmTester, SimpleTester } from "../tester.js"; const header = "@service namespace Test;"; const cases = [ @@ -36,7 +34,8 @@ function diagnostic(verb: string, prefix: string, operationName: string, target }; } -describe.each(cases)("$verb SDK names", ({ verb, prefix, invalid }) => { +describe("HTTP SDK names", () => { + const { verb, prefix, invalid } = cases[2]; let tester: LinterRuleTester; beforeEach(async () => { const runner = await SimpleTester.import("@typespec/openapi").createInstance(); @@ -49,6 +48,14 @@ describe.each(cases)("$verb SDK names", ({ verb, prefix, invalid }) => { const endpoint = (name: string) => `@${verb} @route("/widgets") op ${name}(): void;`; const finding = (name: string, target = name) => diagnostic(verb, prefix, name, target); + it.each(cases)("maps $verb to $prefix", async ({ verb, prefix, invalid }) => { + await tester + .expect(`${header} @${verb} @route("/widgets") op ${prefix}Widget(): void;`) + .toBeValid(); + await tester + .expect(`${header} @${verb} @route("/widgets") op ${invalid}(): void;`) + .toEmitDiagnostics([diagnostic(verb, prefix, invalid)]); + }); it.each(["", "Widget", "OrUpdate"])("accepts the prefix with suffix %s", async (suffix) => { await tester.expect(`${header} ${endpoint(prefix + suffix)}`).toBeValid(); }); @@ -128,13 +135,6 @@ describe.each(cases)("$verb SDK names", ({ verb, prefix, invalid }) => { ) .toEmitDiagnostics([finding(`${invalid}_${prefix}`)]); }); - it.each([200, 201, 202, 204])("checks names independently of response code %s", async (code) => { - await tester - .expect( - `${header} @${verb} @route("/widgets") op ${invalid}(): { @statusCode code: ${code} };`, - ) - .toEmitDiagnostics([finding(invalid)]); - }); it("reports each concrete endpoint once", async () => { await tester .expect( @@ -183,17 +183,72 @@ describe("combined endpoint selection", () => { cases.map(({ verb, prefix, invalid }) => diagnostic(verb, prefix, invalid)), ); }); - it("registers only the combined rule, without enabling it for C#", () => { - const names = $linter.rules.map((rule) => rule.name); - expect(names).toContain("use-correct-verb"); - for (const oldName of ["use-create-for-put", "use-update-for-patch", "use-delete-for-delete"]) { - expect(names).not.toContain(oldName); - } - expect( - $linter.ruleSets?.["best-practices:csharp"].enable?.[ - "@azure-tools/typespec-client-generator-core/use-correct-verb" - ], - ).toBeUndefined(); + it("reports a nested service endpoint exactly once", async () => { + await tester + .expect( + ` + @service namespace Outer { + @service namespace Inner { + @delete @route("/widgets") op remove(): void; + } + } + `, + ) + .toEmitDiagnostics([diagnostic("delete", "delete", "remove")]); + }); + it("includes child namespaces and interfaces of an explicit service only", async () => { + await tester + .expect( + ` + @delete @route("/global") op globalHelper(): void; + @service namespace Service { + namespace Child { + @delete @route("/child") op removeChild(): void; + interface Widgets { @delete @route("/interface") removeWidget(): void; } + } + } + namespace Other { @delete @route("/other") op helper(): void; } + `, + ) + .toEmitDiagnostics([ + diagnostic("delete", "delete", "removeChild"), + diagnostic("delete", "delete", "removeWidget"), + ]); + }); + it("uses only direct global operations and interfaces when no service is declared", async () => { + await tester + .expect( + ` + @delete @route("/global") op removeGlobal(): void; + interface Widgets { @delete @route("/interface") removeWidget(): void; } + namespace Other { + @delete @route("/other") op helper(): void; + interface Helpers { @delete @route("/helper") removeHelper(): void; } + } + `, + ) + .toEmitDiagnostics([ + diagnostic("delete", "delete", "removeGlobal"), + diagnostic("delete", "delete", "removeWidget"), + ]); + }); + it("checks concrete aliases but not template declarations or instances", async () => { + await tester + .expect( + ` + ${header} + @delete @route("/template") op removeTemplate(): T; + op removeAlias is removeTemplate; + interface Templates { + @delete @route("/interface") removeInterface(): T; + } + interface Widgets extends Templates {} + `, + ) + .toEmitDiagnostics([ + diagnostic("delete", "delete", "removeAlias"), + diagnostic("delete", "delete", "removeInterface"), + ]); }); }); @@ -239,33 +294,3 @@ describe("standard ARM operations", () => { }, ); }); - -describe("published ARM examples", () => { - it("compiles the incorrect, correct, and common-name examples", async () => { - const docs = await readFile( - new URL("../../src/rules/use-correct-verb.md", import.meta.url), - "utf8", - ); - const snippets = [...docs.matchAll(/```typespec\r?\n([\s\S]*?)```/g)].map((match) => match[1]); - expect(snippets).toHaveLength(5); - const tester = createLinterRuleTester( - await ArmBaseTester.createInstance(), - useCorrectVerbRule, - "@azure-tools/typespec-client-generator-core", - ); - await tester - .expect(`${snippets[0]}\n${snippets[1]}`) - .toEmitDiagnostics( - cases.map(({ verb, prefix, invalid }) => diagnostic(verb, prefix, invalid)), - ); - await tester.expect(`${snippets[0]}\n${snippets[2]}`).toBeValid(); - await tester.expect(`${snippets[0]}\n${snippets[3]}`).toBeValid(); - const runner = await ArmBaseTester.createInstance(); - const [, diagnostics] = await runner.compileAndDiagnose(`${snippets[0]}\n${snippets[4]}`, { - linterRuleSet: { - enable: { "@azure-tools/typespec-client-generator-core/use-correct-verb": true }, - }, - }); - expect(diagnostics).toEqual([]); - }); -}); From aaf8b5ca082a9dd946e4d7db8d0b3b53281555ea Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 21 Oct 2026 17:45:45 +0800 Subject: [PATCH 16/17] Update packages/typespec-client-generator-core/src/rules/use-correct-verb.md Co-authored-by: Mark Cowlishaw <1054056+markcowl@users.noreply.github.com> --- .../src/rules/use-correct-verb.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/typespec-client-generator-core/src/rules/use-correct-verb.md b/packages/typespec-client-generator-core/src/rules/use-correct-verb.md index abf691a418..63b27ef5fd 100644 --- a/packages/typespec-client-generator-core/src/rules/use-correct-verb.md +++ b/packages/typespec-client-generator-core/src/rules/use-correct-verb.md @@ -1,4 +1,4 @@ -Use common SDK method names beginning with `create` for PUT, `update` for PATCH, +Use common SDK method names for resource lifecycle operations. Operation names should begin with `create` for PUT, `update` for PATCH, and `delete` for DELETE. Prefix matching is case-insensitive. GET, POST, HEAD, and other HTTP verbs are outside this rule. From 9a8f4f968c27beaf5d821558f779686994b5b665 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 21 Oct 2026 17:54:11 +0800 Subject: [PATCH 17/17] docs(tcgc): focus verb naming guidance on spec authors Simplify implementation notes, retain opt-in and released-rule migration guidance, and keep migration provenance in the PR description. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/rules/use-correct-verb.md | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/packages/typespec-client-generator-core/src/rules/use-correct-verb.md b/packages/typespec-client-generator-core/src/rules/use-correct-verb.md index 63b27ef5fd..3c38540c43 100644 --- a/packages/typespec-client-generator-core/src/rules/use-correct-verb.md +++ b/packages/typespec-client-generator-core/src/rules/use-correct-verb.md @@ -1,16 +1,13 @@ -Use common SDK method names for resource lifecycle operations. Operation names should begin with `create` for PUT, `update` for PATCH, +Use common SDK method names for resource lifecycle operations. Operation names should begin with `create` for PUT, `update` for PATCH, and `delete` for DELETE. Prefix matching is case-insensitive. GET, POST, HEAD, and other HTTP verbs are outside this rule. -The rule checks concrete HTTP service endpoints in ARM and data-plane services. -It does not require ARM provider metadata or check client-only declarations -outside services. It resolves common names through TCGC, honoring unscoped -`@clientName`, exact names, and the supported `@friendlyName` fallback. -Language-specific overrides and explicit OpenAPI operation IDs do not change -the common name. Diagnostics target the original operation. +This guidance applies to both ARM and data-plane services. To fix a warning, +rename the operation or use `@clientName` without a language scope, as shown below. +Changing only a language-specific name or an OpenAPI operation ID does not fix +the common SDK name. -The rule is disabled by default in the `client-sdk` ruleset. It does not select -an emitter or API-version projection, or reconstruct historical renamed names. +The rule is disabled by default in the `client-sdk` ruleset. These examples assume `Widget` and `WidgetProperties` are defined ARM resource types. @@ -68,10 +65,9 @@ interface Widgets { ## Migration -This rule replaces `use-create-for-put` and consolidates the proposed -`use-update-for-patch` and `use-delete-for-delete` rules. Replace the old name -in linter configuration and suppressions with `use-correct-verb`. -Enabling the replacement checks all three verbs, not just PUT. +If you use `use-create-for-put`, replace it with `use-correct-verb` in linter +configuration and suppressions. Enabling the replacement checks PATCH and DELETE +names as well as PUT names. ## LintDiff Equivalent @@ -80,7 +76,3 @@ Enabling the replacement checks all three verbs, not just PUT. | [PutInOperationName](https://github.com/Azure/azure-openapi-validator/blob/main/docs/put-in-operation-name.md) | PUT names start with `create`. | | [PatchInOperationName](https://github.com/Azure/azure-openapi-validator/blob/main/docs/patch-in-operation-name.md) | PATCH names start with `update`. | | [DeleteInOperationName](https://github.com/Azure/azure-openapi-validator/blob/main/docs/delete-in-operation-name.md) | DELETE names start with `delete`. | - -The native rule checks common SDK names rather than serialized `operationId` -or lintdiff authored names. It intentionally does not reproduce underscore, -group-prefix, or missing-ID exemptions. No emitted Swagger parity is claimed.