Repository navigation
[Swagger Linter Migration] PatchInOperationName -> use-update-for-patch - #5665
Yuchao Yan (msyyc) wants to merge 2 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bf3420c-d859-4ac1-81e8-2081d1fded84
|
All changed packages have been documented.
Show changes
|
There was a problem hiding this comment.
🟢 Approval recommended
The implementation, registration, documentation, tests, and release entries are consistent and complete.
0 open findings
What changed in this PR
Promotes the migrated PatchInOperationName behavior into the ARM library as use-update-for-patch.
Changes:
- Adds and tests the PATCH operation naming rule.
- Registers the rule as disabled in the ARM ruleset.
- Adds documentation and release entries.
| File | Description |
|---|---|
website/.../reference/linter.md |
Lists the new rule. |
website/.../ARM/arm-rules.md |
Adds migration and impact guidance. |
packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts |
Registers the rule as disabled. |
packages/typespec-azure-resource-manager/test/rules/use-update-for-patch.test.ts |
Covers naming, verbs, responses, and documentation examples. |
packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.ts |
Implements the rule. |
packages/typespec-azure-resource-manager/src/rules/use-update-for-patch.md |
Documents usage and suppression. |
packages/typespec-azure-resource-manager/src/linter.ts |
Registers the rule with the ARM linter. |
packages/typespec-azure-resource-manager/README.md |
Lists the new rule. |
.chronus/changes/register-use-update-for-patch.md |
Records ruleset registration. |
.chronus/changes/arm-use-update-for-patch.md |
Records the ARM feature. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
📦 Package size report✅ No notable package size changes compared to the base branch. 14 package(s) with no notable change
Packed = gzipped |
commit: |
|
You can try these changes here
|
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
## Purpose Make the TCGC operation-naming promotion pattern explicit for similar rules such as `DeleteInOperationName`, `ListInOperationName`, and `PatchInOperationName`, as requested. This independent, skill-only PR targets the current branch, `feature/lintdiff-migration-new`; it does not modify the rule promotion PRs or package code. ## Evidence and change PUT promotion #5457 established common SDK naming in TCGC. PATCH promotion #5665 initially used authored operation names in ARM and was corrected to the same TCGC pattern at the user's request. Its independently verified Balanced review reported zero findings on commit `5d131d487ec0e1dd8f1bd0b6ff1050fe4df2f747`. The promotion skill now gives this naming family an explicit, bounded adaptation contract: prefer TCGC, use non-mutating context creation and `getLibraryName(..., AllScopes)`, honor common client/exact/friendly names, and ignore emitter-scoped names and OpenAPI operation IDs. Preserve rule-specific endpoint, verb, paging, prefix, severity, and diagnostic-target behavior; a list-only policy must not be replaced mechanically by a generic GET naming rule. The instructions also require both-direction override regressions, appropriate opt-in audience handling, disabled `client-sdk` registration, native documentation and release metadata, and an explicit distinction between immutable authored-source evidence and adapted SDK-name behavior. The review skill consumes that same authorized contract rather than incorrectly classifying the documented name-domain adaptation as an unfixed source defect. Other semantic changes retain the existing source-repair stop policy. No source changes, new diagnostics, auto-merge, or new retry permissions are authorized by this update. ## Validation Inspected the complete two-file skill diff, checked the new cross-skill anchor and existing references, and ran scoped Prettier write/check and `git diff --check`. The initial formatter invocation attempted to load a plugin from the uninitialized skill-only worktree. It failed before formatting; using the existing coordinator's identical formatter configuration and installed plugins passed without installing dependencies or initializing/building `core`. No package tests, builds, documentation regeneration, corpus runs, or release entries are needed for this Markdown-only change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bf3420c-d859-4ac1-81e8-2081d1fded84
Original Swagger linter
linter code: PatchInOperationName
linter doc: patch-in-operation-name.md
Select PATCH operation IDs under
pathsandx-ms-paths.Exempt missing, empty, non-string, and ungrouped IDs.
Require grouped IDs to match the case-sensitive
^(\w+)_(Update)or^(Update)alternatives.Report warnings at the selected operation ID independently of response status codes.
How the Swagger linter works
Spectral selects PATCH operation IDs and applies its grouped-name regular expression. Its second alternative accepts an
Updategroup, such asUpdate_Recommendation, regardless of the method name. Generated naming and SDK overrides can mask authored names. These group-name exemptions are not reproduced as native SDK naming policy.Source TypeSpec lintdiff rule
PatchInOperationName; canonical slug and local rule:patch-in-operation-name.patch-in-operation-name.Azure/typespec-azure:feature/lintdiff-patch-in-operation-name.e009a4f53cc01eaa5c6a9ce85654fb1c4a8a6f4f, PR #5664.C:\dev\worktrees\lintdiff-patch-in-operation-name; the recorded clean source had no uncommitted rule changes. This correction reads only pinned Git source and does not edit the source checkout.The source is assumed done for this promotion and remains unchanged. Its authored-name contract and corpus counts are source evidence, not claims about the corrected SDK-name rule.
Destination analysis
The user explicitly selected TCGC and approved the same common SDK naming adaptation as merged #5457,
use-create-for-put. This correction updates the existing draft #5665, not the source rule, and retains the public nameuse-update-for-patch.The policy governs the common SDK method name. TCGC owns
getLibraryName(..., AllScopes)and its supported client naming metadata. ARM ownership would incorrectly encourage checking onlyoperation.nameor adding an ARM-to-TCGC dependency. Shared Azure Core's data-planeuse-standard-namespolicy is different: it checks PATCH naming only when a response includes 201 and permitscreate. Swagger metadata sayingBothdoes not override the explicitly approved SDK semantic ownership.All prior ARM rule implementation, registration, tests, release notes, and generated reference additions are removed from this PR. The maintained ARM inventory links to the TCGC rule, following the PUT precedent. This remains opt-in ARM SDK naming guidance, not automatic naming enforcement on every data-plane API.
How the promoted TypeSpec linter works
The implementation mirrors current main's
use-create-for-put: create a TCGC context withmutateNamespace: false, resolve concrete HTTP endpoints usinggetAllHttpServices, and check each PATCH operation'sgetLibraryName(tcgcContext, operation, AllScopes)for a case-insensitiveupdateprefix. Common unscoped@clientName,exactnames, and the usual friendly-name fallback affect compliance. Language-scoped client names and OpenAPI operation IDs do not define this common name.One warning targets each offending original operation, even when its invalid name comes from a client override. Response status codes, interface names, and underscore suffixes do not make an invalid prefix compliant. No emitter, generated reference-string inference, or version projection is used. Removed source operations remain visible; historical renamed names are not reconstructed.
There is no provider guard: enabling this opt-in rule selects its audience, as in
use-create-for-put. A minimal ordinary service namespace is explicitly tested. The rule's message names the SDK method and warns about compatibility when renaming already-shipped SDK APIs.User-approved semantic adaptation/provenance: this intentionally changes the immutable lintdiff rule's authored-name domain to common SDK naming. For example, authored
modifywith common@clientName("updateWidget")now passes; authoredupdatewith common@clientName("modifyWidget")now warns onupdate. This is not a source repair and is not behavior/count parity with the authored source. Seven source corpus differences attributed to SDK overrides are therefore not asserted as findings of this corrected rule. No new corpus run or result is claimed.Fixture-to-native test mapping
it("validates the exact incorrect documentation example")ArmResourcePatchSync<Widget, Properties = WidgetProperties>namedmodify; one warning on the original operation.it("validates the exact correct documentation example")update; no warning.it("validates the exact correct documentation example")it("ignores an operationId that would mask an invalid common SDK name")modifyWidgetConfigdespite its directory name; reduced to the invalid default/common-name branch without unrelated suppressions.it("accepts an unscoped client name override")it("reports an invalid unscoped client name on the authored operation")update, not the override decorator.it("ignores emitter-scoped overrides of a compliant default name")it("does not let an emitter-scoped override mask an invalid default name")it("uses the unscoped name when scoped and unscoped overrides coexist")it("does not let a scoped name mask an invalid unscoped override")it("honors client names on standard ARM patch template aliases")it("reports invalid common client names on standard ARM patch template aliases")it("normalizes exact client names")it("reports invalid exact client names on the authored operation")it("uses the friendly name fallback")it("reports an invalid friendly name on the authored operation")it("prefers a common client name over a friendly name")it.each(["patch", "modify", "create", "patch_Update", "u", "updat"])("rejects PATCH name %s")it.each(["update", "updateWidget", "updateTags", "UpdateTags", "UPDATE", "uPdAtEWidget"])("accepts update prefix %s")it.each(["get", "put", "post", "delete", "head"])("ignores HTTP verb %s")it.each([200, 201, 202, 204])("checks PATCH names independently of response code %s")it("does not let an interface name mask the operation prefix")it("checks ordinary service namespaces without provider metadata")it("reports distinct offending endpoints on their authored operations")it("ignores an operationId that would invalidate a compliant common SDK name")it("retains historical operations but does not simulate historical names")it("validates the exact common client name documentation example")it("validates the exact suppression documentation example")Standard framework-only traversal/template-filtering/import cases are not duplicated. No validator snapshots or migration harness infrastructure are copied. The native suite mocks only AutoRest as an emitter boundary; TCGC is the intended native dependency.
Migration evidence
See source migration.md for focused source coverage, real-service comparisons, selected-version attribution, one-sided projects, compile failures, and remaining uncertainty. Its detailed corpus table is not duplicated here. Those counts describe the authored source, not this user-approved SDK adaptation.
Registration and release notes
The TCGC rule is registered in the library and with plain
enable: falseinclient-sdk. It is absent from ARM/data-plane ruleset enable lists. Existing PUT registration is unchanged. A rulesets regression asserts exact PATCHfalseand absence of the prior ARM entry. TCGC has afeatureChronus entry; disabled-only SDK ruleset registration has a separateinternalentry. No ARM package release-note entry remains.Validation
dist; existing dependencies reused, no install).regen-docs: completed; references inspected. Scoped empty-ignore Prettier write/check explicitly covered TCGC README, authored rule documentation, website linter reference/generated page, retained tracked ARM references and maintained ARM matrix.Required full package validation and retained timeout history
The full affected package checks are required and passed with the unchanged populations and timeout settings under the bounded serial profiles:
mise exec -- pnpm --filter @azure-tools/typespec-azure-resource-manager test --maxWorkers=1naturally exited 0; 66 files, 474 tests passed, no skipped tests; 355.82s. Parent authorized one per-test-only reduced-concurrency diagnostic after verifying the original timeout-only failure.mise exec -- pnpm --filter @azure-tools/typespec-client-generator-core test --maxWorkers=1naturally exited 0; 102 files, 1681 tests passed and the same two existing skips (1683 total); 337.49s. The user specifically directed recovery from the notified timeout blocker; parent bounded that grant to this one full serial rerun after ARM completed.Original failures are preserved, not replaced:
mise exec -- pnpm --filter @azure-tools/typespec-client-generator-core test: naturally exited 1; 102 files (18 failed, 84 passed), 1683 tests (36 failed, 1645 passed, 2 skipped). All failures are timeouts: 34 ordinary 10,000ms test timeouts and two 10,000ms setup-hook timeouts (first PUT/PATCH naming cases).mise exec -- pnpm --filter @azure-tools/typespec-azure-resource-manager test: naturally exited 1; 66 files (4 failed, 62 passed), 474 tests (4 failed, 470 passed). All four failures are ordinary 10,000ms test timeouts.The original runs started concurrently; resource contention is plausible, but no causal environmental claim is established by these receipts. The original TCGC PATCH suite had 43 passing cases; its first case failed in setup at
beforeEach(reported 10028ms), not in the name predicate. All four final documentation examples passed even in that failed full run. An identical first-case PUT setup also timed out. The bounded read-only inspection found no concrete deterministic task-introduced code/configuration defect explaining the distributed 34 ordinary and two hook timeouts. The serial passes preserve test IDs, assertions, counts, skips, dependencies, 10,000ms ordinary test timeout and default 10,000ms hook timeout; concurrency is the only runner change.Both one-run recovery grants were consumed before their commands. No timeout increase, baseline comparison, source change, or further retry occurred. Raw originals and serial receipts are retained. A fresh independent local precommit reviewer checked all 19 changed paths, including untracked additions and the main-relative semantic PR diff, and found no significant issues. Parent independently verified the exact diff, file hashes, raw receipts, unchanged input head, empty index and passing recovered required gates before explicitly approving this existing-PR publication.
Correction history: initial focused run had 43 passed and one failure because the direct rule tester collects diagnostics before compiler suppression. One understood promotion test-integration correction uses full compiler linting only for the actual suppression documentation snippet; the same 44-test selection then passed. Source semantics were not changed. The unrelated initial readiness probe also reported missing TCGC
dist, which justified the successful dependency-closure build.Validation limitations
TCGC regeneration reports existing unused
@paramwarnings and missing documentation forunnecessary-emitter-option; ARM regeneration reports existing missing rule/diagnostic-documentation warnings. Both commands exit successfully; these are not new rule warnings. No website build or optional broad wrapper was run. CI's Website job remains authoritative. Original required full-suite timeout failures are disclosed above alongside their specifically authorized serial passing checks; they were not demoted to supplemental checks.Promotion sync policy
The source remains immutable. This explicit completed-task correction approves only the documented TCGC SDK-name adaptation; it does not reopen source repair, resume the old queue, or reset its counters. Further source-semantic defects require the applicable explicitly authorized repair flow. Adaptation-only findings can be corrected here within their finite budget. The destination and semantic change invalidated prior ARM promotion approval; a fresh independent precommit review has now completed with no significant issues. Parent owns the fresh Balanced post-publication loop. Neither PR is auto-merged.