Skip to content

Commit

Permalink
Warn about declarations with multiple comments
Browse files Browse the repository at this point in the history
Resolves #1855
  • Loading branch information
Gerrit0 committed Feb 15, 2022
1 parent c6116de commit 848d2aa
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
32 changes: 30 additions & 2 deletions 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
Expand Down Expand Up @@ -66,12 +68,15 @@ const wantedKinds: Record<ReflectionKind, ts.SyntaxKind[]> = {

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)) {
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/lib/converter/comments/index.ts
Expand Up @@ -59,7 +59,7 @@ export function getComment(
logger: Logger
): Comment | undefined {
const comment = getCommentWithCache(
discoverComment(symbol, kind),
discoverComment(symbol, kind, logger),
config,
logger
);
Expand Down
26 changes: 26 additions & 0 deletions src/test/TestLogger.ts
@@ -0,0 +1,26 @@
import { Logger, LogLevel } from "../lib/utils";
import { fail } from "assert/strict";

const levelMap: Record<LogLevel, string> = {
[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);
}
}
20 changes: 18 additions & 2 deletions src/test/behaviorTests.ts
Expand Up @@ -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);
Expand All @@ -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");
Expand Down Expand Up @@ -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) =>
Expand Down
7 changes: 5 additions & 2 deletions src/test/converter2.test.ts
Expand Up @@ -9,14 +9,15 @@ import {
getConverter2Base,
getConverter2Program,
} from "./programs";
import { TestLogger } from "./TestLogger";

const base = getConverter2Base();
const app = getConverter2App();

function runTest(
title: string,
entry: string,
check: (project: ProjectReflection) => void
check: (project: ProjectReflection, logger: TestLogger) => void
) {
it(title, () => {
const program = getConverter2Program();
Expand All @@ -33,14 +34,16 @@ 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,
program,
sourceFile,
},
]);
check(project);
check(project, logger);
});
}

Expand Down
8 changes: 8 additions & 0 deletions 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 {}

0 comments on commit 848d2aa

Please sign in to comment.