Repository navigation
[Swagger Linter Migration] AllResourcesMustHaveGetOperation -> resource-missing-read - #5654
Merged
Yuchao Yan (msyyc) merged 9 commits intoOct 19, 2026
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
microsoft-github-policy-service
Bot
added
int:azure-specs
Contributor
|
All changed packages have been documented.
Show changes
|
Contributor
📦 Package size report✅ No notable package size changes compared to the base branch. 14 package(s) with no notable change
Packed = gzipped |
commit: |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Diagnostic targeting can incorrectly select an unrelated lifecycle interface when operations are split.
1 open finding
What changed in this PR
Adds the opt-in resource-missing-read ARM lint rule for writable resources.
Changes:
- Implements and registers the rule.
- Adds tests and documentation.
- Registers it as disabled in the resource-manager ruleset.
| File | Description |
|---|---|
.chronus/changes/arm-resource-missing-read.md |
Records the ARM package feature. |
.chronus/changes/register-resource-missing-read.md |
Records ruleset registration. |
packages/typespec-azure-resource-manager/README.md |
Lists the new rule. |
packages/typespec-azure-resource-manager/src/linter.ts |
Registers the rule. |
packages/typespec-azure-resource-manager/src/rules/resource-missing-read.md |
Documents usage and migration semantics. |
packages/typespec-azure-resource-manager/src/rules/resource-missing-read.ts |
Implements validation and diagnostic targeting. |
packages/typespec-azure-resource-manager/test/rules/resource-missing-read.test.ts |
Tests lifecycle and documentation scenarios. |
packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts |
Registers the rule as disabled. |
website/src/content/docs/docs/howtos/ARM/arm-rules.md |
Adds the ARM inventory entry. |
website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md |
Adds generated rule reference. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
|
You can try these changes here
|
6 tasks done
Refresh from reviewed lintdiff source c4cd068, preserve create-before-update target priority, and add exact-target and full-linter suppression regressions with native guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Yuchao Yan (msyyc)
added a commit
that referenced
this pull request
Oct 10, 2026
…5653) ## Original Swagger linter linter code: [AllResourcesMustHaveGetOperation](https://github.com/Azure/azure-openapi-validator/blob/main/packages/rulesets/src/native/legacyRules/AllResourcesMustHaveGetOperation.ts) linter doc: [all-resources-must-have-get-operation.md](https://github.com/Azure/azure-openapi-validator/blob/main/docs/all-resources-must-have-get-operation.md) Original checks: - [x] Run composed ARM validation over the document using `ArmHelper`. - [x] Identify resource-looking response definitions associated with PUT or PATCH operations; DELETE alone does not establish the prerequisite. - [x] Obtain response identity from direct response references, selecting status 200 or status 201 when 200 is absent. - [x] Require a GET whose selected response resolves to the same resource definition name; a collection wrapper or a different/empty response does not satisfy that lookup. - [x] Exempt concrete polymorphic definitions with a discriminator in their ancestry. - [x] Report the warning on `definitions.<resource>` for each nonexempt missing-GET entry. ## How the Swagger linter works `ArmHelper` builds resource/operation associations from emitted response definitions. The rule compares the names in its PUT/PATCH and GET populations, rather than proving that both operations belong to one native resource lifecycle. Consequently, a converted action payload such as Confluent's `ConfluentAgreementResource` can be selected as a written resource even though it has no separately registered ARM lifecycle. Conversely, a registered ProviderHub read returning an array does not supply the direct response reference required by the validator. These findings are not universally classified as validator false positives. They demonstrate a different semantic boundary, not a reason to reconstruct emission or reference formatting inside a native rule. The migration note records the source-backed categories and their limitations. ## How the migrated TypeSpec linter works Mapping: `AllResourcesMustHaveGetOperation` → `all-resources-must-have-get-operation` (`tsp-lintdiff-local-linter/all-resources-must-have-get-operation`). Supported ARM `getArmResources(program)` metadata supplies the resource population: a registered `createOrUpdate` or `update` lifecycle role requires a registered `read`. A collection list is not a read. The rule retains its discriminator-ancestor exemption through compiler metadata. It emits one warning per offending resource, targeting the `createOrUpdate` operation's concrete interface, otherwise the `update` interface. Only when neither write has an interface does it retain the existing lifecycle/list/action interface and resource-model fallbacks. It does not inspect generated OpenAPI, invoke an emitter, infer identity from reference strings, or depend on TCGC. Native lint observes source declarations; attribution to the retained Swagger's selected latest versions is established separately in the migration evidence. For example, ContainerApps' legacy read templates supply an HTTP GET but not native read lifecycle metadata, while its standard write templates do register writes. This intentionally differs from the executable Swagger rule. Supported standard-template behavior is tested; universal Swagger equivalence and clean supported polymorphic authoring are not claimed. ## Source target repair [Promotion review finding](#5654 (comment)) exposed the same defect in the pinned source: selecting the first lifecycle interface could target `Deletes` instead of `Writes` when standard operation templates are composed on separate interfaces. That made a suppression on the concrete write interface ineffective, while suppression on unrelated deletes hid the warning. The repair prioritizes the semantic write roles instead of registration order. Delete-first create-only/update-only regressions assert exact targets, a separate-interface create/update case proves create priority, and full-linter tests prove write suppression works while delete suppression does not. The predicate, exemptions, message, and one-warning-per-resource unit are unchanged. The new full corpus independently preserves the affected project/message population and all existing source targets; it does not substitute for the supported split-interface regressions. ## Changes and validation - Add native predicate, compliance, exact-target, deduplication, documentation-example, split-interface and full-linter suppression coverage using supported ARM templates. - Preserve the reviewed ambient expectations for the original compliant comparison fixtures. - Apply the explicitly user-authorized narrow baseline correction in `XmsResourceInPutResponse/global-put-ignored`: remove only the stale `put-in-operation-name` expectation and matching diagnostic entry. No unrelated production rule was changed; the complete original fixture group now passes. - Update native authoring guidance and migration evidence, retaining the partial-equivalence and polymorphic coverage limitations. - Required explicit formatting, package build, changed-TypeScript lint, native tests, every affected fixture group, representative/full production corpus, and manifest-based generated-output cleanup pass. Fresh outer-owned independent persistent local precommit review, including same-reviewer follow-up, found no actionable findings on the exact approved source diff. ## Migration evidence See [AllResourcesMustHaveGetOperation migration evidence](https://github.com/Azure/typespec-azure/blob/lintdiff-all-resources-must-have-get-operation/packages/typespec-lintdiff/test/fixtures/AllResourcesMustHaveGetOperation/migration.md) for pinned revisions, fresh full-run counts, complete one-sided project sets, raw and deduplicated identities, selected-version attribution, code-backed outliers, compile-failure exclusions, fixture results, target-regression evidence, and remaining uncertainty. The supplemental noise audit's preexisting official-only invocation limitation remains disclosed there; it is not evidence of local-rule coverage. Generated corpus reports and raw outputs are archived external validation evidence and excluded from this PR. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Yuchao Yan (msyyc)
marked this pull request as ready for review
October 10, 2026 02:43
Yuchao Yan (msyyc)
requested review from
Mark Cowlishaw (markcowl),
Timothee Guerin (timotheeguerin) and
Jeff Fisher (xirzec)
as code owners
October 10, 2026 02:43
Retain direct create and update interface targeting checks instead of replaying compiler suppression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Retain literal rule fixtures and package-isolation regressions. Published examples were independently executed and recorded as one-time session evidence; no continuous Markdown coverage is claimed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Timothee Guerin (timotheeguerin)
approved these changes
Oct 16, 2026
…operation-2026-9-16-10-5-33.md
Yuchao Yan (msyyc)
enabled auto-merge
October 19, 2026 02:40
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 19, 2026
Yuchao Yan (msyyc)
deleted the
promote-lintdiff-all-resources-must-have-get-operation
branch
October 19, 2026 05:17
Yuchao Yan (msyyc)
added a commit
that referenced
this pull request
Oct 19, 2026
## Summary Update five lint development/promotion skills to prevent the recurring review feedback in the supplied 58-comment retrospective across 17 migration PRs. Target `feature/lintdiff-migration-new`, the branch current when this work was requested. | Review theme | Comments | Prevention | | --- | ---: | --- | | Redundant internal change entries | 28 | Inspect and reuse task-owned unreleased feature/fix coverage before creating entries; preserve genuinely needed coverage for other packages. | | Multiple behavior suites for one public rule | 14 | Extend the canonical suite, found by its imported rule object even when its historical filename differs from the public name. | | Repetitive test headings | 13 | Use file-scope cases or meaningful scenario groups, preserving scoped setup rather than repeating the rule/filename. | | Private fixture APIs | 2 | Use supported public imports/decorators; redesign or remove artificial private-only cases without changing production scope. | | Bespoke Markdown parser | 1 | Enforce the existing prohibition without relocating the parser or losing distinct semantic/example evidence. | These are five recurring lessons, not 58 independent design judgments: 55 comments explicitly disclose Copilot generation, and three have no automation disclosure. Original report IDs 37-39 on #5591 and #5654 remain outside the selection. ## Changes - `typespec-lint-validate`: define canonical suite ownership and meaningful grouping; clarify genuine public-entry/package tests versus duplicate rule execution under import guards; require public fixture APIs. - `typespec-lint-implement`: centralize unreleased change-entry reconciliation and include the shared test policies in implementation/final review. - `lintdiff-rule-promote`: replace unconditional creation of a test file/change entry with discovery and reuse; reconcile acceptance-evidence and review instructions with the shared policies. - `develop-lintdiff-rule`: reference the shared policies during test planning and independent review; preserve migration/corpus requirements. - `create-linter-rule`: apply the same guidance to scaffold cleanup, existing-rule extensions, public fixtures, and changelog coverage. ## Evidence and boundaries - [Changelog guidance, report 22](#5502 (comment)) - [Private APIs, report 23](#5502 (comment)) - [One behavior suite per rule, report 36](#5556 (comment)) - [Historical canonical filename, report 48](#5663 (comment)) - [Isolation-suite duplication, report 53](#5665 (comment)) - [Meaningful scenario grouping, report 55](#5673 (comment)) - [Markdown parser removal, report 33](#5551 (comment)) Keep independent utility tests, genuine package/registration evidence, rule-owned diagnostic regressions, and actual published-example validation. Keep separate rulesets-package internal coverage when needed; do not delete all internal entries. Existing production semantics, audience/enablement, source immutability, parity evidence, and required validation remain unchanged. Skills only: no generator, rule implementation/test, package, dependency, release entry, or CI changes. The scaffolder's redundant wrapper is explicitly handled by cleanup guidance; changing the generator is a separate follow-up. ## Validation - Scoped Prettier write/check on the five changed Markdown files, using the repository's scalar formatting settings without optional plugins because the configured TypeSpec plugin has no built output. No dependencies or submodules were initialized for this documentation update. - Relative Markdown file/heading reference checks and full five-file diff review. - `git diff --check` and explicit staged-file scope verification. No package builds, corpus runs, docs regeneration, or repository-wide formatting/source linting are needed for this Markdown-only change. Co-authored-by: Copilot <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.
Original Swagger linter
linter code: AllResourcesMustHaveGetOperation
linter doc: all-resources-must-have-get-operation.md
Original checks:
ArmHelper.definitions.<resource>for each nonexempt missing-GET entry.How the Swagger linter works
The helper associates operations with emitted response definitions, and the rule compares names in its PUT/PATCH and GET populations. This can select converted action payloads that have no separate native ARM lifecycle. It can also reject a registered read whose response is an array or empty body. Confluent's agreement response and ProviderHub's customized read illustrate those two categories.
These are explicit native-contract differences, not a blanket claim that validator findings are false positives. The native rule does not reproduce emitted-reference lookup or encoding.
Source TypeSpec lintdiff rule
AllResourcesMustHaveGetOperation; canonical validator slug:all-resources-must-have-get-operation.all-resources-must-have-get-operation.Azure/typespec-azure:lintdiff-all-resources-must-have-get-operation.c4cd068e8f03fcd770ecc71ced7f596043c2ce77.e39ac7fc2a10668f14e8160b56a69f83d4d1631d.C:\dev\worktrees\lintdiff-all-resources-must-have-get-operation, clean at that commit.The source rule was assumed done for this promotion run. Queue cycle1 repaired the source target defect and completed a fresh one-round Balanced development review (review5468097630 at the exact new source SHA, zero comments and unresolved Copilot threads). The clean source remains immutable throughout this promotion refresh.
The prior promotion finding exposed a shared source defect: first-lifecycle-interface selection could target
Deletesinstead ofWrites, making write-interface suppression ineffective. This refresh consumes only the newly reviewed semantic write-interface priority, ports its exact-target and full-linter suppression regressions, and updates native guidance. Existing naming, registration, fallback adaptation, Chronus entries and disabled rulesets are preserved. No entire source-branch merge or cherry-pick was performed. The historical finding remains unresolved until the reviewed official repair is actually published and verified.Destination analysis
The agent selected
@azure-tools/typespec-azure-resource-managerbecause the rule consumesgetArmResources, ARM lifecycle roles, and ARM resource identity, implements an ARM RPC requirement, and has ARM-only catalog/fixture applicability. The existing generic rules validate present operations or require DELETE; Agent child lifecycle enforcement does not cover generic resources.Azure Core cannot depend on ARM for this check. TCGC is not a plausible owner because this is a resource API policy rather than generated SDK naming, and ARM must not depend on TCGC. The public native name is the concise
resource-missing-read, not the validator slug.How the promoted TypeSpec linter works
The public rule is
@azure-tools/typespec-azure-resource-manager/resource-missing-read.It traverses registered ARM resources and requires a
readlifecycle role whencreateOrUpdateorupdateis present. DELETE-only resources are excluded; a collection list does not substitute for an individual read. It retains the compiler-metadata discriminator-ancestor exemption and reports once per resource with the original warning text. It targets thecreateOrUpdateoperation interface first, otherwise theupdateinterface, before the preserved fallback search. A separate delete interface is not selected ahead of a concrete write.Behavior-preserving destination adaptations:
resource.jsandoperations.jsimports.utils.getInterfacelifecycle fallback and the source's subsequent list/action/model fallbacks.Programtype; add the official name, documentation reference, URL, and linter registration.Testerlifecycle. Validate the actual published incorrect/correct snippets using a namedWidgetPropertiescustomization of standard ARM templates.There is no emitter, OpenAPI, TCGC, route-guessing, or reference-string dependency in the rule. It observes source metadata, not emitted version snapshots. The reviewed source's selected-version attribution and material parity limitations remain unchanged. A clean supported polymorphic ARM fixture remains unestablished; universal executable Swagger equivalence is not claimed.
The resource-manager ruleset explicitly lists the new rule as
false. This promotion makes the rule available without enabling new diagnostics in existing service specs. The ARM package has afeatureChronus entry; disabled-only ruleset registration has a separateinternalentry.Fixture-to-native test mapping
Snapshots were not copied. Each relevant source fixture is reduced to direct native authoring and rule-specific diagnostics.
it("reports a missing read for a create operation on its interface")it("requires a read for a nested resource independently of its parent")it("reports a missing read for an update-only resource")it("accepts a read alongside create and update operations")toBeValid().it("does not require a read for a delete-only resource")it("reports once when both create and update lack a read")it("does not treat a collection list as a resource read")it("reports each resource once when writes share an interface")it("reports on the incorrect documentation example")it("accepts the correct documentation example")| N/A (cycle1 reviewed source target regression) |
it("targets create writes when delete is registered first")| Create-only exact write target. || N/A (cycle1 reviewed source target regression) |
it("targets update writes when delete is registered first")| Update-only exact write target. || N/A (cycle1 reviewed source target regression) |
it("targets shared writes when delete is registered first")| Both writes produce one diagnostic on Writes, not Deletes. || N/A (cycle1 reviewed source target regression) |
it("prefers createOrUpdate over update on separate interfaces")| Create takes priority even when update is registered first. || N/A (cycle1 reviewed source target regression) |
it("applies create suppression to the write interface, not delete")| Full native linter: no suppression warns once, Writes suppression removes it, Deletes suppression does not; complete diagnostics asserted. || N/A (cycle1 reviewed source target regression) |
it("applies update suppression to the write interface, not delete")| Same full-linter controls for update-only supported authoring. |Migration evidence
See the reviewed source migration note for the pinned corpus, latest full-run counts, complete one-sided project sets, conservative deduplication, selected-version attribution, code-backed outliers, six compile-failure exclusions, strict fixture evidence, and remaining limitations.
Validation
Required native validation passed: prepared dependency/tool fingerprints and outputs reverified without replaying setup; invalidated ARM package output rebuilt; focused sixteen-case native suite; ARM package build/lint and complete package tests (490 tests in 67 files); rulesets build/lint and complete package tests (five tests in one file); supported plain Chronus status and two LF changes; required ARM docs regeneration; explicit empty-ignore formatting/checks of the authored rule, generated README, linter reference, generated rule page, and maintained ARM inventory; final changed-code formatting and diff hygiene. Both complete native suites used the checked-in profiles without timeout or concurrency overrides.
Fresh outer-owned independent persistent local precommit review of the complete ten-file promotion diff and three incremental files completed with no actionable findings. Actual same-reviewer follow-up was verified, and the parent independently verified the approved content, generated-page and validation identities before publication. The reviewed incremental repair is now published; the historical finding remains open for evaluation by the fresh outer promotion review pair. The promotion is based on canonical main
f4d313479a5d3edf12374845500cf943eb1fa534and the exact clean source commit stated above.Validation limitations
Docs regeneration completed with exit 0 and preexisting missing-documentation warnings for unrelated ARM diagnostic definitions. The new rule's authored and generated pages are present and validated. The generated website rule page is ignored by repository policy; it was inspected and formatted but is not force-added.
Repository-wide
validate:prand local website builds were not run: the required affected-package plan covers this isolated promotion, and CI's dedicated Website job is authoritative for Astro. No full-repository success is claimed. The source migration note separately discloses its official-only noise-audit limitation and excluded corpus projects; those do not establish universal Swagger equivalence.Promotion sync policy
The lintdiff source stays immutable during promotion. A confirmed source-semantic defect blocks this promotion and returns to the owning queue's bounded source-repair flow. This PR may be refreshed only from the exact newly pushed source commit after clean development review; source repair is not performed in the promoted copy. Destination-only adaptations are recorded and validated here.
heavy
Review follow-up: documentation-test scope (2026-10-16)
Removed the bespoke Markdown-parsing test in ee03812, without moving or replacing the parser. All 12 retained literal tests pass, preserving the rule-owned positive/negative and exact authored-target controls. The dependency-closure build and repository format/lint passed.
The actual published incorrect/correct snippets were separately compiled once, with exactly one expected warning versus zero diagnostics and hash/target receipts. Literal tests do not continuously validate Markdown. This supersedes historical description claims about direct Markdown coverage; a standard documentation-testing system is outside this promotion.
The previously described suppression matrix had already been removed in 4e68e82 before this fix cycle; all 12 current literal cases and direct target assertions remain. No nonexistent suppression coverage is claimed.