Repository navigation
[Swagger Linter Migration] DescriptionMustNotBeNodeName + DescriptiveDescriptionRequired -> documentation-required #5663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
d7b4a89
9bdd488
2c5214b
0a9455b
603330a
b6834e1
2068349
5e8dc59
208e3e6
7cd29bd
8a3c53e
105eb1b
e4e52da
4153348
4868be2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| changeKind: feature | ||
| packages: | ||
| - "@azure-tools/typespec-azure-core" | ||
| --- | ||
|
|
||
| Add the `no-uninformative-doc` lint rule to reject explicit documentation that repeats a declaration name or contains placeholder text. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| changeKind: internal | ||
| packages: | ||
| - "@azure-tools/typespec-azure-rulesets" | ||
| --- | ||
|
|
||
| Register the Azure Core `no-uninformative-doc` lint rule as disabled in the data-plane and resource-manager rulesets. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| Documentation should explain what a declaration means or how to use it, rather than repeat its | ||
| name. Meaningful descriptions help API users understand generated client types, parameters, | ||
| and operations. | ||
|
|
||
| This rule checks explicit `@doc` text and doc comments on named models, scalars, enums, unions, | ||
| enum members, union variants, properties, and operations. It ignores surrounding whitespace, | ||
| periods, and case when comparing text. The placeholder `description` also produces a warning. | ||
|
|
||
| HTTP path, query, and header parameters are compared with their configured wire names. | ||
| Ordinary properties are compared with their JSON encoded name, falling back to their source | ||
| name when no JSON override exists. Parameter wire names take precedence over JSON encoded names. | ||
| Operations are compared with their effective HTTP verb, not their operation name, including | ||
| inferred GET for operations without a body and POST for operations with a body. | ||
| Named union variants are compared with their variant name; unnamed string variants are compared | ||
| with their string value. Other unnamed variants are checked only for placeholder text. | ||
|
|
||
| Missing or empty documentation is outside this rule; use `documentation-required` to require | ||
| documentation. Status-code properties are exempt. Key properties are not exempt. | ||
|
|
||
| ## Impact | ||
|
|
||
| - **Area:** API, SDK | ||
|
|
||
| Repeating names or leaving placeholder text makes API reference documentation and generated | ||
| client documentation less useful. | ||
|
|
||
| ## Examples | ||
|
|
||
| These examples illustrate shared model and property documentation, applicable to both | ||
| data-plane and Resource Manager APIs. | ||
|
|
||
| #### Incorrect | ||
|
|
||
| ```tsp | ||
| /** Widget */ | ||
| model Widget { | ||
| /** Name */ | ||
| name: string; | ||
| } | ||
| ``` | ||
|
|
||
| #### Correct | ||
|
|
||
| ```tsp | ||
| /** A configurable device available in the store. */ | ||
| model Widget { | ||
| /** The display name shown to users. */ | ||
| name: string; | ||
| } | ||
| ``` | ||
|
|
||
| ## Suppression | ||
|
|
||
| Prefer replacing repetitive or placeholder text with meaningful documentation. Suppress this | ||
| rule only when the name itself is the clearest useful description and adding prose would not | ||
| help API consumers. | ||
|
|
||
| ## LintDiff Equivalent | ||
|
|
||
| This rule corresponds to | ||
| [DescriptionMustNotBeNodeName](https://github.com/Azure/azure-openapi-validator/blob/main/docs/description-must-not-be-node-name.md). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| import { | ||
| createRule, | ||
| fileRef, | ||
| getDoc, | ||
| ignoreDiagnostics, | ||
| paramMessage, | ||
| resolveEncodedName, | ||
| type Enum, | ||
| type EnumMember, | ||
| type Model, | ||
| type ModelProperty, | ||
| type Operation, | ||
| type Scalar, | ||
| type Union, | ||
| type UnionVariant, | ||
| } from "@typespec/compiler"; | ||
| import { | ||
| getHeaderFieldName, | ||
| getHttpOperation, | ||
| getPathParamName, | ||
| getQueryParamName, | ||
| isStatusCode, | ||
| } from "@typespec/http"; | ||
|
|
||
| type NamedTarget = Enum | EnumMember | Model | Scalar | Union; | ||
|
|
||
| export const noUninformativeDocRule = createRule({ | ||
| name: "no-uninformative-doc", | ||
| docs: fileRef.fromPackageRoot("src/rules/no-uninformative-doc.md"), | ||
| description: "Explicit documentation should describe a declaration rather than repeat its name.", | ||
| severity: "warning", | ||
| url: "https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/no-uninformative-doc", | ||
| messages: { | ||
| default: paramMessage`Description must not match the name of the node it describes. Node name:'${"name"}' Description:'${"description"}'`, | ||
| }, | ||
| create(context) { | ||
| const checkTarget = (target: Parameters<typeof getDoc>[1], nodeName: string | undefined) => { | ||
| const doc = getDoc(context.program, target); | ||
| if (doc === undefined) { | ||
| return; | ||
| } | ||
|
|
||
| const normalizedDescription = normalize(doc); | ||
| if (normalizedDescription.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| const normalizedNodeName = nodeName ? normalize(nodeName) : undefined; | ||
| if ( | ||
| normalizedDescription !== "description" && | ||
| (normalizedNodeName === undefined || normalizedNodeName !== normalizedDescription) | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| context.reportDiagnostic({ | ||
| target, | ||
| format: { | ||
| name: nodeName ?? "description", | ||
| description: doc, | ||
| }, | ||
| }); | ||
| }; | ||
|
|
||
| const checkNamedTarget = (target: NamedTarget) => { | ||
| const { name } = target; | ||
| if (!name || name.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| checkTarget(target, name); | ||
| }; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this extra function or can it be combined with the one above?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The wrapper separates checking named declarations from checking other targets.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Combined the wrappers into one checker with a default declaration name. Kept the named-declaration empty-name exemption and the explicit HTTP/JSON/multipart/union-variant name overrides, so this refactor does not broaden documentation policy. Implemented in 603330abc. Scoped formatting, type-aware lint, package build and the focused native tests passed. Review threads are intentionally left open. |
||
|
|
||
| return { | ||
| model: checkNamedTarget, | ||
| scalar: checkNamedTarget, | ||
| enum: (target: Enum) => { | ||
| checkNamedTarget(target); | ||
| for (const member of target.members.values()) { | ||
| checkNamedTarget(member); | ||
| } | ||
| }, | ||
| union: checkNamedTarget, | ||
| unionVariant: (target: UnionVariant) => { | ||
| checkTarget( | ||
| target, | ||
| typeof target.name === "string" | ||
| ? target.name | ||
| : target.type.kind === "String" | ||
| ? target.type.value | ||
| : undefined, | ||
| ); | ||
| }, | ||
| operation: (target: Operation) => { | ||
| // HTTP validation owns resolution diagnostics; this rule only checks documentation. | ||
| const httpOperation = ignoreDiagnostics(getHttpOperation(context.program, target)); | ||
| checkTarget(target, httpOperation.verb); | ||
| }, | ||
| modelProperty: (target: ModelProperty) => { | ||
| if (isStatusCode(context.program, target)) { | ||
| return; | ||
| } | ||
|
|
||
| const emittedName = | ||
| getPathParamName(context.program, target) ?? | ||
| getQueryParamName(context.program, target) ?? | ||
| getHeaderFieldName(context.program, target); | ||
|
msyyc marked this conversation as resolved.
Outdated
msyyc marked this conversation as resolved.
Outdated
|
||
|
|
||
| if (emittedName !== undefined) { | ||
| checkTarget(target, emittedName); | ||
| return; | ||
| } | ||
|
|
||
| checkTarget(target, resolveEncodedName(context.program, target, "application/json")); | ||
| }, | ||
| }; | ||
| }, | ||
| }); | ||
|
|
||
| function normalize(value: string): string { | ||
| return value.trim().replace(/\./g, "").toLowerCase(); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.