[Swagger Linter Migration] ParametersSchemaAsTypeObject - #5361
[Swagger Linter Migration] ParametersSchemaAsTypeObject#5361Yuchao Yan (msyyc) wants to merge 12 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…hema-as-type-object-to-arm
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 report✅ No notable package size changes compared to the base branch. 13 package(s) with no notable change
Packed = gzipped |
commit: |
|
You can try these changes here
|
There was a problem hiding this comment.
🟡 Changes recommended
Required suppression guidance and the manually maintained ARM rules table entry are missing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Promotes the Swagger request-body schema check into the ARM TypeSpec linter.
Changes:
- Adds and registers
request-body-must-be-object. - Covers AutoRest schema-emission behavior with extensive tests.
- Updates rulesets, references, and release metadata.
File summaries
| File | Description |
|---|---|
.chronus/changes/promote-parameters-schema-as-type-object-2026-08-31.md |
Records the feature release. |
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/request-body-must-be-object.md |
Documents rule behavior. |
packages/typespec-azure-resource-manager/src/rules/request-body-must-be-object.ts |
Implements schema classification. |
packages/typespec-azure-resource-manager/test/rules/request-body-must-be-object.test.ts |
Tests supported body forms. |
packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts |
Adds the disabled ruleset entry. |
website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md |
Updates website rule references. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- 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>
There was a problem hiding this comment.
🔵 Needs a closer look
The implementation self-imports its package root, creating a circular dependency and coupling source builds to generated output.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
packages/typespec-azure-resource-manager/src/rules/request-body-must-be-object.ts:2
- This self-import resolves through the package's public
dist/src/index.*export and creates anindex -> linter -> rule -> indexcycle. It can also bind source builds/tests to stale generated output instead of the current implementation. Use the direct intra-package module import, consistent with the rest of this package.
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Timothee Guerin (@timotheeguerin) updated with your comments and pls take a review again. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| } | ||
| const targetFormat = isSecret(program, target) ? "password" : undefined; | ||
| const encodedSchema = getEmittedScalarSchema(program, encoding.type); | ||
| const mergedFormat = mergeFormatAndEncoding( |
There was a problem hiding this comment.
why do we need this whole format thing
There was a problem hiding this comment.
This is not handling authored @format; that support was removed. It mirrors AutoRest's internal @encode behavior because encoding only replaces the schema type when mergeFormatAndEncoding() produces a format.
For example:
model Payload {
value: string;
}
model Request {
@encode("custom", int32)
payload: Payload;
}
@post op create(@body body: Request.payload): void;Although the underlying TypeSpec property is model-shaped, AutoRest applies the encoding and emits an integer schema such as { type: "integer", format: "int32" }; the migrated Swagger rule must therefore report it. Other encodings can produce no usable format and leave an untyped schema, which the original rule does not report.
So the format value is only an intermediate signal used to determine whether AutoRest applies the encoding and changes schema.type. I agree the reason is subtle and will add documentation around this logic.
There was a problem hiding this comment.
but this doesn't make sense, the goal of this rule is to check the body is a model, either it is or it;s not. If it has any encode to anything else it is not(If that is even worth checking), that's it don't need to check further. The rules should NOT try to mimic exactly what autorest output did they should work wiht what you can do in typespec.
There was a problem hiding this comment.
Context
A TypeSpec Model is a compiler-level construct, not necessarily a Swagger type: object.
Examples:
model Request {
value: string;
}emits an object schema.
model Request is Array<string>;is also a TypeSpec Model, but emits:
{ "type": "array", "items": { "type": "string" } }Visibility transformations, model properties, templates, files, and encodings can further change the effective emitted schema.
The reverse also matters: a non-Model TypeSpec type can emit no Swagger type, such as unknown; the Swagger rule ignores it even though it is not a TypeSpec model.
Need discussion
Now the gap is clear and we should answer the question: whether real behavior equivalence is strict standard for the rule migration. And I need discussion with catalinaperalta
There was a problem hiding this comment.
yeah by model I mean a plain model (one without an indexer) which is easily checkable, what does this have to do with encode.
There was a problem hiding this comment.
This resolves my response to the array example: although arrays have kind === "Model", they have an indexer, so the simple plain-model check still rejects them. The remaining decision is product-level: whether this promotion prioritizes Swagger diagnostic parity or translates the Swagger guideline into idiomatic TypeSpec policy. CC catalinaperalta
yeah by model I mean a plain model (one without an indexer) which is easily checkable, what does this have to do with encode.
There was a problem hiding this comment.
it is still feature parity, you cannot do that in TypeSpec, the check that swagger was doing with format has no equivalence in TypeSpec, this is just dead code. Can you show me a test that would pass here without all that extra noise.
There was a problem hiding this comment.
We should focus on being idiomatic to TypeSpec and not bringing in swagger logic into the linter rules. Wherever relevant we should maintain parity with the swagger checks but only if it's not violating the previous statement of not being idiomatic to typespec
There was a problem hiding this comment.
Updated in a29fa65 following this guidance and Catalina's TypeSpec-first decision. The rule now checks the authored body semantically: a valid single body must resolve to a Model without an indexer. It retains only the HTTP-level void and multipart exemptions and project-owned diagnostic targeting. All AutoRest encoding, format, union-emission, scalar-emission, and reference-resolution simulation was removed. The focused suite was rewritten around plain models, model-property references, arrays/records, transformed ARM action arrays, file bodies, primitives/non-models, synthetic void, and multipart bodies; all 10 tests pass.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…igrations (#5446) ## Summary Update only `.github/skills/develop-lintdiff-rule/SKILL.md` to codify the migration principle settled in [PR #5361](#5361 (comment)): prefer idiomatic TypeSpec validation and preserve Swagger parity only where it naturally maps to TypeSpec semantics. Exact executable Swagger behavior is not the migration contract when reproducing it requires non-idiomatic checks, AutoRest schema formatting/encoding simulation, or handling constructs already invalidated by TypeSpec or Azure rules. The guidance now requires tests showing that added special cases affect valid supported TypeSpec; otherwise, omit the complexity and document the intentional parity gap. Align coverage classification, the semantic completeness gate, fixture requirements, migration evidence, and review guidance with this priority. Corpus/lintdiff and emission-matrix evidence remain useful research tools, not mandates to reproduce emitter behavior. This follows Timothée Guérin's [plain-model/indexer guidance](#5361 (comment)) and Catalina Peralta's confirmation of the idiomatic-TypeSpec-first principle. ## Scope Independent documentation-only PR based on and targeting `feature/lintdiff-migration-new`, not `main`. No production rules, fixtures, generated corpus data, dependency changes, or release metadata. Chronus excludes Markdown files from change detection. ## Validation - Prettier check limited to the changed skill document. - `git diff --check`. - No repository-wide formatting, build, or tests for this documentation-only change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Yuchao Yan (msyyc)
changed the title
## Summary Adopt the TypeSpec-first migration principle settled in #5361: native linter rules should validate idiomatic TypeSpec semantics, preserving Swagger parity only where it naturally maps to that contract. Exact executable Swagger behavior is not a requirement when it entails emitter/format/encoding simulation or handling constructs already invalidated by TypeSpec or Azure rules. - Update the develop-lintdiff-rule skill's implementation boundary, evidence gate, fixture requirements, migration reporting, and review guidance. Added complexity must have tests demonstrating value for valid supported TypeSpec; otherwise omit it and document intentional parity gaps. - Update ParametersSchemaAsTypeObject/migration.md to preserve historical corpus numbers and evidence while labeling exact-emission parity as historical, not the promotion acceptance criterion. State the promoted use-model-request-body contract: a plain model without an indexer, with absent/synthetic void and multipart exemptions. ## Scope Independent documentation-only PR against `feature/lintdiff-migration-new`. Contains exactly the skill and migration note; no production implementation, generated corpus data, or dependency changes. Markdown is excluded by the Chronus changedFiles configuration, so no release metadata is added. The existing corpus evidence remains historical; no new corpus run or broad repository build/test was performed for this documentation change. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Yuchao Yan (msyyc)
changed the title
Summary
Promotes the ARM request-body guideline represented by Swagger validator rule
ParametersSchemaAsTypeObjectas the idiomatic TypeSpec ruleuse-model-request-body.TypeSpec-first behavior
The native rule validates authored TypeSpec semantics rather than reproducing AutoRest's emitted Swagger schema details:
Modelwithout an indexervoidbodies, and multipart bodies are exemptThis follows the review decision to prefer idiomatic TypeSpec validation and preserve Swagger parity only where it maps naturally to TypeSpec. Constructs already rejected by Azure rules or emitter diagnostics do not justify additional AutoRest simulation in this rule.
Relationship to lintdiff evidence
The original lintdiff
migration.mdrecords the earlier exact-emission parity investigation and remains useful historical evidence. Its exact diagnostic-parity conclusion is not the acceptance criterion for this promoted rule; differences caused by invalid or non-idiomatic TypeSpec are intentional. The source document lives only onfeature/lintdiff-migration-new, so revising it requires a separate change on that branch rather than adding the lintdiff package to thismain-targeted PR.Validation
use-model-request-bodytests pass