diff --git a/.chronus/changes/no-uninformative-doc-2026-10-10.md b/.chronus/changes/no-uninformative-doc-2026-10-10.md new file mode 100644 index 0000000000..8dd31b08c2 --- /dev/null +++ b/.chronus/changes/no-uninformative-doc-2026-10-10.md @@ -0,0 +1,13 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-azure-core" +--- + +Strengthen `documentation-required` to reject explicitly empty or whitespace-only documentation and descriptions that repeat a declaration's name or the placeholder `description`. Ordinary properties use their authored names, regardless of JSON serialization names; HTTP parameters and multipart parts retain their HTTP names. Explicit-documentation checks also cover namespaces, interfaces, scalars, inline properties, union variants, and declarations exempt from missing-doc requirements when their documentation is exposed by the compiler. Operation descriptions are compared with both the source operation name and effective HTTP verb. Missing-documentation requirements and exemptions are unchanged. The existing data-plane and resource-manager rulesets already enable this rule. + +```tsp +// Warns: the description only repeats the authored name. +/** Widget */ +model Widget {} +``` diff --git a/.chronus/changes/promote-lintdiff-description-must-not-be-node-name-2026-9-14-18-55-26.md b/.chronus/changes/promote-lintdiff-description-must-not-be-node-name-2026-9-14-18-55-26.md new file mode 100644 index 0000000000..4a0683cf44 --- /dev/null +++ b/.chronus/changes/promote-lintdiff-description-must-not-be-node-name-2026-9-14-18-55-26.md @@ -0,0 +1,7 @@ +--- +changeKind: internal +packages: + - "@azure-tools/typespec-samples" +--- + +Update Widget Manager color and ARM library provisioning-state descriptions to satisfy documentation-required, and regenerate sample snapshots. \ No newline at end of file diff --git a/packages/samples/specs/data-plane/widget-manager/main.tsp b/packages/samples/specs/data-plane/widget-manager/main.tsp index 675be2e300..81305693a5 100644 --- a/packages/samples/specs/data-plane/widget-manager/main.tsp +++ b/packages/samples/specs/data-plane/widget-manager/main.tsp @@ -39,19 +39,19 @@ enum Versions { union WidgetColor { string, - @doc("Black") + @doc("The widget is black.") Black: "Black", - @doc("White") + @doc("The widget is white.") White: "White", - @doc("Red") + @doc("The widget is red.") Red: "Red", - @doc("Green") + @doc("The widget is green.") Green: "Green", - @doc("Blue") + @doc("The widget is blue.") Blue: "Blue", } diff --git a/packages/samples/specs/resource-manager/arm-library/main.tsp b/packages/samples/specs/resource-manager/arm-library/main.tsp index 20bdb2acd7..4bac917f2b 100644 --- a/packages/samples/specs/resource-manager/arm-library/main.tsp +++ b/packages/samples/specs/resource-manager/arm-library/main.tsp @@ -75,7 +75,7 @@ union SystemEnvelopeProvisioningState { string, ResourceProvisioningState, - @doc("Working") + @doc("The resource provisioning operation is in progress.") Working: "Working", } diff --git a/packages/samples/test/output/azure/core/data-plane/widget-manager/@azure-tools/typespec-autorest/2022-08-31/openapi.json b/packages/samples/test/output/azure/core/data-plane/widget-manager/@azure-tools/typespec-autorest/2022-08-31/openapi.json index 67050635a9..ade9c2b31c 100644 --- a/packages/samples/test/output/azure/core/data-plane/widget-manager/@azure-tools/typespec-autorest/2022-08-31/openapi.json +++ b/packages/samples/test/output/azure/core/data-plane/widget-manager/@azure-tools/typespec-autorest/2022-08-31/openapi.json @@ -2249,27 +2249,27 @@ { "name": "Black", "value": "Black", - "description": "Black" + "description": "The widget is black." }, { "name": "White", "value": "White", - "description": "White" + "description": "The widget is white." }, { "name": "Red", "value": "Red", - "description": "Red" + "description": "The widget is red." }, { "name": "Green", "value": "Green", - "description": "Green" + "description": "The widget is green." }, { "name": "Blue", "value": "Blue", - "description": "Blue" + "description": "The widget is blue." } ] } diff --git a/packages/samples/test/output/azure/resource-manager/arm-library/@azure-tools/typespec-autorest/2021-09-21-preview/openapi.json b/packages/samples/test/output/azure/resource-manager/arm-library/@azure-tools/typespec-autorest/2021-09-21-preview/openapi.json index 126f7aa3fe..ff2059c646 100644 --- a/packages/samples/test/output/azure/resource-manager/arm-library/@azure-tools/typespec-autorest/2021-09-21-preview/openapi.json +++ b/packages/samples/test/output/azure/resource-manager/arm-library/@azure-tools/typespec-autorest/2021-09-21-preview/openapi.json @@ -1982,7 +1982,7 @@ { "name": "Working", "value": "Working", - "description": "Working" + "description": "The resource provisioning operation is in progress." } ] }, diff --git a/packages/typespec-azure-core/README.md b/packages/typespec-azure-core/README.md index 5a34006aa4..2b3e59b9ae 100644 --- a/packages/typespec-azure-core/README.md +++ b/packages/typespec-azure-core/README.md @@ -58,7 +58,7 @@ Available ruleSets: | [`@azure-tools/typespec-azure-core/no-rest-library-interfaces`](https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/no-rest-library-interfaces) | Resource interfaces from the TypeSpec.Rest.Resource library are incompatible with Azure.Core. | | [`@azure-tools/typespec-azure-core/no-unknown`](https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/no-unknown) | Azure services must not have properties of type `unknown`. | | [`@azure-tools/typespec-azure-core/bad-record-type`](https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/bad-record-type) | Identify bad record definitions. | -| [`@azure-tools/typespec-azure-core/documentation-required`](https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/documentation-required) | Require documentation over enums, models, and operations. | +| [`@azure-tools/typespec-azure-core/documentation-required`](https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/documentation-required) | Require meaningful documentation over enums, models, and operations. | | [`@azure-tools/typespec-azure-core/key-visibility-required`](https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/key-visibility-required) | Key properties need to have a Lifecycle visibility setting. | | [`@azure-tools/typespec-azure-core/response-schema-problem`](https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/response-schema-problem) | Warn about operations having multiple non-error response schemas. | | [`@azure-tools/typespec-azure-core/rpc-operation-request-body`](https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/rpc-operation-request-body) | Warning for RPC body problems. | diff --git a/packages/typespec-azure-core/src/rules/documentation-required.md b/packages/typespec-azure-core/src/rules/documentation-required.md index 5b28cd5200..e6bfaf1e27 100644 --- a/packages/typespec-azure-core/src/rules/documentation-required.md +++ b/packages/typespec-azure-core/src/rules/documentation-required.md @@ -1,18 +1,22 @@ -Enums, models, operations, and their members/properties should have documentation. Use doc comments (`/** */`) to provide descriptions. +Enums, models, operations, and their members/properties should have meaningful documentation. Use doc comments (`/** */`) or `@doc` to provide descriptions. -## Impact +Explicit descriptions must not be empty, whitespace-only, the placeholder `description`, or a repetition of the name they describe. Name comparisons ignore surrounding whitespace, periods, and case. -- **Area:** API, SDK +- Ordinary properties use their authored names, regardless of serialization names. +- Operations are compared with their authored names and effective HTTP verbs. HTTP parameters and multipart parts use their HTTP names, falling back to the property name for unnamed parts. +- Enum members and named union variants use their names; unnamed string variants use their values. HTTP status-code properties are exempt from name comparisons. + +Namespaces, interfaces, and scalars do not require documentation, but explicitly supplied blank descriptions are checked. Explicit documentation on inline properties and declarations exempt from missing-documentation requirements is also checked. Uninstantiated template documentation unavailable from the compiler is not checked. -Missing documentation produces poor SDK reference docs and API documentation. +## Impact -## LintDiff Equivalent +- **Area:** API, SDK -This rule corresponds to the LintDiff rule [SummaryOrDescription](https://github.com/Azure/azure-openapi-validator/blob/main/docs/operation-summary-or-description.md). +Missing or uninformative documentation produces poor SDK reference docs and API documentation. #### ❌ Incorrect -Model without documentation: +Missing documentation: ```tsp model Pet { @@ -20,19 +24,21 @@ model Pet { } ``` -Operation without documentation: +Descriptions that only repeat the authored names: ```tsp -op read(): void; +/** Widget */ +model Widget { + /** Name */ + name: string; +} ``` -Enum without documentation: +An explicitly blank description: ```tsp -enum PetKind { - Cat, - Dog, -} +@doc(" \t") +scalar Label extends string; ``` #### ✅ Correct @@ -55,12 +61,15 @@ enum PetKind { /** A dog. */ Dog, } + +/** A display label shown to users. */ +scalar Label extends string; ``` :::note -Version enums and discriminator enums/unions are exempt from this rule as they are considered self-documenting. +Version enums and discriminator enums/unions are exempt from missing-documentation requirements as they are considered self-documenting. Explicit descriptions on these declarations are still checked for blank or uninformative content. ::: ## Suppression -Do not suppress. Add a `/** */` doc comment or an `@doc` decorator. +Do not suppress. Add a meaningful `/** */` doc comment or an `@doc` decorator. diff --git a/packages/typespec-azure-core/src/rules/require-docs.ts b/packages/typespec-azure-core/src/rules/require-docs.ts index 1291512b01..657d597527 100644 --- a/packages/typespec-azure-core/src/rules/require-docs.ts +++ b/packages/typespec-azure-core/src/rules/require-docs.ts @@ -1,4 +1,12 @@ import { + createRule, + fileRef, + getDiscriminatedTypes, + getDiscriminator, + getDoc, + ignoreDiagnostics, + isArrayModelType, + paramMessage, type Discriminator, type Enum, type Model, @@ -7,13 +15,15 @@ import { type Program, type Union, type UnionVariant, - createRule, - fileRef, - getDiscriminatedTypes, - getDiscriminator, - getDoc, - paramMessage, } from "@typespec/compiler"; +import { + getHeaderFieldName, + getHttpOperation, + getHttpPart, + getPathParamName, + getQueryParamName, + isStatusCode, +} from "@typespec/http"; import { getVersionsForEnum } from "@typespec/versioning"; import { isExcludedCoreType, @@ -23,6 +33,47 @@ import { isTemplatedOperationSignature, } from "./utils.js"; +type DocumentationTarget = Parameters[1]; + +function normalizeDocumentation(value: string): string { + return value.trim().replace(/\./g, "").toLowerCase(); +} + +function getDocumentationName( + program: Program, + target: DocumentationTarget, + doc: string, +): string | undefined { + switch (target.kind) { + case "UnionVariant": + return typeof target.name === "string" + ? target.name + : target.type.kind === "String" + ? target.type.value + : undefined; + case "Operation": + if (normalizeDocumentation(doc) === normalizeDocumentation(target.name)) { + return target.name; + } + // HTTP validation owns resolution diagnostics; this rule only checks documentation. + return ignoreDiagnostics(getHttpOperation(program, target)).verb; + case "ModelProperty": { + const httpName = + getPathParamName(program, target) ?? + getQueryParamName(program, target) ?? + getHeaderFieldName(program, target); + if (httpName !== undefined) return httpName; + const partType = + target.type.kind === "Model" && isArrayModelType(target.type) + ? target.type.indexer.value + : target.type; + return getHttpPart(program, partType)?.options.name ?? target.name; + } + default: + return "name" in target && typeof target.name === "string" ? target.name : undefined; + } +} + /** Versioning enums are self-documenting and don't need separate documentation. */ function isExcludedVersionEnum(program: Program, enumObj: Enum): boolean { const versions = getVersionsForEnum(program, enumObj); @@ -85,16 +136,64 @@ function getVariantName(variant: UnionVariant): string | undefined { export const requireDocumentation = createRule({ name: "documentation-required", docs: fileRef.fromPackageRoot("src/rules/documentation-required.md"), - description: "Require documentation over enums, models, and operations.", + description: "Require meaningful documentation over enums, models, and operations.", severity: "warning", url: "https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/documentation-required", messages: { default: paramMessage`The ${"kind"} named '${"name"}' should have a documentation or description, use doc comment /** */ to provide it.`, + empty: "Descriptions cannot be empty or whitespace-only. Provide a meaningful doc string.", + uninformative: paramMessage`Description must not match the name of the node it describes. Node name:'${"name"}' Description:'${"description"}'`, }, create(context) { const discTypes = getDiscriminatedTypes(context.program); + const needsDocumentation = (target: DocumentationTarget) => { + return getDoc(context.program, target) === undefined; + }; + const checkDocumentation = (target: DocumentationTarget) => { + const doc = getDoc(context.program, target); + if (doc === undefined) return; + if (doc.trim().length === 0) { + context.reportDiagnostic({ target, messageId: "empty" }); + return; + } + if ( + target.kind === "Namespace" || + target.kind === "Interface" || + (target.kind === "ModelProperty" && isStatusCode(context.program, target)) + ) + return; + if ( + (target.kind === "Model" || + target.kind === "Scalar" || + target.kind === "Enum" || + target.kind === "EnumMember" || + target.kind === "Union") && + !target.name + ) + return; + const normalizedDoc = normalizeDocumentation(doc); + if (normalizedDoc.length === 0) return; + const name = getDocumentationName(context.program, target, doc); + if ( + normalizedDoc === "description" || + (name !== undefined && normalizedDoc === normalizeDocumentation(name)) + ) { + context.reportDiagnostic({ + target, + messageId: "uninformative", + format: { name: name ?? "description", description: doc }, + }); + } + }; return { + namespace: checkDocumentation, + interface: checkDocumentation, + scalar: checkDocumentation, + modelProperty: checkDocumentation, + unionVariant: checkDocumentation, + enumMember: checkDocumentation, enum: (enumObj: Enum) => { + checkDocumentation(enumObj); // version enums and enum members are considered intrinsically self-documenting if (isExcludedVersionEnum(context.program, enumObj)) { return; @@ -102,14 +201,14 @@ export const requireDocumentation = createRule({ if (isExcludedDiscriminator(context.program, enumObj, discTypes)) { return; } - if (!getDoc(context.program, enumObj) && !isExcludedCoreType(context.program, enumObj)) { + if (!isExcludedCoreType(context.program, enumObj) && needsDocumentation(enumObj)) { context.reportDiagnostic({ target: enumObj, format: { kind: enumObj.kind, name: enumObj.name }, }); } for (const member of enumObj.members.values()) { - if (!getDoc(context.program, member)) { + if (needsDocumentation(member)) { context.reportDiagnostic({ target: member, format: { kind: member.kind, name: member.name }, @@ -118,6 +217,7 @@ export const requireDocumentation = createRule({ } }, operation: (operation: Operation) => { + checkDocumentation(operation); // Don't pay attention to operations on templated interfaces that // haven't been filled in with parameters yet if ( @@ -125,7 +225,7 @@ export const requireDocumentation = createRule({ !isTemplatedOperationSignature(operation) && !isExcludedCoreType(context.program, operation) ) { - if (!getDoc(context.program, operation)) { + if (needsDocumentation(operation)) { context.reportDiagnostic({ target: operation, format: { kind: operation.kind, name: operation.name }, @@ -133,7 +233,7 @@ export const requireDocumentation = createRule({ } for (const param of operation.parameters.properties.values()) { - if (!getDoc(context.program, param)) { + if (needsDocumentation(param)) { context.reportDiagnostic({ target: param, format: { kind: param.kind, name: param.name }, @@ -143,6 +243,7 @@ export const requireDocumentation = createRule({ } }, model: (model: Model) => { + checkDocumentation(model); // it's by design that the `getDoc` function can't get the `doc` for template declaration type. if ( !isTemplateDeclarationType(model) && @@ -150,7 +251,7 @@ export const requireDocumentation = createRule({ !isExcludedCoreType(context.program, model) && model.name !== "object" ) { - if (!getDoc(context.program, model)) { + if (needsDocumentation(model)) { context.reportDiagnostic({ target: model, format: { kind: model.kind, name: model.name }, @@ -161,7 +262,7 @@ export const requireDocumentation = createRule({ if (isExcludedDiscriminator(context.program, prop, discTypes)) { return; } - if (!getDoc(context.program, prop)) { + if (needsDocumentation(prop)) { context.reportDiagnostic({ target: prop, format: { kind: prop.kind, name: prop.name }, @@ -171,10 +272,11 @@ export const requireDocumentation = createRule({ } }, union: (union: Union) => { + checkDocumentation(union); if (isExcludedDiscriminator(context.program, union, discTypes)) { return; } - if (!getDoc(context.program, union) && !isExcludedCoreType(context.program, union)) { + if (!isExcludedCoreType(context.program, union) && needsDocumentation(union)) { const name = getUnionName(union); if (name !== undefined) { context.reportDiagnostic({ @@ -184,7 +286,7 @@ export const requireDocumentation = createRule({ } } for (const variant of union.variants.values()) { - if (!getDoc(context.program, variant)) { + if (needsDocumentation(variant)) { // symbols don't need documentation const variantName = getVariantName(variant); if (variantName !== undefined) { diff --git a/packages/typespec-azure-core/test/rules/require-docs.test.ts b/packages/typespec-azure-core/test/rules/require-docs.test.ts index 254366d7b3..2316eae540 100644 --- a/packages/typespec-azure-core/test/rules/require-docs.test.ts +++ b/packages/typespec-azure-core/test/rules/require-docs.test.ts @@ -1,4 +1,6 @@ +import { getSourceLocation } from "@typespec/compiler"; import { createLinterRuleTester, type LinterRuleTester } from "@typespec/compiler/testing"; +import { ok } from "node:assert"; import { beforeEach, describe, it } from "vitest"; import { requireDocumentation } from "../../src/rules/require-docs.js"; import { Tester } from "../test-host.js"; @@ -6,150 +8,350 @@ import { Tester } from "../test-host.js"; let tester: LinterRuleTester; beforeEach(async () => { - const runner = await Tester.createInstance(); - tester = createLinterRuleTester(runner, requireDocumentation, "@azure-tools/typespec-azure-core"); + tester = createLinterRuleTester( + await Tester.createInstance(), + requireDocumentation, + "@azure-tools/typespec-azure-core", + ); }); -async function checkDocRequired(code: string, message: string) { - await tester.expect(code).toEmitDiagnostics((x) => ({ - code: "@azure-tools/typespec-azure-core/documentation-required", - message, - pos: x.pos.loc.pos, - })); +const code = "@azure-tools/typespec-azure-core/documentation-required"; +const empty = { + code, + message: "Descriptions cannot be empty or whitespace-only. Provide a meaningful doc string.", +}; + +function missing(kind: string, name: string) { + return { + code, + message: `The ${kind} named '${name}' should have a documentation or description, use doc comment /** */ to provide it.`, + }; +} + +function uninformative(name: string, description = name) { + return { + code, + message: `Description must not match the name of the node it describes. Node name:'${name}' Description:'${description}'`, + }; } -describe("emit `documentation-required` when documentation is missing", () => { - it("on model", async () => - await checkDocRequired( - "/*loc*/model Foo {}", - "The Model named 'Foo' should have a documentation or description, use doc comment /** */ to provide it.", - )); - - it("on model property", async () => - await checkDocRequired( - `@doc("Abc") model Foo { - /*loc*/x: string; - }`, - "The ModelProperty named 'x' should have a documentation or description, use doc comment /** */ to provide it.", - )); - - it("on operation", async () => - await checkDocRequired( - `/*loc*/op read(): void;`, - "The Operation named 'read' should have a documentation or description, use doc comment /** */ to provide it.", - )); - - it("on property", async () => - await checkDocRequired( - `@doc("op doc") op read(/*loc*/param1: string): void;`, - "The ModelProperty named 'param1' should have a documentation or description, use doc comment /** */ to provide it.", - )); - - it("on enum", async () => { - await checkDocRequired( - "/*loc*/enum Foo {}", - "The Enum named 'Foo' should have a documentation or description, use doc comment /** */ to provide it.", - ); - }); - - it("on enum member", async () => { - await checkDocRequired( - `@doc(".") enum Foo { - /*loc*/Bar, - }`, - "The EnumMember named 'Bar' should have a documentation or description, use doc comment /** */ to provide it.", - ); - }); - - it("does not require documentation on version enums", async () => { +function location(target: Parameters[0] | undefined) { + ok(target); + const source = getSourceLocation(target); + ok(source); + const { file, pos, end } = source; + return { file: file.path, pos, end }; +} + +describe("missing documentation", () => { + it.each([ + ["model /*target*/Foo {}", "Model", "Foo"], + ['@doc("A resource.") model Foo { /*target*/x: string; }', "ModelProperty", "x"], + ["op /*target*/read(): void;", "Operation", "read"], + ['@doc("Reads a resource.") op read(/*target*/param: string): void;', "ModelProperty", "param"], + ["enum /*target*/Foo {}", "Enum", "Foo"], + ['@doc("Available values.") enum Foo { /*target*/Bar }', "EnumMember", "Bar"], + ])("requires documentation in %s", async (source, kind, name) => { + await tester + .expect(source) + .toEmitDiagnostics(({ target }) => [{ ...missing(kind, name), ...location(target) }]); + }); + + it("requires documentation on unions and named or string variants", async () => { + await tester + .expect('union PetKind { Cat: "Cat", "Dog", string }') + .toEmitDiagnostics([ + missing("Union", "PetKind"), + missing("UnionVariant", "Cat"), + missing("UnionVariant", "Dog"), + ]); + }); + + it("does not newly require namespace, interface, or scalar documentation", async () => { + await tester + .expect("namespace Widgets {} interface Empty {} scalar Label extends string;") + .toBeValid(); + }); + + it("exempts version enums and their members", async () => { + await tester + .expect('@versioned(Widgets.Versions) namespace Widgets; enum Versions { v1: "2026-01-01" }') + .toBeValid(); + }); + + it.each(["enum PetKind { cat }", 'union PetKind { cat: "cat", string }'])( + "exempts discriminator types and properties: %s", + async (type) => { + await tester + .expect( + ` + ${type} + @discriminator("kind") @doc("A pet resource.") model Pet { kind: PetKind; } + @doc("A cat resource.") model Cat extends Pet { kind: PetKind.cat; } + `, + ) + .toBeValid(); + }, + ); +}); + +describe("blank and meaningful documentation", () => { + it("distinguishes missing, blank, punctuation, source names, and HTTP verbs without duplicates", async () => { await tester .expect( ` - @versioned(Contoso.WidgetManager.Versions) - namespace Contoso.WidgetManager; - - enum Versions { - "2022-08-30", - }`, + @route("/absent") @get op absent(): string; + @route("/blank") @get @doc(" ") op blank(): string; + @route("/punctuation") @get @doc(" ... ") op punctuation(): string; + @route("/read") @get @doc(" READ. ") op read(): string; + @route("/fetch") @get @doc("GET.") op fetch(): string; + `, ) - .toBeValid(); + .toEmitDiagnostics([ + missing("Operation", "absent"), + empty, + uninformative("read", " READ. "), + uninformative("get", "GET."), + ]); }); - it("does not require documentation on discriminator enums", async () => { + it("rejects blank docs on namespaces, interfaces, and scalars without requiring absent docs", async () => { + await tester + .expect( + '@doc(" ") namespace Widgets {} @doc("") interface Empty {} @doc("\\t\\r\\n") scalar Label extends string;', + ) + .toEmitDiagnostics([empty, empty, empty]); + }); + + it("checks inline properties, unnamed variants, and instantiated models", async () => { await tester .expect( ` - enum PetKind { - cat, - dog, - } - - @discriminator("kind") - @doc("Base Pet model") - model Pet { - kind: PetKind; - - @doc("Pet name") - name: string; - } - - @doc("A Cat") - model Cat extends Pet { - kind: PetKind.cat; - }`, + @doc("A generic container.") model Box {} + @doc(" ") model Widget is Box; + @doc("Payload data.") model Payload { + @doc("The nested data.") data: { @doc("\u00a0") value: string }; + } + @doc("Available values.") union Values { @doc(" ") int32, string } + `, + ) + .toEmitDiagnostics([empty, empty, empty]); + }); + + it("checks explicit blanks on version and discriminator declarations despite missing-doc exemptions", async () => { + await tester + .expect( + ` + @versioned(Widgets.Versions) namespace Widgets; + @doc(" ") enum Versions { @doc(" ") v1: "2026-01-01" } + @doc(" ") enum PetKind { @doc(" ") cat } + @doc("A pet resource.") @discriminator("kind") model Pet { @doc(" ") kind: PetKind; } + `, + ) + .toEmitDiagnostics([empty, empty, empty, empty, empty]); + }); + + it("targets each blank property and enum member rather than its container", async () => { + await tester + .expect( + ` + @doc("A widget resource.") model Widget { + @doc("") /*name*/name: string; + @doc(" ") /*value*/value: string; + @doc("A valid label.") label: string; + } + @doc("Available states.") enum State { @doc(" ") /*ready*/Ready } + `, + ) + .toEmitDiagnostics(({ name, value, ready }) => + [name, value, ready].map((target) => ({ ...empty, ...location(target) })), + ); + }); + + it("accepts meaningful doc comments, surrounding whitespace, and punctuation-only content", async () => { + await tester + .expect( + ` + /** A configurable device available in the store. */ + model Widget { + /** The display name shown to users. */ + name: string; + } + @doc(" A display label. ") scalar Label extends string; + @doc(".") model Empty {} + `, ) .toBeValid(); }); +}); - it("does not require documentation on discriminator unions", async () => { +describe("uninformative documentation", () => { + it.each([ + ["/** wIdGeT. */ model Widget {}", "Widget", "wIdGeT."], + ['@doc("Label.") scalar Label extends string;', "Label", "Label."], + ['@doc("State") enum State { @doc("Available for use.") Ready }', "State", "State"], + [ + '@doc("State") union State { @doc("Available for use.") Ready: "ready", string }', + "State", + "State", + ], + ['@doc("description.") model Widget {}', "Widget", "description."], + ])("rejects repeated names or placeholder text: %s", async (source, name, doc) => { + await tester.expect(source).toEmitDiagnostics([uninformative(name, doc)]); + }); + + it("compares enum members with their authored names rather than serialized values", async () => { + await tester + .expect('@doc("Available states.") enum State { @doc("Ready.") Ready: "available" }') + .toEmitDiagnostics([uninformative("Ready", "Ready.")]); + await tester + .expect('@doc("Available states.") enum State { @doc("available") Ready: "available" }') + .toBeValid(); + }); + + it("checks named and unnamed string variants, with placeholder-only checks on other unnamed variants", async () => { await tester .expect( ` - union PetKind { - cat: "cat", - string, - } - - @discriminator("kind") - @doc("Base Pet model") - model Pet { - kind: PetKind; - } - - @doc("A Merry Ol' Cat") - model Cat extends Pet { - kind: PetKind.cat, - }`, + @doc("Available states.") union State { + @doc("Ready") Ready: "available", + @doc("ready") "ready", + @doc("description") int32, + @doc("string") string + } + `, ) + .toEmitDiagnostics([ + uninformative("Ready"), + uninformative("ready"), + uninformative("description"), + ]); + await tester + .expect('@doc("Available states.") union State { @doc("available") Ready: "available" }') .toBeValid(); }); - it("on union (non-discriminator)", async () => { + it("checks authored property names even when serialization renames them", async () => { + await tester + .expect( + ` + @doc("A widget resource.") model Widget { + @encodedName("application/json", "wire-name") + @doc("sourceName") /*source*/sourceName: string; + } + `, + ) + .toEmitDiagnostics(({ source }) => [{ ...uninformative("sourceName"), ...location(source) }]); + }); + + it("checks explicit names on version enums and discriminator unions and properties", async () => { await tester .expect( ` - union PetKind { - Cat: "Cat", - "Dog", - string, - }`, + @versioned(Widgets.Versions) namespace Widgets; + @doc("Versions") enum Versions { @doc("v1") v1: "2026-01-01" } + @doc("PetKind") union PetKind { @doc("cat") cat: "cat" } + @doc("A pet resource.") @discriminator("kind") model Pet { @doc("kind") kind: PetKind; } + `, ) .toEmitDiagnostics([ - { - code: "@azure-tools/typespec-azure-core/documentation-required", - message: - "The Union named 'PetKind' should have a documentation or description, use doc comment /** */ to provide it.", - }, - { - code: "@azure-tools/typespec-azure-core/documentation-required", - message: - "The UnionVariant named 'Cat' should have a documentation or description, use doc comment /** */ to provide it.", - }, - { - code: "@azure-tools/typespec-azure-core/documentation-required", - message: - "The UnionVariant named 'Dog' should have a documentation or description, use doc comment /** */ to provide it.", - }, + uninformative("kind"), + uninformative("PetKind"), + uninformative("cat"), + uninformative("Versions"), + uninformative("v1"), + ]); + }); + + it("targets both an offending model and its key property", async () => { + await tester + .expect("/** Widget */ model /*widget*/Widget { @key /** Name */ /*name*/name: string; }") + .toEmitDiagnostics(({ widget, name }) => [ + { ...uninformative("Widget"), ...location(widget) }, + { ...uninformative("name", "Name"), ...location(name) }, + ]); + }); +}); + +describe("HTTP documentation names", () => { + it("checks inferred GET and POST verbs while preserving explicitly declared verbs", async () => { + await tester + .expect( + ` + @route("/read") @doc("get") op /*read*/read(): string; + @route("/create") @doc("post") op /*create*/create(@doc("The payload.") @body body: string): string; + @route("/explicit") @get @doc("post") op fetch(): string; + `, + ) + .toEmitDiagnostics(({ read, create }) => [ + { ...uninformative("get"), ...location(read) }, + { ...uninformative("post"), ...location(create) }, + ]); + }); + + it("compares path, query, and header parameters with their HTTP names", async () => { + for (const kind of ["path", "query", "header"]) { + const route = kind === "path" ? '@route("/widgets/{wireName}")' : ""; + const operation = (doc: string) => + `${route} @get @doc("Reads a widget.") op read(@${kind}("wireName") @doc("${doc}") source: string): string;`; + await tester + .expect(operation("wireName.")) + .toEmitDiagnostics([uninformative("wireName", "wireName.")]); + await tester.expect(operation("source")).toBeValid(); + } + }); + + it("checks authored multipart names and placeholder text at the property", async () => { + const operation = (doc: string) => ` + @doc("Uploads text for processing.") op upload( + @doc("The request media type.") @header contentType: "multipart/form-data", + @doc("The submitted parts.") @multipartBody body: { + @doc("${doc}") /*part*/propName: HttpPart; + } + ): void; + `; + await tester + .expect(operation(" Part_Name. ")) + .toEmitDiagnostics(({ part }) => [ + { ...uninformative("part_name", " Part_Name. "), ...location(part) }, + ]); + await tester.expect(operation("propName")).toBeValid(); + await tester + .expect(operation("description.")) + .toEmitDiagnostics([uninformative("part_name", "description.")]); + }); + + it("uses source names for unnamed parts and authored names for repeated parts", async () => { + await tester + .expect( + ` + @doc("Uploads text for processing.") op upload( + @doc("The request media type.") @header contentType: "multipart/form-data", + @doc("The submitted parts.") @multipartBody body: { + @doc("text") /*text*/text: HttpPart; + @doc("part_name") /*repeated*/repeated: HttpPart[]; + } + ): void; + `, + ) + .toEmitDiagnostics(({ text, repeated }) => [ + { ...uninformative("text"), ...location(text) }, + { ...uninformative("part_name"), ...location(repeated) }, ]); }); + + it("exempts status-code names but still rejects explicitly blank documentation", async () => { + await tester + .expect( + ` + @doc("Response metadata.") model Response { + @statusCode @doc("status") status: 200; + } + `, + ) + .toBeValid(); + await tester + .expect('@doc("Response metadata.") model Response { @statusCode @doc(" ") status: 200; }') + .toEmitDiagnostics([empty]); + }); }); diff --git a/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts b/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts index 1e118368db..03c934af4d 100644 --- a/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts +++ b/packages/typespec-azure-rulesets/test/validate-rules-defined.test.ts @@ -90,6 +90,20 @@ describe("expect all rules to be defined", () => { } }); + it("enables the strengthened documentation-required rule without a separate quality rule", () => { + const ruleName = "@azure-tools/typespec-azure-core/documentation-required"; + for (const rulesetName of ["data-plane", "resource-manager"]) { + strictEqual($linter.ruleSets?.[rulesetName].enable?.[ruleName], true); + strictEqual( + $linter.ruleSets?.[rulesetName].enable?.[ + "@azure-tools/typespec-azure-core/no-uninformative-doc" + ], + undefined, + ); + } + ok(!AzureCoreLinter.rules.some((rule) => rule.name === "no-uninformative-doc")); + }); + it("enables consolidated ARM common-types version guidance", () => { const ruleset = $linter.ruleSets?.["resource-manager"]; ok(ruleset); diff --git a/website/src/content/docs/docs/libraries/azure-core/reference/linter.md b/website/src/content/docs/docs/libraries/azure-core/reference/linter.md index e3e8a7fb90..4902292022 100644 --- a/website/src/content/docs/docs/libraries/azure-core/reference/linter.md +++ b/website/src/content/docs/docs/libraries/azure-core/reference/linter.md @@ -52,7 +52,7 @@ Available ruleSets: | [`@azure-tools/typespec-azure-core/no-rest-library-interfaces`](../rules/no-rest-library-interfaces.md) | Resource interfaces from the TypeSpec.Rest.Resource library are incompatible with Azure.Core. | | [`@azure-tools/typespec-azure-core/no-unknown`](../rules/no-unknown.md) | Azure services must not have properties of type `unknown`. | | [`@azure-tools/typespec-azure-core/bad-record-type`](../rules/bad-record-type.md) | Identify bad record definitions. | -| [`@azure-tools/typespec-azure-core/documentation-required`](../rules/documentation-required.md) | Require documentation over enums, models, and operations. | +| [`@azure-tools/typespec-azure-core/documentation-required`](../rules/documentation-required.md) | Require meaningful documentation over enums, models, and operations. | | [`@azure-tools/typespec-azure-core/key-visibility-required`](../rules/key-visibility-required.md) | Key properties need to have a Lifecycle visibility setting. | | [`@azure-tools/typespec-azure-core/response-schema-problem`](../rules/response-schema-problem.md) | Warn about operations having multiple non-error response schemas. | | [`@azure-tools/typespec-azure-core/rpc-operation-request-body`](../rules/rpc-operation-request-body.md) | Warning for RPC body problems. |