Skip to content

[WIP][Swagger Linter Development] DescriptionMustNotBeNodeName - #5662

Merged
Yuchao Yan (msyyc) merged 5 commits into
feature/lintdiff-migration-newfrom
lintdiff-description-must-not-be-node-name
Oct 12, 2026
Merged

Yuchao Yan (msyyc) merged 5 commits into
feature/lintdiff-migration-newfrom
lintdiff-description-must-not-be-node-name

Conversation

@msyyc

@msyyc Yuchao Yan (msyyc) commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Original Swagger linter

Specific checks:

  • Select objects containing a description field recursively.

  • Ignore non-string descriptions.

  • Normalize description/name text by trimming whitespace, removing periods, and lowercasing.

  • Reject a description matching an explicit string name.

  • Otherwise reject a description matching the containing node's string path key.

  • Otherwise reject the literal placeholder description.

  • Report at the offending description field.

How the Swagger linter works

The legacy ARM rule recursively selects description-bearing Swagger objects and compares their text with an explicit name or the object's path key. Consequently it can inspect schema/property names, HTTP verbs, parameters, and x-ms-enum.values metadata. It emits at most one finding per selected object using the ordered conditions above. Its executable ARM registration is the audience source of truth despite the documentation's broader applicability wording.

The validator counts emitted occurrences, not unique authored declarations. SQL's repeated root files and AlertsManagement's inline parameter copies therefore multiply findings. Generated or qualified schema names, client compatibility types, unused source declarations, and literal route parameters also make output nodes differ from editable TypeSpec targets. Those differences are investigated rather than recreated through emitter-dependent production logic.

How the migrated TypeSpec linter works

Mapping: DescriptionMustNotBeNodeName → tsp-lintdiff-local-linter/description-must-not-be-node-name.

The native rule checks explicit documentation obtained through compiler getDoc on named models, scalars, enums, unions, enum members, union variants, model properties, and operations. HTTP path/query/header names take precedence over compiler-resolved ordinary JSON property names. getHttpOperation supplies the effective operation verb, including inferred GET without a body and POST with a body. Missing or normalized-empty documentation remains outside this rule; status-code properties remain exempt.

Earlier changes closed missing enum-member/union-variant checks and removed the blanket key-property exemption. Enum members are iterated from the enum listener, following official documentation-required prior art because the installed compiler walker does not dispatch enum-member events. Named union variants compare their name; unnamed string variants compare their literal value, while other unnamed variants are checked only for placeholder text.

Cycle1 repairs two additional native semantic omissions confirmed during promotion review:

  • @route("/widgets") @doc("get") op read(): string; now warns using its inferred GET verb; a body-bearing operation documented only as post likewise warns.

  • @encodedName("application/json", "wire-name") @doc("wire-name") sourceName: string; now warns using its JSON wire name. Documentation matching only sourceName is compliant when JSON serialization renames it.

These are supported compiler/HTTP semantics, not emitter overrides. Explicit verbs, normalization, placeholder handling, HTTP parameter-name precedence, and authored diagnostic targets are preserved.

Each violation targets its semantic declaration. The rule does not deduplicate distinct semantic instances merely because they share a source location. It does not project versions, invoke emission, predict generated schema names, or depend on OpenAPI/emitter/SDK APIs. Valid authored warnings from older versions remain native findings; selected-version attribution is separate research evidence.

Migration evidence and validation

See migration.md for the native contract, supported-shape evidence, regenerated full corpus, verified selected-version attribution reuse, all one-sided projects, six compilation exclusions, concrete outlier explanations, and preserved research limitations. The unchanged corpus diagnostic multiset did not expose the two repaired shapes; focused native tests and emitted comparison fixtures establish those fixes.

Required cycle1 validation passed: explicit maintained-file Prettier write/check, package build, changed-TypeScript oxlint with denied warnings, 41 native predicate tests, 13 strict rule fixtures, the affected 14-case PATCH fixture group, representative and full corpus runs, and diff hygiene. Independent persistent local precommit review was clean on the complete branch diff and verified all supplied content/receipt hashes, with nine additional native probes.

The documented native contract is covered; arbitrary executable Swagger equivalence remains partial, not claimed from project overlap or raw count equality. All 1,005 generated corpus changes were archived and restored through the exact approved manifest plan. Corpus/coverage outputs and temporary fixture junctions are excluded from this PR.

Cycle2 multipart source repair

Confirmed promotion review thread PRRT_kwDOKoCXvs6rCcjD (comment4237005053) exposed a missed authored multipart part name. The native rule now uses an authored getHttpPart(program, type)?.options.name when present and otherwise the source property name, including repeated parts via compiler array metadata; JSON encoded names remain limited to ordinary non-part properties. Path/query/header precedence and status-code exemption remain intact. Renamed-part violation, source-name-only compliance, default-name JSON-isolation, meaningful-documentation, and supported scalar/array/repeated-part regressions preserve earlier inferred-verb and ordinary JSON-name fixes. Production uses public compiler/HTTP APIs only.

Validation:47 native tests,15 strict target-rule fixtures and all five genuinely affected fixture groups pass. The full current468-project corpus has462 successful projects and six preserved compiler exclusions. Complete8,499 diagnostic multisets match prior archived results, allowing conservative selected-version attribution reuse:7,501 findings/142projects. Generated corpus output was archived and exactly1,005 planned paths restored; none is included in this PR. Detailed native contract, parity limits and current corpus evidence remain in the linked migration document above.

Historical Formdata required-gate failure is preserved; explicit user authorization prospectively excluded its unchanged unmapped data-plane group without retry or edit. A separately authorized single snapshot correction only reordered seven identical warnings for preexisting listener-phase drift; no unrelated semantics changed.

Outer persistent independent local review f235a949 is clean, with actual follow-up capability and additional multiple-operation/shared/spread/repeated-part probes verified. SourceRepairCycles2/3, development ordinary corrections3/3 and task-wide reserve2/3 remain unchanged. Fresh GitHub review of this exact repair head is required before promotion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The placeholder diagnostic is misleading, and a union-variant test does not distinguish the intended name-selection behavior.

1 open finding
What changed in this PR

Extends the migrated lint rule to cover enum members, union variants, and key properties, with migration evidence and fixtures.

Changes:

  • Expands semantic target coverage.
  • Adds focused unit tests and comparison fixtures.
  • Documents migration scope, evidence, and intentional differences.
File Description
src/​rules/​description-must-not-be-node-name.ts Extends rule targets and removes key exemption.
test/​rules/​description-must-not-be-node-name.test.ts Adds predicate and diagnostic tests.
test/​fixtures/​DescriptionMustNotBeNodeName/​rule.md Documents the native contract and cases.
test/​fixtures/​DescriptionMustNotBeNodeName/​migration.md Records migration analysis and validation.
member-and-key-documentation/​main.tsp Adds representative fixture inputs.
member-and-key-documentation/​output.json Captures emitted Swagger.
member-and-key-documentation/​validator-diagnostics.json Captures validator findings.
member-and-key-documentation/​tsp-diagnostics.json Captures TypeSpec findings.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/typespec-lintdiff/test/rules/description-must-not-be-node-name.test.ts Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Reproduction commands are missing, and the unnamed non-string union-variant branch lacks regression coverage.

0 open findings

1 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Add regression tests for unnamed non-string variant fallback

packages/​typespec-lintdiff/​src/​rules/​description-must-not-be-node-name.ts:86

The new fallback for unnamed, non-string variants is not covered by the new suite, even though the documented contract says these variants should run only the placeholder check. Add one violating case such as union U { @doc("description") int32 } and one compliant case such as union U { @doc("int32") int32 }; otherwise this undefined branch could start comparing the underlying type name or stop checking placeholders without a regression failure.

Low severity Document commands needed to reproduce migration test results

packages/​typespec-lintdiff/​test/​fixtures/​DescriptionMustNotBeNodeName/​migration.md:546

These pass claims are not reproducible because this migration note does not record any of the commands used for the focused tests, strict fixture validation, or corpus generation. Since the generated corpus artifacts are intentionally excluded, established migration notes preserve the exact invocations here (for example, GetCollectionOnlyHasValueAndNextLink/migration.md:44-50 and ConsistentResponseSchemaForPut/migration.md:196-204). Please add the commands, including the pinned specs checkout argument and filtering/concurrency options, so the reported counts can be regenerated.

🧠 Review effort: Balanced

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The implementation is coherent, but the extensive cross-repository corpus and migration-equivalence claims warrant final human verification.

0 open findings

🧠 Review effort: Balanced

Resolve native HTTP part metadata before JSON naming and cover scalar, array, repeated and default parts. Refresh migration evidence and the diagnosed order-only snapshot baseline.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Multipart default-name resolution and placeholder diagnostic formatting currently diverge from the Swagger validator.

1 open finding
Previously missed (4)

In code that hasn't changed since last review

Medium severity Preserve literal placeholder name in mismatch diagnostics

packages/​typespec-lintdiff/​src/​rules/​description-must-not-be-node-name.ts:50

The literal-placeholder branch must format Node name:'description' when the actual node name does not match. The legacy validator does this in its final description === "description" branch, but checkTarget always formats the supplied name; the new encoded-name fallback therefore changes the existing placeholder snapshot from description to patchDescription. Keep the actual name only when normalized name equality caused the warning.

This issue also appears on line 119 of the same file.

Medium severity Restore unchanged placeholder diagnostic snapshot

packages/​typespec-lintdiff/​test/​fixtures/​PatchPropertiesCorrespondToPutProperties/​encoded-name-mismatch/​tsp-diagnostics.json:15

This snapshot change records an incorrect diagnostic message. The encoded name patchDescription does not match the normalized text Description, so the legacy validator reaches its placeholder branch and emits Node name:'description'; this expectation should remain unchanged once checkTarget formats that branch correctly.

Medium severity Fix placeholder branch expected diagnostic name

packages/​typespec-lintdiff/​test/​rules/​description-must-not-be-node-name.test.ts:53

This expected name encodes the same placeholder-branch regression as the implementation. Because description. does not match wire-name, the legacy rule's final placeholder branch reports the literal node name description, not the encoded property name.

Low severity Restore legacy placeholder diagnostic name

packages/​typespec-lintdiff/​test/​fixtures/​DescriptionMustNotBeNodeName/​migration.md:635

patchDescription is not the correct legacy diagnostic name for this placeholder case. Since Description does not match the encoded node name, the validator reaches its literal-placeholder branch and reports Node name:'description'; the updated snapshot demonstrates a message-parity regression rather than a correction.

🧠 Review effort: Balanced

Comment thread packages/typespec-lintdiff/test/fixtures/DescriptionMustNotBeNodeName/rule.md Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Unnamed numeric union variants remain undetected, and placeholder diagnostics can report a node name that does not match the description.

0 open findings

1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Validate unnamed numeric union variants

packages/​typespec-lintdiff/​src/​rules/​description-must-not-be-node-name.ts:90

Unnamed numeric union variants are a supported emitted enum shape: getUnionAsEnum retains Number literals, and AutoRest emits each unnamed value with a stringified x-ms-enum.values[*].name. This branch leaves their node name undefined, so union Code { @doc("1") 1 } is missed even though the Swagger validator reports it. Handle Number literals as ${target.type.value} and add violating/compliant coverage (with the contract updated accordingly).

🧠 Review effort: Balanced

@msyyc
Yuchao Yan (msyyc) marked this pull request as ready for review October 12, 2026 03:59
@msyyc
Yuchao Yan (msyyc) merged commit 11eff40 into feature/lintdiff-migration-new Oct 12, 2026
3 checks passed
@msyyc
Yuchao Yan (msyyc) deleted the lintdiff-description-must-not-be-node-name branch October 12, 2026 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants