diff --git a/.chronus/changes/register-use-correct-verb.md b/.chronus/changes/register-use-correct-verb.md new file mode 100644 index 0000000000..f8e2936ce5 --- /dev/null +++ b/.chronus/changes/register-use-correct-verb.md @@ -0,0 +1,7 @@ +--- +changeKind: internal +packages: + - "@azure-tools/typespec-azure-rulesets" +--- + +Replace the separate SDK verb naming registrations with `@azure-tools/typespec-client-generator-core/use-correct-verb`, disabled by default in the `client-sdk` ruleset. diff --git a/.chronus/changes/use-correct-verb.md b/.chronus/changes/use-correct-verb.md new file mode 100644 index 0000000000..c51966afdf --- /dev/null +++ b/.chronus/changes/use-correct-verb.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +Consolidate `use-create-for-put`, PATCH naming guidance, and DELETE naming guidance into the opt-in `use-correct-verb` lint rule. Common SDK method names must start with `create` for PUT, `update` for PATCH, and `delete` for DELETE, case-insensitively. Replace `use-create-for-put` in linter configuration and suppressions with `use-correct-verb`; enabling it also checks PATCH and DELETE. Renaming an already-shipped SDK method may be a breaking change. diff --git a/eng/scripts/doc-updater/knowledge/tcgc.md b/eng/scripts/doc-updater/knowledge/tcgc.md index 15979e7fec..5609639240 100644 --- a/eng/scripts/doc-updater/knowledge/tcgc.md +++ b/eng/scripts/doc-updater/knowledge/tcgc.md @@ -219,7 +219,7 @@ namespace (@clientNamespace), naming (@clientName), overload, structure (@client ## SDK Method Naming Rules - `get-operation-name` checks the common TCGC SDK name of concrete GET operations and requires a `get` or `list` prefix. It honors unscoped `@clientName`, ignores emitter-scoped overrides and OpenAPI operation IDs, and skips template declarations/artifacts and non-GET operations. -- `use-create-for-put` checks concrete PUT endpoints and requires the common TCGC SDK name to start with `create`, case-insensitively. It uses the same common-name resolution, applies without requiring ARM provider metadata, and is disabled by default in the `client-sdk` ruleset. +- `use-correct-verb` checks concrete PUT, PATCH, and DELETE endpoints and requires the common TCGC SDK name to start with `create`, `update`, and `delete`, respectively, case-insensitively. It replaces `use-create-for-put`, uses the same common-name resolution, applies without requiring ARM provider metadata, and is disabled by default in the `client-sdk` ruleset. - Linter-only naming rules do not alter the generated client graph or wire behavior, so their unit tests and generated rule reference pages are the appropriate coverage; do not add Spector carrier scenarios for them. ## isExactName Property (May 2026) diff --git a/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts b/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts index 2cfd0b40bd..fc11263834 100644 --- a/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts +++ b/packages/typespec-azure-rulesets/src/rulesets/client-sdk.ts @@ -8,10 +8,9 @@ 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-correct-verb": false, }, disable: { - "@azure-tools/typespec-client-generator-core/use-create-for-put": - "Opt-in ARM naming guidance; data-plane PUT methods may use other verbs.", "@azure-tools/typespec-client-generator-core/csharp-use-standard-acronyms": "Disabled until the C# emitter handles standard acronym casing.", }, 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 81346f615e..2077157f26 100644 --- a/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts +++ b/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts @@ -60,16 +60,33 @@ 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-correct-verb"], + false, + ); ok( ruleset.disable?.["@azure-tools/typespec-client-generator-core/csharp-use-standard-acronyms"], ); }); - it("keeps PUT SDK naming guidance opt-in", () => { - const ruleName = "@azure-tools/typespec-client-generator-core/use-create-for-put"; - ok($linter.ruleSets?.["client-sdk"].disable?.[ruleName]); - for (const rulesetName of ["data-plane", "resource-manager", "client-sdk"]) { - ok(!$linter.ruleSets?.[rulesetName].enable?.[ruleName]); + it("keeps combined SDK naming guidance opt-in", () => { + const ruleName = "@azure-tools/typespec-client-generator-core/use-correct-verb"; + 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].disable?.[ruleName], undefined); + } + for (const oldName of ["use-create-for-put", "use-update-for-patch", "use-delete-for-delete"]) { + for (const ruleset of Object.values($linter.ruleSets ?? {})) { + strictEqual( + ruleset.enable?.[`@azure-tools/typespec-client-generator-core/${oldName}`], + undefined, + ); + strictEqual( + ruleset.disable?.[`@azure-tools/typespec-client-generator-core/${oldName}`], + undefined, + ); + } } }); diff --git a/packages/typespec-client-generator-core/README.md b/packages/typespec-client-generator-core/README.md index f4e5917f11..bb741fbb78 100644 --- a/packages/typespec-client-generator-core/README.md +++ b/packages/typespec-client-generator-core/README.md @@ -120,7 +120,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-correct-verb`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-correct-verb) | PUT, PATCH, and DELETE SDK method names should start with 'create', 'update', and 'delete', respectively. | | [`@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..a7fe35b5c0 100644 --- a/packages/typespec-client-generator-core/src/linter.ts +++ b/packages/typespec-client-generator-core/src/linter.ts @@ -5,11 +5,11 @@ import { csharpUseStandardAcronymsRule } from "./rules/csharp-use-standard-acron 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 { useCorrectVerbRule } from "./rules/use-correct-verb.js"; import { useUnionHierarchyRule } from "./rules/use-union-hierarchy.js"; const rules = [ - useCreateForPutRule, + useCorrectVerbRule, requireClientSuffixRule, propertyNameConflictRule, csharpNoUrlSuffixRule, 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 new file mode 100644 index 0000000000..3c38540c43 --- /dev/null +++ b/packages/typespec-client-generator-core/src/rules/use-correct-verb.md @@ -0,0 +1,78 @@ +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. + +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. + +These examples assume `Widget` and `WidgetProperties` are defined ARM resource types. + +#### ❌ Incorrect + +```typespec +@armResourceOperations +interface Widgets { + replace is ArmResourceCreateOrReplaceAsync; + modify is ArmResourcePatchSync; + remove is ArmResourceDeleteSync; +} +``` + +#### ✅ Correct + +```typespec +@armResourceOperations +interface Widgets { + createOrUpdate is ArmResourceCreateOrReplaceAsync; + update is ArmResourcePatchSync; + delete is ArmResourceDeleteSync; +} +``` + +## Impact + +- **Area:** SDK + +Renaming an already-shipped SDK method may introduce a breaking change. +Review compatibility before changing names. An unscoped `@clientName` changes +the common SDK name without renaming the TypeSpec operation: + +```typespec +@armResourceOperations +interface Widgets { + @clientName("deleteWidget") + remove is ArmResourceDeleteSync; +} +``` + +## Suppression + +For new methods, fix the name or use an unscoped `@clientName`. When preserving +an already-shipped name requires an API-reviewer-approved exception, suppress +the warning on that operation with a justification: + +```typespec +@armResourceOperations +interface Widgets { + #suppress "@azure-tools/typespec-client-generator-core/use-correct-verb" "Preserve the already-shipped SDK method name." + remove is ArmResourceDeleteSync; +} +``` + +## Migration + +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 + +| Swagger validator | Native SDK naming requirement | +| -------------------------------------------------------------------------------------------------------------------- | --------------------------------- | +| [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`. | 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 new file mode 100644 index 0000000000..99084e5a2d --- /dev/null +++ b/packages/typespec-client-generator-core/src/rules/use-correct-verb.ts @@ -0,0 +1,81 @@ +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"; + +const verbPrefixes = { put: "create", patch: "update", delete: "delete" }; + +export const useCorrectVerbRule = createRule({ + name: "use-correct-verb", + description: + "PUT, PATCH, and DELETE SDK method names should start with 'create', 'update', and 'delete', respectively.", + severity: "warning", + url: "https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-correct-verb", + docs: fileRef.fromPackageRoot("src/rules/use-correct-verb.md"), + messages: { + default: paramMessage`${"verb"} SDK method name '${"operationName"}' 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.`, + }, + create(context) { + const tcgcContext = createTCGCContext( + context.program, + "@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 { + 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/src/rules/use-create-for-put.md b/packages/typespec-client-generator-core/src/rules/use-create-for-put.md deleted file mode 100644 index dc4aef6682..0000000000 --- a/packages/typespec-client-generator-core/src/rules/use-create-for-put.md +++ /dev/null @@ -1,82 +0,0 @@ -Use an SDK method name beginning with `create` for ARM PUT operations, such as `create`, -`createOrUpdate`, or `createOrReplace`. This applies even when the operation -uses a standard ARM create template or only returns a success response for -updating an existing resource. - -The rule checks the common TCGC SDK method name case-insensitively on concrete HTTP -endpoints, including aliases, inherited interface operations, and operations in -nested namespaces. Generic declarations and their internal instantiations are -not separate endpoints. When enabled, the rule 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 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 { - set is ArmResourceCreateOrReplaceAsync; -} -``` - -#### ✅ Correct - -```typespec -@armResourceOperations -interface Widgets { - createOrUpdate is ArmResourceCreateOrReplaceAsync; -} -``` - -## Impact - -- **Area:** SDK - -Consistent create-method naming makes resource creation and replacement easier -to discover in generated clients. Renaming an operation after an SDK has shipped -may introduce a breaking change to its public methods. 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("createOrUpdate") - set is ArmResourceCreateOrReplaceAsync; -} -``` - -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-create-for-put" "Preserve the already-shipped SDK method name." - set is ArmResourceCreateOrReplaceAsync; -} -``` - -## LintDiff Equivalent - -This rule enforces the ARM SDK method-naming intent of -[`PutInOperationName`](https://github.com/Azure/azure-openapi-validator/blob/a970d991d2785184d2786b85e0a345dc3f37bc25/docs/put-in-operation-name.md) -([automated guideline R1006](https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/openapi-authoring-automated-guidelines.md#r1006)). -It does not reproduce Swagger `operationId` underscore or group-name exemptions. -Explicit OpenAPI IDs do not change the SDK method name checked by this rule; -unscoped SDK name overrides do. No emitter or version projection is used. 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 deleted file mode 100644 index 979aba880a..0000000000 --- a/packages/typespec-client-generator-core/src/rules/use-create-for-put.ts +++ /dev/null @@ -1,47 +0,0 @@ -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 useCreateForPutRule = createRule({ - name: "use-create-for-put", - description: "ARM PUT SDK method names should use 'create' as the verb prefix.", - severity: "warning", - url: "https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-create-for-put", - docs: fileRef.fromPackageRoot("src/rules/use-create-for-put.md"), - messages: { - default: paramMessage`PUT SDK method name '${"operationName"}' should start with 'create'. 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 !== "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 }, - }); - } - } - }, - }; - }, -}); 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 new file mode 100644 index 0000000000..615451540a --- /dev/null +++ b/packages/typespec-client-generator-core/test/rules/use-correct-verb.test.ts @@ -0,0 +1,296 @@ +import { createLinterRuleTester, type LinterRuleTester } from "@typespec/compiler/testing"; +import { beforeEach, describe, it } from "vitest"; +import { useCorrectVerbRule } from "../../src/rules/use-correct-verb.js"; +import { ArmTester, SimpleTester } from "../tester.js"; + +const header = "@service namespace Test;"; +const cases = [ + { + verb: "put", + prefix: "create", + invalid: "replace", + template: "ArmResourceCreateOrReplaceAsync", + }, + { + verb: "patch", + prefix: "update", + invalid: "modify", + template: "ArmResourcePatchSync", + }, + { + verb: "delete", + prefix: "delete", + invalid: "remove", + template: "ArmResourceDeleteSync", + }, +] as const; + +function diagnostic(verb: string, prefix: string, operationName: string, target = operationName) { + return { + code: "@azure-tools/typespec-client-generator-core/use-correct-verb", + severity: "warning" as const, + message: `${verb.toUpperCase()} SDK method name '${operationName}' 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.`, + target, + }; +} + +describe("HTTP SDK names", () => { + const { verb, prefix, invalid } = cases[2]; + let tester: LinterRuleTester; + beforeEach(async () => { + const runner = await SimpleTester.import("@typespec/openapi").createInstance(); + tester = createLinterRuleTester( + runner, + useCorrectVerbRule, + "@azure-tools/typespec-client-generator-core", + ); + }); + 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(); + }); + it("accepts prefixes case-insensitively", async () => { + await tester.expect(`${header} ${endpoint(prefix.toUpperCase() + "Widget")}`).toBeValid(); + }); + it.each(["invalid", "prefix-inside", "truncated"])("rejects a %s name", async (kind) => { + const name = + kind === "invalid" + ? invalid + : kind === "prefix-inside" + ? `soft${prefix}` + : prefix.slice(0, -1); + await tester.expect(`${header} ${endpoint(name)}`).toEmitDiagnostics([finding(name)]); + }); + it("honors an unscoped common name", async () => { + await tester + .expect(`${header} @clientName("${prefix}Widget") ${endpoint(invalid)}`) + .toBeValid(); + }); + it("targets the authored operation for invalid common names", async () => { + await tester + .expect(`${header} @clientName("${invalid}") ${endpoint(prefix)}`) + .toEmitDiagnostics([finding(invalid, prefix)]); + }); + it("ignores a scoped override of a compliant common name", async () => { + await tester + .expect(`${header} @clientName("${invalid}", "csharp") ${endpoint(prefix)}`) + .toBeValid(); + }); + it("does not let a scoped override mask an invalid common name", async () => { + await tester + .expect(`${header} @clientName("${prefix}", "python") ${endpoint(invalid)}`) + .toEmitDiagnostics([finding(invalid)]); + }); + it("uses common names when scoped overrides coexist", async () => { + await tester + .expect( + `${header} @clientName("${prefix}") @clientName("${invalid}", "csharp") ${endpoint(invalid)}`, + ) + .toBeValid(); + await tester + .expect( + `${header} @clientName("${invalid}") @clientName("${prefix}", "python") ${endpoint(prefix)}`, + ) + .toEmitDiagnostics([finding(invalid, prefix)]); + }); + it("normalizes exact common names", async () => { + await tester + .expect(`${header} @clientName(exact("${prefix.toUpperCase()}Widget")) ${endpoint(invalid)}`) + .toBeValid(); + await tester + .expect(`${header} @clientName(exact("${invalid}Widget")) ${endpoint(prefix)}`) + .toEmitDiagnostics([finding(invalid + "Widget", prefix)]); + }); + it("uses friendly names as a fallback, not instead of common names", async () => { + await tester.expect(`${header} @friendlyName("${prefix}") ${endpoint(invalid)}`).toBeValid(); + await tester + .expect(`${header} @friendlyName("${invalid}") ${endpoint(prefix)}`) + .toEmitDiagnostics([finding(invalid, prefix)]); + await tester + .expect(`${header} @friendlyName("${invalid}") @clientName("${prefix}") ${endpoint(invalid)}`) + .toBeValid(); + }); + it("ignores OpenAPI operation IDs in both directions", async () => { + await tester + .expect(`${header} @TypeSpec.OpenAPI.operationId("Widgets_${prefix}") ${endpoint(invalid)}`) + .toEmitDiagnostics([finding(invalid)]); + await tester + .expect(`${header} @TypeSpec.OpenAPI.operationId("Widgets_${invalid}") ${endpoint(prefix)}`) + .toBeValid(); + }); + it("does not let group names or underscores mask an invalid method", async () => { + await tester + .expect( + `${header} interface ${prefix}Widgets { @${verb} @route("/widgets") ${invalid}_${prefix}(): void; }`, + ) + .toEmitDiagnostics([finding(`${invalid}_${prefix}`)]); + }); + it("reports each concrete endpoint once", async () => { + await tester + .expect( + `${header} + @${verb} @route("/one") op ${invalid}One(): void; + @${verb} @route("/two") op ${invalid}Two(): void; + @${verb} @route("/valid") op ${prefix}(): void; + `, + ) + .toEmitDiagnostics([finding(invalid + "One"), finding(invalid + "Two")]); + }); + it("does not check client-only declarations outside the service", async () => { + await tester + .expect( + ` + @service namespace Test { ${endpoint(invalid)} } + namespace Customizations { @${verb} @route("/other") op helper(): void; } + `, + ) + .toEmitDiagnostics([finding(invalid)]); + }); +}); + +describe("combined endpoint selection", () => { + let tester: LinterRuleTester; + beforeEach(async () => { + tester = createLinterRuleTester( + await SimpleTester.createInstance(), + useCorrectVerbRule, + "@azure-tools/typespec-client-generator-core", + ); + }); + it("reports PUT, PATCH, and DELETE together without duplicate findings", async () => { + await tester + .expect( + `${header} + @put @route("/put") op replace(): void; + @patch @route("/patch") op modify(): void; + @delete @route("/delete") op remove(): void; + @get @route("/get") op read(): void; + @post @route("/post") op submit(): void; + @head @route("/head") op inspect(): void; + `, + ) + .toEmitDiagnostics( + cases.map(({ verb, prefix, invalid }) => diagnostic(verb, prefix, invalid)), + ); + }); + 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"), + ]); + }); +}); + +describe("standard ARM operations", () => { + let tester: LinterRuleTester; + const resource = ` + @armProviderNamespace @service @armCommonTypesVersion(CommonTypes.Versions.v5) namespace Arm; + model Widget is TrackedResource { + @key("widgetName") @segment("widgets") name: string; + } + model WidgetProperties { provisioningState?: ResourceProvisioningState; } + `; + beforeEach(async () => { + tester = createLinterRuleTester( + await ArmTester.createInstance(), + useCorrectVerbRule, + "@azure-tools/typespec-client-generator-core", + ); + }); + it.each(cases)( + "checks $verb aliases and common overrides", + async ({ verb, prefix, invalid, template }) => { + await tester + .expect( + `${resource} @armResourceOperations interface Widgets { ${invalid} is ${template}; }`, + ) + .toEmitDiagnostics([diagnostic(verb, prefix, invalid)]); + await tester + .expect( + `${resource} @armResourceOperations interface Widgets { ${prefix} is ${template}; }`, + ) + .toBeValid(); + await tester + .expect( + `${resource} @armResourceOperations interface Widgets { @clientName("${prefix}Widget") ${invalid} is ${template}; }`, + ) + .toBeValid(); + await tester + .expect( + `${resource} @armResourceOperations interface Widgets { @clientName("${invalid}") ${prefix} is ${template}; }`, + ) + .toEmitDiagnostics([diagnostic(verb, prefix, invalid, prefix)]); + }, + ); +}); 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 deleted file mode 100644 index 4b27ceda2b..0000000000 --- a/packages/typespec-client-generator-core/test/rules/use-create-for-put.test.ts +++ /dev/null @@ -1,310 +0,0 @@ -import { LinterRuleTester, createLinterRuleTester } from "@typespec/compiler/testing"; -import { beforeEach, describe, it, vi } 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 () => { - const runner = await ArmTester.import("@typespec/openapi").createInstance(); - tester = createLinterRuleTester( - runner, - useCreateForPutRule, - "@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 Properties { provisioningState?: ResourceProvisioningState; } -`; - -function diagnostic(operationName: string, target = operationName) { - return { - code: "@azure-tools/typespec-client-generator-core/use-create-for-put", - severity: "warning" as const, - message: `PUT SDK method name '${operationName}' should start with 'create'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.`, - target, - }; -} - -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("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("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} - @clientName("createWidget") - @clientName("setWidget", "csharp") - @put @route("/item") op set(): string;`, - ) - .toBeValid(); - }); - - it("honors client names on standard ARM template aliases", async () => { - await tester - .expect( - `${header} - ${widget} - @armResourceOperations interface Widgets { - @clientName("createOrUpdate") - set is ArmResourceCreateOrReplaceAsync; - }`, - ) - .toBeValid(); - }); - - it("normalizes exact client names", async () => { - await tester - .expect( - `${header} - @clientName(exact("CreateWidget")) - @put @route("/item") op set(): string;`, - ) - .toBeValid(); - }); - - 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(); - }, - ); - - 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} @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(); - }); - - 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")]); - }); - - 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")]); - }); - - 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")]); - }); - - 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") @put op set(): string; - @renamedFrom(Versions.v2, "replace") @route("/renamed") @put op create(): string; - `, - ) - .toEmitDiagnostics([diagnostic("set")]); - }); - - it("checks ordinary service namespaces without provider metadata", async () => { - await tester - .expect( - ` - @service namespace Contoso { - @put @route("/item") op set(): string; - } - `, - ) - .toEmitDiagnostics([diagnostic("set")]); - }); - - it("checks nested service namespaces without provider metadata", async () => { - await tester - .expect( - ` - namespace Contoso { - @service namespace Management { - namespace Nested { - @route("/item") interface Items { @put op replace(): string; } - } - } - } - `, - ) - .toEmitDiagnostics([diagnostic("replace")]); - }); - - 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; } - namespace Azure.ResourceManager { @put op SetTemplate(@body body: T): T; } - @service namespace Contoso { - @put @route("/item") op create(): string; - } - `, - ) - .toBeValid(); - }); - - it("reports the authored setWidgetConfig name despite an explicit operationId", async () => { - await tester - .expect( - ` - ${header} - @TypeSpec.OpenAPI.operationId("Widgets_Create") - @route("/config") @put - op setWidgetConfig(@body body: { config: string }): { - @statusCode statusCode: 200; - @body result: { config: string }; - }; - `, - ) - .toEmitDiagnostics([diagnostic("setWidgetConfig")]); - }); - - 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")]); - }); - - it("allows a standard ARM create template aliased as createOrUpdate", async () => { - await tester - .expect( - `${header} - ${widget} - @armResourceOperations interface Widgets { - createOrUpdate is ArmResourceCreateOrReplaceAsync; - } - `, - ) - .toBeValid(); - }); - - 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(); - }); -}); 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/website/src/content/docs/docs/howtos/ARM/arm-rules.md b/website/src/content/docs/docs/howtos/ARM/arm-rules.md index 4e4f5eb0cc..26824ee352 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 | [`unsupported-type`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/unsupported-type/) | — | **SDK.** The data type cannot be modeled in SDKs. | | [`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-correct-verb`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/use-correct-verb/) | [PutInOperationName (R1006)](https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/openapi-authoring-automated-guidelines.md#r1006), [PatchInOperationName](https://github.com/Azure/azure-openapi-validator/blob/main/docs/patch-in-operation-name.md), [DeleteInOperationName](https://github.com/Azure/azure-openapi-validator/blob/main/docs/delete-in-operation-name.md) | **SDK.** Common SDK names must start with `create` for PUT, `update` for PATCH, and `delete` for DELETE. Honors unscoped `@clientName`; renaming a shipped method may break SDK compatibility. | | [`use-resource-path-for-put`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/use-resource-path-for-put/) | [EvenSegmentedPathForPutOperation](https://github.com/Azure/azure-openapi-validator/blob/main/docs/even-segmented-path-for-put-operation.md) | **API, SDK.** PUT paths must identify a resource instance using resource type/name pairs. Disabled by default in the resource-manager ruleset. | | [`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. | 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..b48dba85dc 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 @@ -23,7 +23,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-correct-verb`](../rules/use-correct-verb.md) | PUT, PATCH, and DELETE SDK method names should start with 'create', 'update', and 'delete', respectively. | | [`@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. |