[Swagger Linter Migration] UnSupportedPatchProperties - #5384
[Swagger Linter Migration] UnSupportedPatchProperties#5384Yuchao Yan (msyyc) wants to merge 3 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Yuchao Yan (msyyc)
added
the
int:azure-specs
microsoft-github-policy-service
Bot
added
lib:azure-resource-manager
|
All changed packages have been documented.
Show changes
|
There was a problem hiding this comment.
🟡 Changes recommended
The rule’s ARM/provider-namespace detection is incorrect (can miss nested namespaces and misclassify global ops), and the suggested regression test should be added to prevent recurrence.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR promotes the Swagger validator rule UnSupportedPatchProperties into the official @azure-tools/typespec-azure-resource-manager linter as no-unsupported-patch-properties, adds native Vitest coverage, and wires the rule into ARM documentation and ruleset registration (disabled by default).
Changes:
- Adds the new ARM linter rule implementation and documentation (
no-unsupported-patch-properties). - Adds a native Vitest suite covering the migrated scenarios.
- Registers the rule in the ARM linter + ruleset and updates ARM docs/readme + Chronus changeset.
File summaries
| File | Description |
|---|---|
| website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md | Adds the new rule to the ARM linter rule reference table. |
| packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts | Registers the rule in the ARM ruleset as disabled by default. |
| packages/typespec-azure-resource-manager/test/rules/no-unsupported-patch-properties.test.ts | Adds native Vitest coverage for the migrated rule behavior. |
| packages/typespec-azure-resource-manager/src/rules/no-unsupported-patch-properties.ts | Implements the new ARM linter rule that flags non-patchable PATCH body properties. |
| packages/typespec-azure-resource-manager/src/rules/no-unsupported-patch-properties.md | Adds end-user rule documentation and examples. |
| packages/typespec-azure-resource-manager/src/linter.ts | Adds the rule to the ARM linter’s exported rule list. |
| packages/typespec-azure-resource-manager/README.md | Adds the rule to the package rule table. |
| .chronus/changes/promote-un-supported-patch-properties-2026-09-03-13-20-00.md | Adds a changeset describing the new rule promotion and default-disabled registration. |
Review details
Suppressed comments (1)
packages/typespec-azure-resource-manager/src/rules/no-unsupported-patch-properties.ts:191
- Rename the helper function to match what it actually does (Lifecycle visibility checks) so it stays consistent with the call site name and doesn’t imply
x-ms-mutabilityhandling.
function canSharePropertyUsingReadonlyOrXmsMutability(
program: Program,
property: ModelProperty,
): boolean {
- Files reviewed: 8/8 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
commit: |
📦 Package size report3 packages changed size, +13.21 KB (+0.1%) packed overall.
10 package(s) with no notable change
Packed = gzipped |
|
You can try these changes here
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The rule implementation is wired correctly, covered by targeted native tests, and is registered as disabled-by-default so it won’t introduce unexpected diagnostics for existing projects.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Original Swagger linter
The original rule performs these checks on every ARM Swagger PATCH operation:
id.name.type.location.properties.provisioningState.readOnly: trueproperties.x-ms-mutabilityexcludesupdate.How the Swagger linter works
The ARM Spectral ruleset selects PATCH operations under both
pathsandx-ms-pathswith resolved references enabled.validatePatchBodyParamPropertiesfinds the first body parameter, resolves its schema throughgetProperties, checks the configured top-level denylist, then separately resolves thepropertiesbag and checksprovisioningState. Every failure targets the PATCH body parameter rather than the individual schema property.For each selected property, the validator treats
readOnly: trueor anx-ms-mutabilityarray withoutupdateas exempt. Spectral's resolved traversal can drop annotations that are siblings of$ref; the migration corpus confirmed false positives for referenced read-onlyprovisioningStateand immutablelocationproperties. The TypeSpec rule intentionally preserves the authored lifecycle semantics instead of reproducing that validator defect. Swagger also reports emitted occurrences, while one authored TypeSpec property can be reused across operations and versions.Source TypeSpec lintdiff rule
UnSupportedPatchPropertiesunsupported-patch-propertiesun-supported-patch-propertiesfeature/lintdiff-un-supported-patch-propertiesde8d12e0244eb128604d2c4208f19325837fa852C:\dev\worktrees\lintdiff-un-supported-patch-propertiesunsupported-patch-properties.tsThe user marked this source rule done after its provider-namespace ownership repair merged. Promotion did not modify the lintdiff source, fixtures, snapshots, package metadata, or documentation.
Destination analysis
The rule belongs in
@azure-tools/typespec-azure-resource-manager. Its semantics are ARM RPC-Patch-V1-02, every fixture uses ARM service shapes, catalog metadata saysapplicability: ARMandsources: ["arm"], and the validation report infers the resource-manager ruleset. Azure Core is not a plausible destination because the rule is not shared data-plane guidance.Existing official rules do not duplicate this behavior.
arm-resource-patchrejects fields absent from the resource but permits envelope fields such asname,type, andlocation; custom ARM PATCH templates also accept arbitrary patch models. The promoted rule therefore closes the documented official coverage gap rather than changing an existing rule.The official name is
no-unsupported-patch-properties, following the TypeSpecno-<thing>convention instead of retaining the validator's historical casing and wording. The rule is explicitly registered asfalsein the resource-manager ruleset so promotion does not immediately add diagnostics to existing service specifications.How the promoted TypeSpec linter works
When enabled in the ARM ruleset, the rule visits operations and selects HTTP PATCH requests with bodies. The official implementation deliberately omits the lintdiff source's provider-namespace guard: lintdiff enables ARM and data-plane rules together and must self-scope, while the destination ARM ruleset already supplies this applicability boundary. This matches neighboring operation-based ARM rules and avoids suppressing diagnostics solely because provider decoration is absent or malformed.
The rule unwraps nullable single-model bodies while leaving scalar and multi-model unions compliant. For the PATCH model and nested
propertiesmodel, it walks inherited properties, resolves theirapplication/jsonencoded names, and uses HTTP metadata plus PATCH request visibility to determine whether each property reaches the payload. Lifecycle visibility distinguishes writable properties from read-only or create-only properties. Diagnostics target authored project properties; library-originated properties fall back to the operation. This avoids emitting duplicate diagnostics for generated Swagger occurrences while preserving actionable source locations.Promotion-specific adaptations are the official rule name, removal of the lintdiff-only applicability guard, canonical documentation URL, and package registration. Property-selection, visibility, and diagnostic semantics remain aligned with the source rule.
Fixture-to-native test mapping
patch-with-id-name/main.tspit("reports writable id, name, and type properties")nested-arm-namespace/main.tspit("reports unsupported properties in nested ARM namespaces")global-operation-compliant/main.tspit("does not require provider namespace decoration when the ARM rule is enabled")patch-with-location-provisioning-state/main.tspit("reports writable location and properties.provisioningState on a nullable body")encoded-inherited-properties/main.tspit("reports inherited properties by their encoded JSON names")readonly-immutable-properties-compliant/main.tspit("accepts read-only and immutable reserved properties")readonly-ref-validator-discrepancy/main.tspit("accepts a read-only referenced provisioning state")$reffalse positive.non-object-bodies-compliant/main.tspit("accepts scalar and multi-model-union bodies")Migration evidence
The checked-in
migration.mdrecords focused fixture coverage, the real-service comparison, latest full-corpus counts, all one-sided project explanations, compile-failure exclusions, and the conclusion that no semantic uncertainty remains.Validation
Validation blocker
The broad website-skipped
pnpm validate:prcompleted all nine phases on the original promotion commit but failed because two unrelated full-repository tests exceeded their existing timeouts:@azure-tools/typespec-javaeng/scripts/core-commit.test.ts: 5-second timeout@azure-tools/typespec-azure-playground-websitecompile Tracked Resource: 10-second timeoutThe same run passed branch freshness, the full build, lint, format, spelling, and diff hygiene. The narrower native validation above is complete and was rerun after the destination adaptation.
Promotion sync policy
If review identifies a semantic gap, this promotion should be blocked until the user explicitly reopens lintdiff repair. Destination-only applicability adaptation must be documented and validated rather than copied or removed mechanically.