[Swagger Linter Migration] LatestVersionOfCommonTypesMustBeUsed - #5271
[Swagger Linter Migration] LatestVersionOfCommonTypesMustBeUsed#5271Yuchao Yan (msyyc) wants to merge 6 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
|
📦 Package size report2 packages changed size, +2.85 KB (+0.0%) packed overall.
11 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
catalinaperalta
left a comment
There was a problem hiding this comment.
looks good, just some comments to improve clarity
| return { | ||
| root: (program) => { | ||
| const latestVersion = getLatestArmCommonTypesVersion(program); | ||
| if (latestVersion === undefined) { |
There was a problem hiding this comment.
Only if common-types metadata is not initialized or is empty. I renamed the helper to tryGetLatestArmCommonTypesVersion to make that defensive path explicit.
| currentVersion, | ||
| latestVersion, | ||
| ) || | ||
| currentVersion !== latestVersion |
There was a problem hiding this comment.
I believe this || currentVersion !== latestVersion is already considered in the `reportIfOutdated method since any case where currentVersion is not undefined or equal to the latest version produces a diagnostic
There was a problem hiding this comment.
Good catch. Removed the redundant check and now handle the undefined case explicitly after reporting outdated versions.
| continue; | ||
| } | ||
|
|
||
| const projected = unsafe_mutateSubgraphWithNamespace( |
There was a problem hiding this comment.
Just wondering if it's ok to use this function Mark Cowlishaw (@markcowl) or there's a better function to use?
There was a problem hiding this comment.
Context: we need this projection step because the rule walks HTTP operations and payload types for each effective API version. Versioning decorators like @added, @removed, and @typeChangedFrom don't rewrite the original service graph, so walking the unprojected namespace could report legacy common types for the wrong API version.
getVersioningMutators(...) gives us the per-version mutator, and unsafe_mutateSubgraphWithNamespace(...) applies it to produce the same kind of version-specific namespace used by the OpenAPI3 and Autorest emitters before generating per-version output. I don't see a stable replacement API in this repo right now, so keeping this matches the existing emitter pattern.
There was a problem hiding this comment.
yeah we definitely do not want to start mutating everything here this is extremely costly
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9f3bdb44-9302-42a3-9b45-ea02a4e8f0a6
3 participants
Original Swagger linter
Original checks:
$refvalue viagiven: "$..['$ref']"in the ARM ruleset.nullrefs and refs that do not match/common-types/resource-management/v\d+/\w+.json#.$refpath.isLatestCommonTypesVersionForFileandLATEST_VERSION_BY_COMMON_TYPES_FILENAMEto decide whether that file is referenced through the latest known common-types version.$refpath when the referenced version differs from the latest map entry.How the Swagger linter works
The Swagger validator is a Spectral function over emitted OpenAPI 2.0 documents. It does not understand TypeSpec symbols, ARM service namespaces,
@armCommonTypesVersion, or version projections. Instead, it scans each emitted$refstring, filters to ARM common-types references under/common-types/resource-management/v*/, extracts{version, file}, and compares the extracted version with a hard-coded filename-to-latest-version map.That string-level traversal means Swagger diagnostic locations are emitted
$refpaths and raw diagnostic cardinality is tied to every repeated emitted occurrence. It also means the validator can be stale when the hard-coded map is stale. The lintdiff evidence records known stale-map false positives for valid v6networksecurityperimeter.jsonandmanagedidentitywithdelegation.jsonreferences that the validator still maps to v5; the promoted rule intentionally does not copy those stale data defects.Source TypeSpec lintdiff rule
Source lintdiff rule id:
LatestVersionOfCommonTypesMustBeUsedLocal rule name:
latest-version-of-common-types-must-be-usedOfficial TypeSpec rule name:
use-latest-version-of-common-types(renamed during promotion to follow TypeSpec linter naming conventions)Source branch:
feature/lintdiff-latest-version-common-typesSource commit inspected:
be310817b95a8c2fba64b7906e23624ec38efab0Source worktree:
C:\dev\worktrees\lintdiff-latest-version-common-types-sourceSource rule: https://github.com/Azure/typespec-azure/blob/feature/lintdiff-latest-version-common-types/packages/typespec-lintdiff/src/rules/latest-version-of-common-types-must-be-used.ts
The source worktree was clean, with no uncommitted rule changes. The user-marked done lintdiff source rule was not modified during promotion.
Destination analysis
Selected package:
@azure-tools/typespec-azure-resource-manager.@azure-tools/typespec-azure-corewas considered but rejected because this rule is ARM-specific and depends on ARM-only concepts and helpers. The validator metadata marks applicability as ARM with sourcearm; the fixture rule docs say it applies to Resource Manager; the implementation checks ARM provider namespaces, reads effective@armCommonTypesVersion, resolves ARM common type references, and imports ARM helpers such asgetArmCommonTypesVersion,getArmCommonTypesVersions,isArmCommonType, andgetArmCommonTypeOpenAPIRef. Moving this to core would require rewriting away ARM package dependencies and would violate the dependency direction that core must not depend on resource-manager.The existing official ARM
arm-common-types-versionrule is related but not equivalent: it requires specifying a common-types version, while this promoted rule requires selecting the latest common-types version and catches latest-version APIs that still emit legacy common-type symbols.The rule is explicitly listed in
@azure-tools/typespec-azure-rulesets/resource-managerbut disabled by default for staged rollout. Existing Azure service specs and repo samples still select older ARM common-types versions, so enabling it immediately would convert those existing warnings into CI failures. Teams can opt in directly, and the default ruleset can be flipped after specs are updated or intentionally suppressed.How the promoted TypeSpec linter works
The promoted rule runs once at program root and visits TypeSpec services registered as ARM provider namespaces. It discovers the latest available ARM common-types version from
Azure.ResourceManager.CommonTypes.Versions, compares the effective@armCommonTypesVersionon the service namespace or each version enum member, and reports older selections on the namespace or enum-member target.When an API version already selects the latest common-types version, it projects versioned services with the same versioning mutators used by the emitter, resolves HTTP operations from the projected service, and walks operation parameters, request bodies, and response bodies. It uses HTTP metadata visibility (
resolveRequestVisibility,Visibility.Read, andcreateMetadataInfo().isPayloadProperty) so properties excluded from the emitted payload are not false positives. For each reachable ARM common type, it asksgetArmCommonTypeOpenAPIRefwhich common-types file would be emitted for that service/version and warns when that resolved reference is older than the latest version.Diagnostics are deduplicated per target plus resolved reference identity, so repeated traversal of the same target does not spam while two distinct operations producing the same legacy reference each get actionable diagnostics. This intentionally differs from the Swagger validator's emitted-
$refoccurrence count while preserving project-level behavior.Fixture-to-native test mapping
Source fixtures are linked from the lintdiff source branch and converted into native
vitestcoverage inpackages/typespec-azure-resource-manager/test/rules/use-latest-version-of-common-types.test.ts:v3.v6.v3.v6.LocationParameterresolving tov5/types.json.v4/managedidentity.json.The native tests use direct TypeSpec snippets and expected diagnostics rather than copying lintdiff snapshots.
Migration evidence
Detailed migration evidence is in the source rule's migration report: https://github.com/Azure/typespec-azure/blob/feature/lintdiff-latest-version-common-types/packages/typespec-lintdiff/test/fixtures/LatestVersionOfCommonTypesMustBeUsed/migration.md
That report records the focused fixture tests, full-corpus comparison, one-sided project explanations, known validator stale-map behavior, compile failures, and remaining uncertainty. In short: after aligning selected API population and excluding known stale validator-map false positives, both sides fire in the same 384 successfully compiled projects, with raw diagnostic counts intentionally different because Swagger reports emitted
$refoccurrences while TypeSpec reports semantic source usages and version selections.Validation
pnpm --filter @azure-tools/typespec-azure-resource-manager exec vitest run test/rules/use-latest-version-of-common-types.test.tspnpm -r --filter "@azure-tools/typespec-azure-resource-manager..." buildpnpm --filter @azure-tools/typespec-azure-resource-manager buildpnpm --filter @azure-tools/typespec-azure-resource-manager lintpnpm --filter @azure-tools/typespec-azure-resource-manager regen-docspnpm --filter @azure-tools/typespec-azure-rulesets buildpnpm --filter @azure-tools/typespec-azure-rulesets testpnpm --filter @azure-tools/typespec-azure-playground-website exec vitest run test/validate-samples.test.tspnpm --filter @azure-tools/typespec-samples exec vitest run test/samples.test.ts -t "legacy/legacy-operations"pnpm --filter @azure-tools/typespec-azure-resource-manager testgit diff --checkValidation blocker
pnpm validate:prwas attempted on the final branch but hung after printing onlyBranch is up to datefor more than 13 minutes. The process was idle and was stopped. The narrower promotion validations above completed successfully.Promotion sync policy
If review finds a semantic gap after promotion, this PR should be blocked until the user explicitly reopens lintdiff rule repair. The done lintdiff source rule was treated as immutable during promotion and was not edited here.