From 848d2aa4cd9a2fb0027ae2e8c204bfd1077aeff9 Mon Sep 17 00:00:00 2001 From: Gerrit Birkeland Date: Mon, 14 Feb 2022 20:25:18 -0700 Subject: [PATCH] Warn about declarations with multiple comments Resolves #1855 --- CHANGELOG.md | 1 + src/lib/converter/comments/discovery.ts | 32 +++++++++++++++++-- src/lib/converter/comments/index.ts | 2 +- src/test/TestLogger.ts | 26 +++++++++++++++ src/test/behaviorTests.ts | 20 ++++++++++-- src/test/converter2.test.ts | 7 ++-- .../converter2/behavior/mergedDeclarations.ts | 8 +++++ 7 files changed, 89 insertions(+), 7 deletions(-) create mode 100644 src/test/TestLogger.ts create mode 100644 src/test/converter2/behavior/mergedDeclarations.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f50c88131..f44c0736e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ ### Features - TypeDoc will now search for `typedoc.js(on)` in the `.config` folder in the current working directory. +- If an exported symbol has multiple declarations, TypeDoc will now check all appropriate declarations for comments, and warn if more than one declaration contains a comment, #1855. ### Bug Fixes diff --git a/src/lib/converter/comments/discovery.ts b/src/lib/converter/comments/discovery.ts index 2c5a7dcc1..2b91d56fa 100644 --- a/src/lib/converter/comments/discovery.ts +++ b/src/lib/converter/comments/discovery.ts @@ -1,5 +1,7 @@ import * as ts from "typescript"; import { ReflectionKind } from "../../models"; +import type { Logger } from "../../utils"; +import { nicePath } from "../../utils/paths"; // Note: This does NOT include JSDoc syntax kinds. This is important! // Comments from @typedef and @callback tags are handled specially by @@ -66,12 +68,15 @@ const wantedKinds: Record = { export function discoverComment( symbol: ts.Symbol, - kind: ReflectionKind + kind: ReflectionKind, + logger: Logger ): [ts.SourceFile, ts.CommentRange] | undefined { // For a module comment, we want the first one defined in the file, // not the last one, since that will apply to the import or declaration. const reverse = symbol.declarations?.some(ts.isSourceFile); + const discovered: [ts.SourceFile, ts.CommentRange][] = []; + for (const decl of symbol.declarations || []) { const text = decl.getSourceFile().text; if (wantedKinds[kind].includes(decl.kind)) { @@ -104,10 +109,33 @@ export function discoverComment( ); if (lastDocComment) { - return [decl.getSourceFile(), lastDocComment]; + discovered.push([decl.getSourceFile(), lastDocComment]); } } } + + switch (discovered.length) { + case 0: + return undefined; + case 1: + return discovered[0]; + default: { + logger.warn( + `${symbol.name} has multiple declarations with a comment. An arbitrary comment will be used.` + ); + const locations = discovered.map(([sf, { pos }]) => { + const path = nicePath(sf.fileName); + const line = ts.getLineAndCharacterOfPosition(sf, pos).line + 1; + return `${path}:${line}`; + }); + logger.info( + `The comments for ${ + symbol.name + } are declared at:\n\t${locations.join("\n\t")}` + ); + return discovered[0]; + } + } } export function discoverSignatureComment( diff --git a/src/lib/converter/comments/index.ts b/src/lib/converter/comments/index.ts index e7133ea2d..ecef6a73c 100644 --- a/src/lib/converter/comments/index.ts +++ b/src/lib/converter/comments/index.ts @@ -59,7 +59,7 @@ export function getComment( logger: Logger ): Comment | undefined { const comment = getCommentWithCache( - discoverComment(symbol, kind), + discoverComment(symbol, kind, logger), config, logger ); diff --git a/src/test/TestLogger.ts b/src/test/TestLogger.ts new file mode 100644 index 000000000..a572e19b7 --- /dev/null +++ b/src/test/TestLogger.ts @@ -0,0 +1,26 @@ +import { Logger, LogLevel } from "../lib/utils"; +import { fail } from "assert/strict"; + +const levelMap: Record = { + [LogLevel.Error]: "error: ", + [LogLevel.Warn]: "warn: ", + [LogLevel.Info]: "info: ", + [LogLevel.Verbose]: "debug: ", +}; + +export class TestLogger extends Logger { + messages: string[] = []; + + expectMessage(message: string) { + if (!this.messages.includes(message)) { + const messages = this.messages.join("\n\t"); + fail( + `Expected "${message}" to be logged. The logged messages were:\n\t${messages}` + ); + } + } + + override log(message: string, level: LogLevel): void { + this.messages.push(levelMap[level] + message); + } +} diff --git a/src/test/behaviorTests.ts b/src/test/behaviorTests.ts index b6efb82de..0017dec88 100644 --- a/src/test/behaviorTests.ts +++ b/src/test/behaviorTests.ts @@ -5,6 +5,7 @@ import { ReflectionKind, Comment, } from "../lib/models"; +import type { TestLogger } from "./TestLogger"; function query(project: ProjectReflection, name: string) { const reflection = project.getChildByName(name); @@ -14,7 +15,7 @@ function query(project: ProjectReflection, name: string) { export const behaviorTests: Record< string, - (project: ProjectReflection) => void + (project: ProjectReflection, logger: TestLogger) => void > = { asConstEnum(project) { const SomeEnumLike = query(project, "SomeEnumLike"); @@ -55,7 +56,22 @@ export const behaviorTests: Record< 'Record<"b", 1>', ]); }, - // Disabled for now, pending https://github.com/TypeStrong/typedoc/issues/1809 + + mergedDeclarations(project, logger) { + const a = query(project, "SingleCommentMultiDeclaration"); + equal( + Comment.combineDisplayParts(a.comment?.summary), + "Comment on second declaration" + ); + + const b = query(project, "MultiCommentMultiDeclaration"); + equal(Comment.combineDisplayParts(b.comment?.summary), "Comment 1"); + + logger.expectMessage( + "warn: MultiCommentMultiDeclaration has multiple declarations with a comment. An arbitrary comment will be used." + ); + }, + overloads(project) { const foo = query(project, "foo"); const fooComments = foo.signatures?.map((sig) => diff --git a/src/test/converter2.test.ts b/src/test/converter2.test.ts index ecfae11be..863482846 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,8 @@ function runTest( const sourceFile = program.getSourceFile(entryPoint); ok(sourceFile, `No source file found for ${entryPoint}`); + const logger = new TestLogger(); + app.logger = logger; const project = app.converter.convert([ { displayName: entry, @@ -40,7 +43,7 @@ function runTest( sourceFile, }, ]); - check(project); + check(project, logger); }); } diff --git a/src/test/converter2/behavior/mergedDeclarations.ts b/src/test/converter2/behavior/mergedDeclarations.ts new file mode 100644 index 000000000..cad2337ea --- /dev/null +++ b/src/test/converter2/behavior/mergedDeclarations.ts @@ -0,0 +1,8 @@ +export namespace SingleCommentMultiDeclaration {} +/** Comment on second declaration */ +export namespace SingleCommentMultiDeclaration {} + +/** Comment 1 */ +export namespace MultiCommentMultiDeclaration {} +/** Comment 2 */ +export namespace MultiCommentMultiDeclaration {}