[Swagger Linter Migration] GuidUsage - #5336
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.
|
📦 Package size report2 packages changed size, +9.14 KB (+0.1%) packed overall.
11 package(s) with no notable change
Packed = gzipped |
commit: |
|
You can try these changes here
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cf125def-85b2-462d-8902-7ac7beb588a3
| ); | ||
| } | ||
| }, | ||
| root: () => { |
There was a problem hiding this comment.
why do you need to make your own traverse here? can't you just use the linter engine, this seems like this should be a pretty simple rule
There was a problem hiding this comment.
The extra traversal is intentional because this rule is trying to cover the resolved HTTP projection of the API while reporting on actionable authored TypeSpec targets. The regular linter traversal handles ordinary declarations, but it does not fully cover generated ARM HTTP shapes with a useful diagnostic location.
For example:
model Widget is TrackedResource<WidgetProperties> {
...ResourceNameParameter<
Resource = Widget,
KeyName = "widgetName",
SegmentName = "widgets",
Type = Azure.Core.uuid
>;
}
@armResourceOperations
interface Widgets {
get is ArmResourceRead<Widget>;
}This emits an HTTP path parameter with format: uuid, which the Swagger GuidUsage rule reports. The corresponding widgetName model property is generated from an ARM library template and is library-owned, so reporting from a basic modelProperty listener would point into code the service author cannot meaningfully fix or suppress. The HTTP/resource traversal recognizes it as the resource key and maps the finding back to the authored Widgets.get operation.
The same projection traversal covers direct ArmResponse<uuid> bodies and formats applied to generated HTTP parameters. It also lets us exclude imported library-owned findings and deduplicate repeated emitted occurrences. These cases came from the lintdiff corpus investigation and have focused tests in this PR.
Some of the recursive type inspection may still be refactorable, but replacing the resolved HTTP traversal with only semantic listeners would narrow the migrated rule's established coverage or change its diagnostic targets.
There was a problem hiding this comment.
hhm really don't like that by that reasoning every single rule should do their own traversal due to the same limitation. I think if this is critical and can't be hacked in the diagnostic target resolution function right now we need to figure out a better way built in for this.
There was a problem hiding this comment.
Can't find a better solution. Let us hold on this PR for now and see if other PRs have similar request for traversal or we could find other solution. CC catalinaperalta
There was a problem hiding this comment.
can you not just check modelProperty type(and any other types you want to check) and report on the model property? The typespec engine should report template instantiation trace if it happens in a template
There was a problem hiding this comment.
Timothee Guerin (@timotheeguerin) Yes, this works for the ordinary cases, and I refactored the rule in afd2b72 to use modelProperty and operation listeners for authored properties, parameters, headers, direct return types, containers, unions, and custom scalars. The resolved HTTP traversal for ordinary parameters, request/response bodies, and headers has been removed.
There is one ARM-specific exception. An instantiated ResourceNameParameter.name property does carry a template mapper, so the compiler can produce an instantiation trace. However, its primary source location remains the library declaration in Azure.ResourceManager.ResourceNameParameter. The linter filters library-targeted diagnostics before presenting related instantiation locations, so reporting that generated property directly produces no user-visible diagnostic.
This is not just theoretical in the migration corpus: there are 9 UUID resource-name declarations across 6 projects (8 across 5 successfully compiled projects; Quota was excluded by its compile failure), producing 35 Swagger operation-parameter findings. Dropping the case would therefore create a known gap in 5 of the 46 projects detected by the final TypeSpec corpus run. (detailed report is here: https://github.com/Azure/typespec-azure/blob/feature/lintdiff-migration-new/packages/typespec-lintdiff/test/fixtures/GuidUsage/migration.md)
As a compromise, the only remaining HTTP projection logic now finds resolved ARM resource-key parameters and maps those library-owned generated parameters to their authored operations. Everything else uses the standard linter traversal. I also added coverage for ArmResponse<Azure.Core.uuid> to ensure a UUID hidden behind a library response wrapper remains detected without recursively reporting project response models. The implementation is reduced by 184 lines, and all 16 focused tests still pass.
There was a problem hiding this comment.
🟡 Changes recommended
The rule can misclassify imported library declarations and misses unreferenced named scalar and union UUID schemas.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds the ARM no-uuid linter rule migrated from Swagger GuidUsage.
Changes:
- Implements UUID detection across ARM schemas and HTTP payloads.
- Adds rule tests and documentation.
- Registers the rule disabled by default and records the feature release.
File summaries
| File | Description |
|---|---|
no-uuid.ts |
Implements UUID traversal and diagnostics. |
no-uuid.test.ts |
Tests supported UUID shapes. |
no-uuid.md |
Documents rationale and examples. |
linter.ts |
Registers the rule. |
resource-manager.ts |
Adds the disabled ruleset entry. |
README.md |
Lists the new rule. |
linter.md |
Updates generated website reference. |
| Chronus change | Records affected packages and feature. |
Review details
Suppressed comments (1)
packages/typespec-azure-resource-manager/src/rules/no-uuid.md:37
- The LintDiff equivalent should deep-link to the canonical GuidUsage entry (R3017) in the OpenAPI automated-guidelines document rather than the legacy validator documentation.
This rule corresponds to the Swagger validator rule [GuidUsage](https://github.com/Azure/azure-openapi-validator/blob/6243cb01c16c7535cd3b8df6f45fbeb3c095ed7f/docs/guid-usage.md).
- Files reviewed: 8/8 changed files
- Comments generated: 5
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…romote-guid-usage-to-arm
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Original Swagger linter
The original rule performs these checks:
formatand value isuuid.format: uuidnode with Azure API review guidance.How the Swagger linter works
The Spectral rule runs the unresolved JSONPath
$..[?(@property === 'format' && @ === 'uuid')]and appliesfalsy. It has no authored-shape exemptions and does not resolve references before traversal. Its diagnostic location is the emitted format node, so one TypeSpec declaration may be reported repeatedly when it is expanded across paths, operations, schemas, visibility variants, or API versions.That representation also produces findings which should not be copied into a native TypeSpec rule: imported
Azure.Core.RequestIdResponseHeaderexpansions are library-owned, and retained Swagger can contain staleAzure.Core.uuiddefinitions absent from the current authored TypeSpec surface. The native rule therefore diagnoses actionable authored targets and deduplicates them rather than matching emitted occurrence counts.Source TypeSpec lintdiff rule
GuidUsagetsp-lintdiff-local-linter/guid-usageguid-usageguid-usage.tsfeature/lintdiff-guid-usagecf3f5db38bf3a540c929a1d2ba9b7fea1638d7f7C:\dev\worktrees\lintdiff-guid-usageThe user marked this source rule done for promotion. The lintdiff source, fixtures, snapshots, manifests, and documentation were not modified during promotion.
Destination analysis
The promoted rule belongs in
@azure-tools/typespec-azure-resource-manager:applicability: ARMandsources: ["arm"]resource-manager@azure-tools/typespec-azure-corewas considered because itsno-formatrule recommendsAzure.Core.uuidinstead of@format("uuid"). It is not equivalent:no-formatgoverns decorator usage across Azure APIs, while this rule prohibits semantic UUID wire shapes throughout ARM APIs, including UUID scalar aliases and template-generated parameters. Placing the rule in core would also require removing its ARM-only resource and provider semantics.The official rule is named
no-uuid, following the TypeSpec convention for a banned construct rather than carrying the validator slug into the user-facing rule name.How the promoted TypeSpec linter works
@azure-tools/typespec-azure-resource-manager/no-uuid:getAllHttpServicessourcePropertychainsModelPropertyorOperationCurrent
mainretains a library-owned source property for generated resource-key parameters. The official-package adaptation therefore prioritizes the resource-key operation fallback before the generic source-property lookup. This preserves the lintdiff rule's intended authored-operation diagnostic target for both typed and formattedResourceNameParameterinstantiations.The rule is available but registered as
falsein the resource-manager ruleset so promotion does not introduce diagnostics into existing Azure service specs.Fixture-to-native test mapping
uuid-propertyit("reports a UUID-typed model property")uuid-query-parameterit("reports a UUID-typed query parameter")uuid-template-parameterit("reports a UUID-typed resource name template parameter")uuid-template-format-parameterit("reports a UUID-formatted resource name template parameter")uuid-bodyit("reports a direct UUID request body")uuid-responseit("reports a direct UUID response body")uuid-custom-scalarit("reports a custom scalar derived from UUID")uuid-format-propertyit("reports a property-level UUID format")uuid-array-propertyit("reports an array property containing UUID values")non-uuid-shapesit("allows non-UUID shapes and UUIDs in client-only namespaces")unreachable-uuid-propertyit("reports a UUID property on an unreferenced named model")it("reports UUID values nested in records, tuples, and unions")it("reports a UUID-typed response header")it("deduplicates a property reached through declarations and HTTP payloads")Migration evidence
See
GuidUsage/migration.mdfor the declared focused tests, real-service comparison, latest full-corpus counts, one-sided project explanations, compile failures, and remaining uncertainty.Validation
no-uuidtest: 14 passedpnpm format && pnpm lintgit diff --checkValidation blocker
pnpm validate:prreported that the already-tested branch was behind a newly advancedorigin/main, then produced no further progress for five minutes. It was stopped under the bounded validation policy. The narrower native validations listed above completed successfully.Promotion sync policy
Any newly discovered semantic gap should block this promotion until the user explicitly reopens lintdiff repair. The done lintdiff source is not modified as part of promotion.
NOTE: heavy