Repository navigation
[Swagger Linter Migration] LatestVersionOfCommonTypesMustBeUsed -> arm-common-types-version - #5271
Yuchao Yan (msyyc) wants to merge 32 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
All changed packages have been documented.
Show changes
|
📦 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
|
Keep the promoted ARM rule available while avoiding immediate failures in existing samples and azure-rest-api-specs projects that still select older common-types versions.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8ed00e6d-dd0a-40f7-8871-ee32f0f371fb
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8ed00e6d-dd0a-40f7-8871-ee32f0f371fb
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8ed00e6d-dd0a-40f7-8871-ee32f0f371fb
catalinaperalta
left a comment
There was a problem hiding this comment.
looks good, just some comments to improve clarity
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9f3bdb44-9302-42a3-9b45-ea02a4e8f0a6
Replace per-version whole-program projection with dependency-aware version metadata traversal and cover availability and historical type changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bb0b0075-253e-4e28-a073-fc5fd64708b1
Move current samples to v6 and regenerate reference snapshots. Preserve grandfathered and version-compatibility schemas with explicit scoped suppressions. Keep the consolidated lint rule enabled. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f0af92c5-fa9d-4992-bb89-31bab99d244b
|
CI follow-up in db29d80:
External Integration is a separate, still-unresolved rollout dependency: existing azure-rest-api-specs services select v3-v5 and now fail under warnings-as-errors (see https://github.com/Azure/typespec-azure/actions/runs/35491909140). The user explicitly chose to fix repository samples only and defer external-spec compatibility changes. This commit does not claim to resolve that check; downstream compatibility updates or scoped suppressions must land before it can pass. |
Copy canonical v6 schemas into the sample reference directory and validate local OpenAPI files and JSON pointers transitively in sample tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f0af92c5-fa9d-4992-bb89-31bab99d244b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f0af92c5-fa9d-4992-bb89-31bab99d244b
Apply the test responsibility guidance from Azure#5572; retain distinct predicate, target, and custom guard regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Hi @Yuchao Yan (@msyyc). Your PR has had no update for 14 days and it is marked as a stale PR. If it is not updated within 14 days, the PR will automatically be closed. If you want to refresh the PR, please remove the |
Preserve main's expanded RPC guideline inventory and the PR's latest common-types requirement. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f0af92c5-fa9d-4992-bb89-31bab99d244b
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merge Azure/typespec-azure main c13ceb4; retain both the migration-specific registration checks and main's opt-in redundant-summary check. No additional rule-policy changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| @@ -0,0 +1,54 @@ | |||
| { | |||
There was a problem hiding this comment.
All these files are copied from specs repo? How do we keep consistence? Why do we need these files here?
There was a problem hiding this comment.
there is a CI check/script that syncs them but please have this change as a separate pr, this is not related to a new linting rule
There was a problem hiding this comment.
Yes—the seven files match packages/samples/common-types/existing/v6 byte for byte. They let emitted sample $refs resolve; the rule itself reads native metadata. Prerequisite #5787 now synchronizes both v6 locations and checks byte equality. The sample delta here still awaits that prerequisite landing.
Timothee Guerin (@timotheeguerin) after 5787 merged, this PR could be simplified.
Separate the sample prerequisite from the ARM rule release note. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace ignored diagnostic target properties with native source markers and supported position checks without changing semantic coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Add concise review guidance for **all linter PRs** to the existing `.github/skills/code-review/SKILL.md`: - Prefer supported APIs and focused changes; keep rule behavior and enablement consistent. - Keep tests lean and scoped to rule-owned behavior, using representative cases and distinct boundaries without redundant permutations or unrelated assertions. - Write author-facing documentation and minimal incorrect/correct examples; validate the published excerpts. - Instruct reviewing agents to check for redundant changelog entries and ensure entries describe the final changes, not revision history or intermediate review cleanup. The guidance applies to all linter PRs. Changelog guidance is directed at reviewing agents; general entry-creation instructions are not repeated. The existing review skill is preserved, and `.github/copilot-instructions.md` is restored to its pre-PR content. ## Review evidence Representative feedback: [test ownership](#5294 (comment)), [representative coverage](#5294 (comment)), [author-facing docs](#5673 (comment)), [focused examples](#5271 (comment)), and [changelog noise](#5502 (comment)). ## Validation Formatting, final-file Prettier check, root lint, and whitespace checks passed. Unrelated formatter-only output was excluded. Verified that the existing review skill is preserved, the moved authoring-guide link resolves, and general Copilot instructions are restored to their pre-PR content. Documentation-only; no package release entry required. Source branch: `Azure/typespec-azure:docs/linter-review-instructions`; target: `main`. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>
Test-target follow-up (October 21, 2026)
Replaced ignored diagnostic
targetexpectations with native source markers and supported position assertions in 6f682caa3. All 16 canonical cases remain, including namespace and per-API-version targets. The dependency-closure build, focused changed-test typecheck, root formatting/lint, and diff hygiene passed. The full test project retains 37 baseline diagnostics (39 before this correction); it is not claimed clean. Final-head CI was pending and was not awaited.This test-only update does not complete the sample split or remove the #5783 prerequisite described below. Earlier test counts describe their historical revisions.
Valid-only follow-up and sample prerequisite (October 21, 2026)
Published focused namespace-selection guidance and a rule-only release note in 94ec55905. The actual final snippets were checked once in explicit public ARM context: missing selection warns, v3 warns, and v6 has zero diagnostics. The 16 canonical rule tests, generated docs, root format/lint and diff hygiene passed. This is not standalone or continuous documentation coverage.
The sample upgrades, emitted snapshots, v6 fixtures, local-reference validation and explicit fixture synchronization are published separately in prerequisite #5783, validated at its own main/compiler revisions (34 sample/reference tests passed). Legacy/version-compatibility suppressions remain tied to this rule's enabled-warning change. The sample delta is still present in this PR until #5783 lands and main can be incorporated; the split is not yet complete, and this PR is not declared ready. No merge is performed by this workflow.
Test-responsibility cleanup
Applied the guidance merged in #5572 from the #5507 review. Removed framework suppression and imported-library exclusion tests while retaining registry selection, overrides, authored targets, and the rule-owned ARM-service guard. No production code changed.
Current cleanup validation:
mise exec -- pnpm --filter @azure-tools/typespec-azure-resource-manager exec vitest run test\rules\arm-common-types-version.test.tspassed 17 tests. Repositorypnpm format,pnpm lint, andgit diff --checkpassed. Earlier validation counts below describe historical revisions, not this cleanup; the full test suite was not rerun.Summary
Extend the existing
@azure-tools/typespec-azure-resource-manager/arm-common-types-versionrule to require the latest effective ARM common-types selection. Following Mark and Timothée's review, this PR no longer adds a separateuse-latest-version-of-common-typesrule or traverses HTTP payloads and legacy type usages.Intentional behavior change:
arm-common-types-versionis already enabled in the resource-manager ruleset. Its new latest-version warnings therefore apply wherever that existing rule is enabled; they are not opt-in. Services selecting older versions must update@armCommonTypesVersionor intentionally suppress the existing rule for compatibility. Existing suppressions cover both missing-configuration and outdated-selection diagnostics.Current implementation
Consolidation commit:
1722f99264f78dea42a5f084ca936a7b77161b47.context.reportDiagnostic, preserving rule suppression and project-location filtering.Deliberately narrower migration scope
This is version-selection guidance, not full behavioral equivalence with the Swagger/lintdiff reference checker. A service selecting the latest version can still use a legacy symbol that resolves to an older common-type record. The consolidated rule intentionally does not detect that condition, inspect payload visibility or historical types, or validate common-type resolution. Existing legacy/deprecation warnings overlap with this concern but do not cover every former traversal case. The emitter's independent validation is unchanged.
The user explicitly approved this consolidation and the enabled-ruleset behavior change on September 20. The immutable lintdiff source is not being repaired or rewritten.
Original Swagger linter
LatestVersionOfCommonTypesMustBeUsed.The Swagger rule scans emitted
$refstrings matching ARM common-types paths and compares their file/version against a filename-to-latest-version map. It reports at each emitted reference, unlike the consolidated TypeSpec rule's namespace/API-version targets. The historical lintdiff report records stale-map false positives for valid v6networksecurityperimeter.jsonandmanagedidentitywithdelegation.jsonreferences; those maps are not copied here.Source provenance and destination
LatestVersionOfCommonTypesMustBeUsed.latest-version-of-common-types-must-be-used.be310817b95a8c2fba64b7906e23624ec38efab0.feature/lintdiff-latest-version-common-types, restored on September 15 at that exact SHA without changing source contents.C:\dev\worktrees\lintdiff-latest-version-common-types-source, recorded as clean during promotion.Destination:
@azure-tools/typespec-azure-resource-manager, consolidated into its existingarm-common-types-versionrule. This behavior depends on ARM provider namespaces and common-types metadata; placing it in azure-core would reverse the package dependency direction. The previous unpublished promotion name,use-latest-version-of-common-types, is removed rather than retained as an alias.Fixture-to-native test mapping
Native tests now live in
packages/typespec-azure-resource-manager/test/rules/arm-common-types-version.test.ts. Fixture links below use the unchanged restored source ref. Removed coverage is marked explicitly rather than presented as equivalent.reports an older namespace selection for each API versiondoes not emit when the service namespace has a common type version without version on enum valuesreports an older enum-member override of a latest namespace selectionaccepts a latest enum-member override of an older namespace selectionchecks only the selected version, not legacy common-type usagesemits diagnostic when a version in the enum is missing a common type versionemits diagnostic when unversioned service namespace is missing a common type versiondoes not emit when the service version enum has a common type version on all enum valuesdoes not emit when an unversioned service selects the latest versionuses the latest version from the common-types registryreports an older unversioned namespace selection: %schecks explicit per-version selections without a namespace selectionsupports string version selectionschecks each ARM service independentlydoes not check non-ARM services or Azure library namespacesenables consolidated ARM common-types version guidanceThe former projection, dependency-history, metadata-layout, and resolution-error regressions were removed with their implementation. They are not claimed as coverage of the consolidated selection-only rule.
Validation
Consolidation follow-up (September 20)
git diff --checkpassed.pnpm validate:pr --skip-build --skip-test --verbosecompleted successfully, with a warning that the branch is behind main and explicit skips for full build/test/regen-docs; affected-package builds, tests, and docs regeneration were run separately. Its committed-diff checks ran before this consolidation commit, so they are not presented as a final-head diff audit.int:azure-specslabel triggered External Integration for the new head: run 35491909140, currently pending. The previous head's successful integration is not evidence for the new enabled warnings.Historical context
Earlier follow-ups replaced whole-service mutation with direct version metadata, fixed reference-resolution reporting through the linter, restored source links, and added migration/inventory documentation. Those changes and their historical validation remain in the commit/review history; traversal-related implementations and tests are superseded by this consolidation. The pinned source report and its corpus results are unchanged.
The original promotion's
pnpm validate:prattempt hung and was stopped; that historical limitation is distinct from the September 20 successful run with the explicit skips described above.Promotion sync policy
The source lintdiff rule and migration report remain immutable at the pinned SHA. The September 20 user-approved consolidation is an explicit destination-scope change, not a source repair or a claim of complete migration parity. Any future repair of the source rule still requires explicitly reopening that work.
Review cleanup (October 19, 2026)
Removed the synthetic future-version fixture that invoked
CommonTypes.Private.armCommonTypesVersions; public defaults and explicit version-selection controls remain. Required validation exposed a pre-existing merge inconsistency: the lockfile/catalog came from the main merge parent but the compiler pin was retained from the other parent. Restored onlycoreto the exact already-merged main-parent revision (not current main), with byte-identical lockfile/catalog and no dependency-manifest edits. Frozen ARM-closure installation and the dependency-closure build now pass. The earlier full-workspace install subsequently stopped in an unrelated local Python prepare step; a full-workspace install is not claimed to pass.Published in d22e03b. Fresh focused validation: 16 tests passed, package production typecheck, root
pnpm format, rootpnpm lint, and diff hygiene passed. The full test TypeScript project retains pre-existing diagnostics (19 before, 18 after); the baseline test-file comparison found no new diagnostics, accounting explicitly for moved tests and any removed 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.