[Swagger Linter Migration] ConsistentPatchProperties (origin) - #5439
Merged
Yuchao Yan (msyyc) merged 3 commits intoSep 11, 2026
Conversation
Remove the TCGC/AutoRest scope dependency while retaining native HTTP and model comparison. Add scope-boundary regressions and record partial Swagger equivalence with refreshed corpus evidence. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…onsistent-patch-properties-native
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The functional changes are well-contained and reinforced with focused tests/fixtures, with only minor wording nits in newly added test descriptions.
Pull request overview
This PR updates the migrated ConsistentPatchProperties TypeSpec lint rule to use the native compiler/HTTP semantic model (instead of AutoRest/TCGC scope filtering), and it explicitly records the resulting Swagger-vs-native contract differences as partial equivalence with additional focused fixtures and evidence.
Changes:
- Refactors the rule implementation to remove
@azure-tools/typespec-client-generator-corecontext/scope checks and rely onProgram+getAllHttpServices. - Adds/extends unit tests and four scope-focused fixtures/snapshots to demonstrate intentional native-vs-emitted differences.
- Updates fixture documentation and evidence metadata (
coverageKind: partial, migration narrative, corpus evidence schema).
File summaries
| File | Description |
|---|---|
| packages/typespec-lintdiff/src/rules/consistent-patch-properties.ts | Removes TCGC/emitter-scope coupling; uses native HTTP semantic graph + compiler APIs for property comparison. |
| packages/typespec-lintdiff/test/rules/consistent-patch-properties.test.ts | Adds regression tests proving behavior without AutoRest/TCGC context and asserting scope doesn’t change native outcomes. |
| packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/rule.md | Marks coverage as partial and documents the native boundary + scope-driven gaps explicitly. |
| packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/migration.md | Rewrites conclusion/evidence summary to reflect the native-boundary repair and partial equivalence decision. |
| packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/corpus-evidence.json | Updates evidence schema/metadata and records partial coverage + run fingerprints. |
| packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/* | Adds a validator-only scope gap fixture and snapshots (validator vs native expectations). |
| packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/* | Adds a TypeSpec-only scope gap fixture where PATCH exists natively but is omitted from emitted Swagger. |
| packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-property/tsp-diagnostics.json | Records the new native diagnostic outcome for a PATCH property omitted by AutoRest scope. |
| packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-get-fallback/tsp-diagnostics.json | Records the new native diagnostic outcome where a GET fallback exists natively but is omitted by AutoRest scope. |
Review details
Suppressed comments (2)
packages/typespec-lintdiff/test/rules/consistent-patch-properties.test.ts:55
- This test name says it runs "without loading an emitter", but the test harness still loads the
@typespec/openapiemitter library via the shared tester setup. Consider tightening the wording to specifically call out that it doesn’t load AutoRest (@azure-tools/typespec-autorest) or the client generator core.
it("accepts a same-level subset without loading an emitter or client generator", async () => {
packages/typespec-lintdiff/test/rules/consistent-patch-properties.test.ts:73
- This test name says it runs "without loading an emitter", but the test harness still loads the
@typespec/openapiemitter library via the shared tester setup. Consider tightening the wording to specifically call out that it doesn’t load AutoRest (@azure-tools/typespec-autorest) or the client generator core.
it("uses the same-path GET model without loading an emitter or client generator", async () => {
- Files reviewed: 17/17 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The refactor cleanly removes the TCGC dependency while preserving intended semantics, and the added/updated tests and fixtures comprehensively lock in the new native-boundary behavior.
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 0 new
- Review effort level: Lite
Yuchao Yan (msyyc)
marked this pull request as ready for review
September 10, 2026 03:34
Yuchao Yan (msyyc)
requested review from
Mark Cowlishaw (markcowl),
Timothee Guerin (timotheeguerin) and
Jeff Fisher (xirzec)
as code owners
September 10, 2026 03:34
Yuchao Yan (msyyc)
merged commit Sep 11, 2026
af74f25
into
feature/lintdiff-migration-new
3 checks passed
Yuchao Yan (msyyc)
deleted the
feature/lintdiff-consistent-patch-properties-native
branch
September 11, 2026 02:19
This was referenced Sep 11, 2026
Yuchao Yan (msyyc)
added a commit
that referenced
this pull request
Sep 11, 2026
## Original Swagger linter linter code: [ConsistentPatchProperties](https://github.com/Azure/azure-openapi-validator/blob/6243cb01c16c7535cd3b8df6f45fbeb3c095ed7f/packages/rulesets/src/spectral/functions/consistent-patch-properties.ts) linter doc: [consistent-patch-properties.md](https://github.com/Azure/azure-openapi-validator/blob/6243cb01c16c7535cd3b8df6f45fbeb3c095ed7f/docs/consistent-patch-properties.md) - [x] Select PATCH operations and the first body parameter schema. - [x] Select PATCH 200, PATCH 201, same-path GET 200, then GET 201 as the resource comparison schema. - [x] Skip comparison when either selected schema is absent. - [x] Merge inherited `allOf` properties and compare property names at the same nesting level. - [x] Recursively compare named object properties, reporting request properties missing from the response schema or placed at another level. - [x] Report each missing path at the PATCH body parameter's schema location. ## Why this follow-up repair is needed PR #5439 removed the downstream emitter-context dependency and has merged. Its official ARM promotion exposed two native inheritance bugs, repaired in separate commits here; the unfinished promotion remains separate. For `Update extends Base { extra?: never; }`, the previous walk skipped the derived `never` declaration and revived `Base.extra`, creating a false positive. The same override in the response incorrectly accepted PATCH `extra`. A redeclaration changing its encoded name could likewise leave the obsolete base name visible. The resumed promotion then found that synthetic discriminator metadata on a derived model hid an effective inherited authored object encoded to the discriminator name. Moving an inline `payload?: { extra?: string }` encoded as `kind` into a base model incorrectly lost nested comparison. This missed request violations and caused false positives for inherited response objects. ## How the Swagger linter works The Spectral rule selects `$.paths.*.patch`. It compares the first body parameter schema with the first available response schema in the precedence above. Its recursive helper merges `allOf` properties and follows matching named properties; it reports a missing parent property at the operation-body schema path rather than using TypeSpec source-property locations. Arrays and scalars do not add named element/key comparisons. The four new comparison fixtures expose native/emitted contract differences, not validator defects. AutoRest omits a derived `never` property but retains the base via `allOf`. For inherited encoded discriminator objects, its derived synthetic string overrides the base object during the validator's schema merge. Each pair produces one native-clean/validator-violation case and one native-violation/validator-clean case. The production lint honors effective TypeSpec authoring rather than simulating emission. ## How the migrated TypeSpec linter works Compiler `getProperty(model, property.name)` identifies the effective authored declaration before JSON encoded-name resolution and `never` filtering. A shadowed base declaration is skipped even if the derived property contributes no payload property. The helper now collects the complete effective authored shape before adding missing synthetic discriminators. Authored properties, including inherited encoded objects, take precedence. Effective source-name lookup also prevents synthesizing an extra discriminator when an inherited authored discriminator name is encoded differently. Genuine synthetic discriminators remain supported, and `never` overrides do not resurrect shadowed nested shapes. The rule still uses the native HTTP service graph, PATCH/GET response-body selection (including status-code ranges after exact matches), compiler model/nullable/discriminator metadata, and recursive same-level comparison. Diagnostics retain authored property/model targets and existing multiplicity; no new deduplication or version projection is introduced. The lintdiff-only provider isolation guard is unchanged. Fourteen added native cases cover request/response `never` overrides, intermediate inheritance, encoded-name redeclarations, inherited/inline discriminator objects, differently encoded discriminator source names, and genuine synthetic metadata. Existing scope differences remain documented; no emitter, TCGC, or OpenAPI dependency was added to production logic. ## Migration evidence [ConsistentPatchProperties migration evidence](https://github.com/Azure/typespec-azure/blob/feature/lintdiff-consistent-patch-properties-native/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/migration.md) records both inheritance repairs, their native/emitted matrix, focused comparisons, the final full-corpus rerun, exact diagnostic-multiset comparison, one-sided project explanations, compile exclusions, and remaining Informatica/version-attribution uncertainty. Swagger equivalence remains partial. Unchanged corpus findings do not replace the supported native-shape regressions that exposed these bugs. ## Validation and scope The source package build, changed-file oxlint/formatting, 23 native rule cases, and all 22 comparison cases passed. Snapshots were regenerated and checked without updates, including after formatting. The final full corpus completed on September 11 at 13:23 +08:00; its six compile exclusions remain explicitly recorded in the linked evidence. Generated corpus data was retained outside the repository and restored before publication. This PR changes only the rule, directly related tests/fixtures, and rule-local migration evidence. Promotion may resume from the final reviewed source; neither the unfinished official copy nor unrelated rules were changed here. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
Summary
Follow-up source repair to #5399. Promotion exposed that preserving AutoRest-specific
@scopebehavior would make the ARM library depend on client-generator-core, which already references ARM in its build graph. This repair adopts the native TypeSpec contract instead of introducing a downstream dependency or a private-state adapter.The rule no longer constructs a TCGC context or filters operations/properties with
isInScope. It uses compilerProgramand HTTP semantic APIs. Swagger equivalence is explicitly partial, not inferred from project overlap.Original Swagger linter
linter code: ConsistentPatchProperties
linter doc: consistent-patch-properties.md
200, then201, then same-path GET200/201response schemas.How the Swagger linter works
The Spectral selector is
$.paths.*.patch, with references resolved.consistentPatchPropertiesselects the request and response schemas, delegates their recursive comparison todiffSchema, and emits one body-schema diagnostic per reported property path. It does not evaluate TypeSpec decorators. AutoRest has already omitted operations and properties outside its scope before validation.Consequently, a C#-scoped PATCH, request property, or GET fallback can produce no Swagger finding even though it exists in the native HTTP contract. Conversely, a C#-scoped response property can disappear from Swagger and cause a validator finding despite a matching native PATCH subset. Neither side is a bug for these differing contracts.
The earlier migration also recorded status-range differences, an Informatica validator miss, API-version population differences, and emitted-occurrence versus authored-target multiplicity. Those distinctions remain evidence rather than reasons to recreate emitter internals inside a lint.
How the migrated TypeSpec linter works
getAllHttpServicessupplies native HTTP operations. The rule compares PATCH bodies with PATCH response bodies or a same-path GET fallback, preserving exact-code precedence over containing status-code ranges. Recursive model traversal retains inheritance, nullable object handling, JSON encoded names, compiler discriminator metadata, cycle guards, and existing authored diagnostic targets.The only production behavior change is that client/emitter scope no longer removes native operations or properties from this comparison. The existing provider check remains lintdiff's ARM isolation in its mixed ruleset; promotion must assess official-library applicability separately. Other rules still use TCGC, so this PR does not remove package-wide dependencies.
Focused evidence
Direct native regressions compile without AutoRest, including tests that do not load client-generator-core at all. Additional tests load only TCGC's decorator library to prove that its scope metadata does not change native lint outcomes.
scoped-propertyproperties.clientOnlyscoped-get-fallbackdisplayNamescoped-patch-operationextrascoped-response-propertydescriptionabsent from emitted responseSnapshots preserve the comparison differences instead of disguising them as equivalence.
coverageKindis changed topartial. No emitter implementation, harness behavior, official package, or generated corpus data is changed.Migration evidence
See migration.md for the latest full-run counts, project overlap, API-version attribution, compile failures, source revisions, code-backed examples, and evidence limitations. The detailed corpus tables are intentionally not duplicated here.
Promotion status
Promotion remains paused. This source repair changes the previously done rule's contract; the repaired rule must be accepted as done before a new promotion attempt. No ARM-to-TCGC dependency or official-library enablement is introduced by this PR.