Repository navigation
Require template-first lint documentation examples - #5678
Merged
Yuchao Yan (msyyc) merged 1 commit intoOct 13, 2026
Merged
Yuchao Yan (msyyc) merged 1 commit into
Yuchao Yan (msyyc) merged 1 commit into
Conversation
Enforce supported incorrect/correct template pairs in the queue, promotion, implementation, and validation skills. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 832854b7-de67-4f1f-8fe1-6a3e9897aeee
Yuchao Yan (msyyc)
marked this pull request as ready for review
October 13, 2026 03:15
Yuchao Yan (msyyc)
requested review from
Mark Cowlishaw (markcowl),
Timothee Guerin (timotheeguerin) and
Jeff Fisher (xirzec)
as code owners
October 13, 2026 03:15
Yuchao Yan (msyyc)
merged commit Oct 13, 2026
2dbe0c0
into
feature/lintdiff-migration-new
3 checks passed
5 tasks done
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Oct 22, 2026
…+ DeleteInOperationName -> use-correct-verb (#5673) ## Summary Consolidate **PutInOperationName**, **PatchInOperationName**, and **DeleteInOperationName** into **`@azure-tools/typespec-client-generator-core/use-correct-verb`**, as agreed in [the review discussion](#5673 (comment)). This PR absorbs the PATCH promotion #5665 and replaces the already-merged PUT rule from #5457. Both open promotion histories are preserved by consolidation merge commit [`d12175650`](d12175650). Following the combined-migration presentation in #5663, #5556, #5294, and #5423, further review of these three SDK naming checks belongs here. Superseding #5665 does not imply CI or review approval; its branch and review history remain intact. ## Combined behavior and legacy-rule mapping | Legacy Swagger validator | Previous native rule/promotion | Combined requirement | | --- | --- | --- | | [PutInOperationName](https://github.com/Azure/azure-openapi-validator/blob/main/docs/put-in-operation-name.md) | `use-create-for-put`, merged #5457 | PUT common SDK names start with `create`. | | [PatchInOperationName](https://github.com/Azure/azure-openapi-validator/blob/main/docs/patch-in-operation-name.md) | Proposed `use-update-for-patch`, #5665 | PATCH common SDK names start with `update`. | | [DeleteInOperationName](https://github.com/Azure/azure-openapi-validator/blob/main/docs/delete-in-operation-name.md) | Proposed `use-delete-for-delete`, this PR | DELETE common SDK names start with `delete`. | - A native operation listener resolves HTTP metadata once per eligible concrete endpoint, using one common SDK-name resolver. Nested services produce one warning per offending operation. Explicit-service ancestors and the nonrecursive global fallback are preserved. Prefix matching remains case-insensitive; GET, POST, and HEAD are not checked. - `getLibraryName(..., AllScopes)` honors unscoped `@clientName`, exact names, and the supported friendly-name fallback. Language-specific overrides and explicit OpenAPI operation IDs do not replace the common name. - Diagnostics target original operations. Both ARM and data-plane service endpoints are supported without requiring provider metadata. Client-only declarations outside HTTP services are not checked. - The rule does not mutate namespaces, select emitters, project API versions, reconstruct historical renamed names, or import AutoRest. - The single `client-sdk` registration is explicitly **disabled by default**. Data-plane/resource-manager rulesets and C# defaults are unchanged. - Separate rule implementations, their single-consumer traversal helper, and duplicate per-verb documentation/test files are replaced by one canonical rule, documentation page, and parameterized behavior suite. The unnecessary package-level AutoRest import isolation test has been removed; the canonical rule suite owns behavior coverage. ## Compatibility and migration **`use-create-for-put` is replaced, not retained as a second public alias.** Update linter configuration and `#suppress` diagnostic codes to `use-correct-verb`. Anyone who had explicitly enabled PUT naming guidance will now opt into PATCH and DELETE checks as well. The proposed PATCH/DELETE rule names are likewise replaced. Renaming an already-shipped SDK method can be breaking. The diagnostic and documentation retain this warning; approved compatibility exceptions can use operation-level suppression with a justification. This change does not itself rename generated methods. The package has one feature Chronus entry describing the consolidated behavior and configuration migration, plus a separate rulesets registration entry. Generated linter references, the ARM migration table, and TCGC documentation knowledge use the combined name. ## Consolidation and immutable source provenance | Check | Recorded lintdiff source | Native promotion incorporated | | --- | --- | --- | | PUT | #5455, `feature/lintdiff-put-in-operation-name`, recorded source `99cead8b6584f5cc1b0db3bf7810b53aa0eb221b` | Merged #5457; implementation already present on main. | | PATCH | #5664, source [`e009a4f53cc01eaa5c6a9ce85654fb1c4a8a6f4f`](e009a4f) | #5665 at [`8710828adc010571458235df8ff50e4019ce3107`](8710828). | | DELETE | #5670, source [`a0b5e8d9a9edee978219c2490c72eac7cc79845b`](a0b5e8d) | This PR at [`5fc361c18b79cfdf8460565fc7e3123669cc394d`](5fc361c), before consolidation. | The lintdiff sources, fixtures, snapshots, and migration evidence are not changed. The PUT source SHA is recorded historical provenance; its GitHub availability is not newly claimed. The approved native SDK-name adaptation remains intentional: a valid unscoped SDK override can fix an invalid authored name, or invalidate an otherwise compliant authored name. Serialized `operationId` underscore/group-prefix/missing-ID exemptions are not reproduced. Historical source corpus counts are not fresh combined-rule results, and exact Swagger diagnostic/count parity is not claimed. The naming policy remains in TCGC rather than introducing SDK dependencies into Azure Core or ARM. ## Regression coverage The canonical suite keeps valid/invalid mappings for all three verbs and exercises shared casing/prefix boundaries, common/scoped/exact/friendly-name precedence, explicit OpenAPI IDs, authored targets, group/underscore contrasts, multiple endpoints, and service selection once. The redundant naming/verb Cartesian product and response-status matrix have been removed. Standard ARM create/patch/delete aliases remain literal fixtures, including both common-name override directions. A mixed-verb regression requires three findings together while allowing GET/POST/HEAD. Nested explicit services require exactly one warning on the original operation; scope controls cover explicit-service descendants, excluded external declarations, nonrecursive global operations/direct interfaces, and concrete aliases versus templates. Public package registration/C# defaults have a separate registration suite; central ruleset coverage retains disabled-by-default activation and legacy-ID absence. The bespoke Markdown reader/fence parser and raw suppression test are deleted, not relocated. Documentation uses short standard-template before/after snippets and one focused common-name fix, without a setup appendix. All four actual final snippets were compiled once in an explicit public ARM context with the rule enabled: incorrect **3 warnings**, correct/common-name/suppression **0 diagnostics**. This receipt is one-time evidence, not continuous documentation coverage. Released-ID migration and SDK compatibility guidance remain intact. ## Validation (selected review fixes, October 21, 2026) - `mise exec -- pnpm --filter @azure-tools/typespec-client-generator-core build`: passed, including production TypeScript and TypeSpec library lint. - `mise exec -- pnpm --filter @azure-tools/typespec-client-generator-core test test/rules/use-correct-verb.test.ts test/linter-registration.test.ts --maxWorkers=1`: **30 tests passed in 2 files**, unchanged test/hook timeouts. - `mise exec -- pnpm --filter @azure-tools/typespec-azure-rulesets test test/validate-rules-defined.test.ts --maxWorkers=1`: **6 tests passed**. - TCGC docs regeneration, explicit native/generated Markdown formatting checks, root `pnpm format` and `pnpm lint`, Chronus status, and diff hygiene passed. - Docs regeneration retains existing missing-diagnostic-documentation and unused-parameter warnings on untouched APIs. - Initial focused-test collection found the admitted head's new examples dependency missing. Frozen-lockfile restoration linked JS dependencies but exited nonzero in unrelated Python preparation (pythonhosted TLS handshake failure); the dependency closure was then built sequentially. An initial target annotation error was corrected and the final target build passed. The install command is not reported as passing. These are local results for the selected findings, not a claim that fresh GitHub CI or review is complete. No new full-package/full-monorepo tests, website build, full test-project typecheck, or lintdiff corpus comparison is claimed. New human documentation feedback outside the selected findings remains for separate review; this update does not claim whole-PR readiness. <details> <summary>Original DELETE promotion description before consolidation (historical)</summary> ## Original Swagger linter - linter code: [DeleteInOperationName](https://github.com/Azure/azure-openapi-validator/blob/main/packages/rulesets/src/spectral/functions/delete-in-operation-name.ts) - linter doc: [delete-in-operation-name.md](https://github.com/Azure/azure-openapi-validator/blob/main/docs/delete-in-operation-name.md) Checks: - [x] Inspect DELETE operations selected from `paths` and `x-ms-paths`. - [x] Read `operationId`, ignoring absent, empty or non-string values. - [x] Ignore IDs without an underscore. - [x] Accept the case-sensitive `Delete` prefix before the underscore or after a word-character group prefix. - [x] Report a warning on the offending `operationId` when neither accepted form matches. ## How the Swagger linter works The Spectral function checks selected DELETE operation objects and applies serialized-ID regular expressions. It does not inspect SDK metadata or require ARM provider metadata. Its underscore/group-prefix exemptions can accept a Delete-prefixed group with a non-delete method, or ignore a standalone invalid method ID. Those serialization-dependent exemptions are intentionally not copied. Native endpoint selection also avoids warnings on declarations used only for client customization. The source migration evidence documents emitted-name overrides, historical version selection, one-sided projects and compilation failures rather than treating equal totals as proof of equivalence. ## Source TypeSpec lintdiff rule - Validator ID: `DeleteInOperationName`; canonical slug: `delete-in-operation-name`. - Local rule: `tsp-lintdiff-local-linter/delete-in-operation-name`. - [Original lintdiff source rule](https://github.com/Azure/typespec-azure/blob/feature/lintdiff-delete-in-operation-name/packages/typespec-lintdiff/src/rules/delete-in-operation-name.ts). - Canonical source repository: `Azure/typespec-azure`; source branch: `feature/lintdiff-delete-in-operation-name`. - Source draft PR: #5670. - Immutable source commit: `a0b5e8d9a9edee978219c2490c72eac7cc79845b`; development GitHub review clean after two Balanced rounds on this final pushed commit. - Source worktree: `C:\dev\worktrees\lintdiff-delete-in-operation-name`; clean, no uncommitted source changes. The source was assumed done for this promotion run and was not modified. It checks authored names on concrete HTTP service endpoints, DELETE only, with a case-insensitive `delete` prefix and warning targets on the original operation. Initial source commit `8af7a71258c41b8ca10950023c62cc4633bb24fc` is historical; the reviewed source above additionally fixes the harness's stale coverage-kind assertion without changing production behavior. ## Destination analysis The agent selected `@azure-tools/typespec-client-generator-core` because the approved naming-family promotion contract governs common SDK method names. Existing `use-create-for-put` and supported `getLibraryName(..., AllScopes)` establish that ownership and implementation pattern. The new public name is `use-delete-for-delete`. `Both` applicability and compiler/HTTP-only authored-name semantics make Azure Core a plausible authored-domain alternative. Core's `use-standard-names` already checks authored DELETE names in data-plane rulesets, but not the common SDK override domain. An SDK resolver must not introduce TCGC dependencies into Core or ARM. ARM fixture origin alone does not establish ARM-only semantics; this rule requires neither provider metadata nor ARM helpers in production. Existing TCGC `get-operation-name` is GET-specific and is not equivalent DELETE coverage. ## How the promoted TypeSpec linter works The root listener uses `getAllHttpServices` and checks concrete service operations with verb `delete`. A TCGC context created with `{ mutateNamespace: false }` resolves the common SDK name using `getLibraryName(context, operation, AllScopes)`. This honors unscoped client names, exact-name normalization and supported friendly-name fallback, independently of positive language-specific overrides and explicit OpenAPI operation IDs. No emitter or API-version projection is selected, and historical renamed names are not reconstructed. The prefix remains case-insensitive, severity remains warning and each finding targets the original operation, including when an override supplies the displayed SDK name. Group names do not mask an invalid method name. The authorized adaptation changes only the authored-name domain to common SDK names; source selection, DELETE condition, prefix policy and diagnostic targets remain intact. There is no hand-rolled resolver, serialized-reference inference, namespace mutation or emitter coupling. The `client-sdk` ruleset registers the new rule with plain `false`; ARM/data-plane rulesets and C# default rules are unchanged. Separate Chronus entries classify destination availability as `feature` and disabled-only ruleset registration as `internal`. ## Fixture-to-native test mapping | Original lintdiff fixture | Native vitest case | Coverage note | | --- | --- | --- | | [compliant-with-template](https://github.com/Azure/typespec-azure/blob/feature/lintdiff-delete-in-operation-name/packages/typespec-lintdiff/test/fixtures/DeleteInOperationName/compliant-with-template/main.tsp) | `it("reports a customized standard ARM delete endpoint")` | Actual source fixture contains a compliant delete and a violating remove despite its directory name. Replace legacy custom action/suppression with supported ArmResourceDeleteSync aliases; preserve prefix contrasts. | | [compliant-with-template](https://github.com/Azure/typespec-azure/blob/feature/lintdiff-delete-in-operation-name/packages/typespec-lintdiff/test/fixtures/DeleteInOperationName/compliant-with-template/main.tsp) | `it("accepts a standard ARM delete endpoint")` | Actual source fixture contains a compliant delete and a violating remove despite its directory name. Replace legacy custom action/suppression with supported ArmResourceDeleteSync aliases; preserve prefix contrasts. | | [remove-endpoint](https://github.com/Azure/typespec-azure/blob/feature/lintdiff-delete-in-operation-name/packages/typespec-lintdiff/test/fixtures/DeleteInOperationName/remove-endpoint/main.tsp) | `it("reports an invalid HTTP SDK name")` | Concrete remove endpoint warns; reduced generic HTTP operation preserves the method-name predicate. | | [delete-endpoint](https://github.com/Azure/typespec-azure/blob/feature/lintdiff-delete-in-operation-name/packages/typespec-lintdiff/test/fixtures/DeleteInOperationName/delete-endpoint/main.tsp) | `it("accepts a valid HTTP SDK name")` | Concrete delete-prefixed endpoint complies. | | [common-name-fixes-id](https://github.com/Azure/typespec-azure/blob/feature/lintdiff-delete-in-operation-name/packages/typespec-lintdiff/test/fixtures/DeleteInOperationName/common-name-fixes-id/main.tsp) | `it("accepts a common SDK name override")` | Authorized domain adaptation: unscoped valid SDK name fixes authored remove; source authored rule intentionally still warns. | | [common-name-invalidates-id](https://github.com/Azure/typespec-azure/blob/feature/lintdiff-delete-in-operation-name/packages/typespec-lintdiff/test/fixtures/DeleteInOperationName/common-name-invalidates-id/main.tsp) | `it("reports an invalid unscoped client name on the authored operation")` | Authorized domain adaptation: invalid common override warns despite authored deleteWidget; target remains original declaration. | | [ungrouped-remove](https://github.com/Azure/typespec-azure/blob/feature/lintdiff-delete-in-operation-name/packages/typespec-lintdiff/test/fixtures/DeleteInOperationName/ungrouped-remove/main.tsp) | `it("reports an invalid HTTP SDK name")` | Same minimal standalone endpoint proves no underscore/group exemption; shared native case is deliberate. | | [delete-group-remove](https://github.com/Azure/typespec-azure/blob/feature/lintdiff-delete-in-operation-name/packages/typespec-lintdiff/test/fixtures/DeleteInOperationName/delete-group-remove/main.tsp) | `it("does not let a Delete-prefixed group mask an invalid method name")` | Group prefix cannot hide invalid common method name. | | N/A (documentation regression) | `it("reports the published incorrect template alias")` | Compile actual Markdown setup and incorrect ArmResourceDeleteSync alias; require one warning on remove. | | N/A (documentation regression) | `it("accepts the published correct template alias")` | Compile actual Markdown correct alias with no target-rule diagnostics. | | N/A (documentation regression) | `it("accepts the published template client name override")` | Compile actual Markdown common-name override with no target-rule diagnostics. | | N/A (documentation regression) | `it("accepts the published template suppression")` | Compile actual Markdown suppression with the rule explicitly enabled through the compiler linter; require no diagnostics. | Additional native cases prove SDK override directions, language-specific and mixed names, exact names, friendly fallback, explicit-ID independence, original targets, multiple endpoint findings, non-DELETE controls, case-insensitive prefixes, source endpoint-selection regression, and a common client name on a supported standard ARM deletion alias. Framework-only import exclusion, ordinary inheritance/traversal, uninstantiated-template and suppression tests were not mechanically copied. No validator snapshots, fixtures or corpus payloads are included. ## Migration evidence See [source migration.md](https://github.com/Azure/typespec-azure/blob/feature/lintdiff-delete-in-operation-name/packages/typespec-lintdiff/test/fixtures/DeleteInOperationName/migration.md) for focused tests, representative services, full-corpus counts, one-sided project explanations, retained compile failures and uncertainty. Those counts describe the immutable authored-name source, not fresh SDK-domain equivalence or predicted impact. Promotion did not rerun the lintdiff harness or corpus. ## Validation - Required target dependency-closure build, TCGC package build/lint: exit 0. - Final emitter-free native rule suite: 28/28, including four tests that read and compile the actual published Markdown examples; target rule also passes all 28 cases in the full package run. - Required TCGC docs regeneration and explicit formatting/check of all four native/generated Markdown paths with `NUL` ignore override: exit 0; generated ignored rule page remains unstaged. - Following template-first skill update #5678 (created before this documentation update), the incorrect/correct, common-name override, and suppression examples use `ArmResourceDeleteSync<Widget>` instead of raw routes. The standalone shared setup includes all imports/usings. Incorrect `remove` produces exactly the target warning; correct `delete`, common-name override, and compiler-enabled suppression produce none. Existing generic HTTP behavior tests remain intact. - Rulesets build/lint: exit 0; discovery tests: 5/5. - Separate feature/internal Chronus entries verified LF; plain `pnpm chronus status`: exit 0. Before commit, both `change add` attempts reported no changed package, so the documented fallback created only these two entries. - Full TCGC initial required run: 1660 passed, one 10000ms per-test timeout in `arm client with sub clients`, two existing skips; 1663 tests in 102 files, natural exit 1. - Single coordinator-granted diagnostic, exactly `mise exec -- pnpm --filter @azure-tools/typespec-client-generator-core test --maxWorkers=1`: 1661 passed, zero failures, same two skips and 1663 tests/102 files, natural exit 0. Same code, assertions, dependencies and 10000ms test/hook limits; no pool change. Timeout recovered with UNPROVEN original cause. The task-wide diagnostic allowance is exhausted (1/1); the scoped concurrency profile may persist for matching later package validation without granting another retry. - Latest full package run after template documentation/test changes: 1665 passed, two unchanged existing skips, 1667 total tests in 102 files, exit 0 using the same `--maxWorkers=1` profile and unchanged test/hook timeouts. The four added tests account for the population increase. - Scoped TypeScript formatting/check and diff hygiene pass. The initial nine-file promotion patch previously passed read-only cached application check with an empty index; subsequent review/doc updates are scoped to the maintained rule documentation and native/rulesets tests. ## Validation limitations Required docs regeneration exits 0 with existing documentation warnings for `unnecessary-emitter-option` and unused API `@param` tags on untouched APIs; these unrelated warnings were retained, not repaired. The optional repository-wide wrapper was not warranted for this isolated rule and registration change. No local website/dependency-closure build was run; CI's Website job remains authoritative. The original full-suite timeout remains retained evidence, not an environmental diagnosis. ## Promotion sync policy The source remains immutable during promotion. A verified source-semantic gap blocks this PR and returns evidence to the owning queue, which alone may authorize its bounded source-repair cycle. Refresh native provenance incrementally only after a new clean reviewed source head; do not repair the source or silently alter its semantics in the promoted copy. The expressly approved common SDK naming adaptation is not source repair. ## Review cleanup (October 19, 2026) Renamed the groups to `HTTP SDK names` and `standard ARM operations`, preserving their separate SimpleTester and ARM/standalone setup. Removed the extra DELETE behavior file; focused package-level public-entry coverage retains the throwing AutoRest guard, now outside the PUT behavior suite as well. Removed redundant internal entries; feature and rulesets coverage remain. Published in 50c4b74. Fresh focused validation: **68 tests passed**, package production typecheck, root `pnpm format`, root `pnpm lint`, and diff hygiene passed. The full test TypeScript project retains pre-existing diagnostics (15 before, 15 after); the exact-head baseline comparison found no new diagnostics, accounting explicitly for moved tests and any removed duplicate fixture. This is not a claim that the full test TypeScript project is error-free or that pending CI has passed. These current file-layout/coverage statements supersede earlier descriptions of the removed test files; historical validation remains historical. </details> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Mark Cowlishaw <1054056+markcowl@users.noreply.github.com> Copilot-Session: 8bf3420c-d859-4ac1-81e8-2081d1fded84 Copilot-Session: 832854b7-de67-4f1f-8fe1-6a3e9897aeee
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The documentation in #5673 used raw HTTP routes for its incorrect/correct examples even though supported
ArmResourceDeleteSyncaliases demonstrate the same diagnostic contrast. The previous policy required ARM templates only after selecting an ARM audience, allowing a generic HTTP audience to bypass available templates.The user requested a template-first requirement in
/do-linter-development-task-one-by-oneand its related skills, with this skill-only PR targeting the current branch,feature/lintdiff-migration-new. The promotion documentation will be updated separately after this PR is created.Changes
Validation
Formatted and checked only the four changed skill Markdown files with the existing Prettier installation;
git diff --checkpassed. Reviewed the complete four-file diff and existing cross-references. No helper, package, dependency, submodule, or rule changes; no package builds or tests are needed.