diff --git a/CHANGELOG.md b/CHANGELOG.md index 03b9daaa6..83db0f377 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,10 +3,10 @@ ### Bug Fixes - Fixed missing comments on `@enum` style enum members defined in declaration files, #1880. -- Fixed `--validation.notDocumented` warnings for functions/methods, #1895. +- Fixed `--validation.notDocumented` warnings for functions/methods/type aliases, #1895, #1898. - Search results will no longer include random items when the search bar is empty, #1881. - Comments on overloaded constructors will now be detected in the same way that overloaded functions/methods are. -- Fixed `removeReflection` not completely removing reflections from the project. +- Fixed `removeReflection` not completely removing reflections from the project, #1898. ### Thanks! diff --git a/src/lib/converter/converter.ts b/src/lib/converter/converter.ts index a847c99cf..a2e154e11 100644 --- a/src/lib/converter/converter.ts +++ b/src/lib/converter/converter.ts @@ -202,6 +202,7 @@ export class Converter extends ChildableComponent< [ReflectionKind.Constructor]: [ts.SyntaxKind.Constructor], [ReflectionKind.Property]: [ ts.SyntaxKind.PropertyDeclaration, + ts.SyntaxKind.PropertyAssignment, ts.SyntaxKind.PropertySignature, ts.SyntaxKind.JSDocPropertyTag, ts.SyntaxKind.BinaryExpression, diff --git a/src/lib/validation/documentation.ts b/src/lib/validation/documentation.ts index 40c69efb8..722038861 100644 --- a/src/lib/validation/documentation.ts +++ b/src/lib/validation/documentation.ts @@ -3,7 +3,9 @@ import * as ts from "typescript"; import { DeclarationReflection, ProjectReflection, + Reflection, ReflectionKind, + ReflectionType, SignatureReflection, } from "../models"; import { Logger, normalizePath } from "../utils"; @@ -35,7 +37,30 @@ export function validateDocumentation( kinds = removeFlag(kinds, ReflectionKind.Accessor); } - for (const ref of project.getReflectionsByKind(kinds)) { + const toProcess = project.getReflectionsByKind(kinds); + const seen = new Set(); + + while (toProcess.length) { + const ref = toProcess.shift()!; + if (seen.has(ref)) continue; + seen.add(ref); + + if (ref instanceof DeclarationReflection) { + const signatures = + ref.type instanceof ReflectionType + ? ref.type.declaration.getNonIndexSignatures() + : ref.getNonIndexSignatures(); + + if (signatures.length) { + // We maybe used to have a comment, but the comment plugin has removed it. + // See CommentPlugin.onResolve. We've been asked to validate this reflection, + // (it's probably a type alias) so we should validate that signatures all have + // comments, but we shouldn't produce a warning here. + toProcess.push(...signatures); + continue; + } + } + let symbol = project.getSymbolFromReflection(ref); let index = 0; diff --git a/src/test/TestLogger.ts b/src/test/TestLogger.ts index a94359dbd..aa62239b5 100644 --- a/src/test/TestLogger.ts +++ b/src/test/TestLogger.ts @@ -1,4 +1,4 @@ -import { Logger, LogLevel } from "../lib/utils"; +import { Logger, LogLevel, removeIf } from "../lib/utils"; import { deepStrictEqual as equal, fail } from "assert"; const levelMap: Record = { @@ -11,6 +11,10 @@ const levelMap: Record = { export class TestLogger extends Logger { messages: string[] = []; + discardDebugMessages() { + removeIf(this.messages, (msg) => msg.startsWith("debug")); + } + expectMessage(message: string) { const index = this.messages.indexOf(message); if (index === -1) { @@ -27,6 +31,7 @@ export class TestLogger extends Logger { } override log(message: string, level: LogLevel): void { + super.log(message, level); this.messages.push(levelMap[level] + message); } } diff --git a/src/test/converter2.test.ts b/src/test/converter2.test.ts index 6b16a421d..d490ca495 100644 --- a/src/test/converter2.test.ts +++ b/src/test/converter2.test.ts @@ -9,6 +9,7 @@ import { getConverter2Base, getConverter2Program, } from "./programs"; +import { TestLogger } from "./TestLogger"; const base = getConverter2Base(); const app = getConverter2App(); @@ -16,7 +17,7 @@ const app = getConverter2App(); function runTest( title: string, entry: string, - check: (project: ProjectReflection) => void + check: (project: ProjectReflection, logger: TestLogger) => void ) { it(title, () => { const program = getConverter2Program(); @@ -33,6 +34,7 @@ function runTest( const sourceFile = program.getSourceFile(entryPoint); ok(sourceFile, `No source file found for ${entryPoint}`); + app.logger = new TestLogger(); const project = app.converter.convert([ { displayName: entry, @@ -40,7 +42,7 @@ function runTest( sourceFile, }, ]); - check(project); + check(project, app.logger as TestLogger); }); } diff --git a/src/test/converter2/issues/gh1898.ts b/src/test/converter2/issues/gh1898.ts new file mode 100644 index 000000000..593fd481f --- /dev/null +++ b/src/test/converter2/issues/gh1898.ts @@ -0,0 +1,34 @@ +/** I am documented, but the validator thinks otherwise. */ +export type FunctionType = (foo: string) => string; + +export type UnDocFn = (foo: string) => string; + +/** This docblock works fine. */ +export class ExampleClass { + /** This wrongly complains about not being documented */ + [Symbol.iterator]() { + return { + /** This also says it's not documented. */ + index: 0, + /** This one too. */ + next(): IteratorResult { + return { done: false, value: this.index++ }; + }, + }; + } + + /** + * @hidden + */ + [Symbol.asyncIterator]() { + return { + // This does _not_ complain, which is what I would expect. + index: 0, + // Even though it's not rendered in the final docs, it still complains + // that this isn't documented. + async next(): Promise> { + return { done: false, value: this.index++ }; + }, + }; + } +} diff --git a/src/test/issueTests.ts b/src/test/issueTests.ts index ebcabee78..7b5707450 100644 --- a/src/test/issueTests.ts +++ b/src/test/issueTests.ts @@ -10,6 +10,8 @@ import { CommentTag, UnionType, } from "../lib/models"; +import { getConverter2App } from "./programs"; +import type { TestLogger } from "./TestLogger"; function query(project: ProjectReflection, name: string) { const reflection = project.getChildByName(name); @@ -18,7 +20,7 @@ function query(project: ProjectReflection, name: string) { } export const issueTests: { - [issue: string]: (project: ProjectReflection) => void; + [issue: string]: (project: ProjectReflection, logger: TestLogger) => void; } = { gh869(project) { const classFoo = project.children?.find( @@ -344,4 +346,14 @@ export const issueTests: { const auto = query(project, "SomeEnum.AUTO"); ok(auto.hasComment(), "Missing @enum member comment"); }, + + gh1898(project, logger) { + const app = getConverter2App(); + app.validate(project); + logger.discardDebugMessages(); + logger.expectMessage( + "warn: UnDocFn.__type, defined at src/test/converter2/issues/gh1898.ts:4, does not have any documentation." + ); + logger.expectNoOtherMessages(); + }, }; diff --git a/src/test/programs.ts b/src/test/programs.ts index 61d0aa620..86e2bab30 100644 --- a/src/test/programs.ts +++ b/src/test/programs.ts @@ -58,6 +58,7 @@ export function getConverter2App() { excludeExternals: true, tsconfig: join(getConverter2Base(), "tsconfig.json"), plugin: [], + validation: true, }); converter2App.options.freeze(); }