Skip to content

Commit

Permalink
Even more fixes for documentation checks
Browse files Browse the repository at this point in the history
Resolves #1898
  • Loading branch information
Gerrit0 committed Mar 27, 2022
1 parent bc6130a commit ea16a7b
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 7 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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!

Expand Down
1 change: 1 addition & 0 deletions src/lib/converter/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 26 additions & 1 deletion src/lib/validation/documentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import * as ts from "typescript";
import {
DeclarationReflection,
ProjectReflection,
Reflection,
ReflectionKind,
ReflectionType,
SignatureReflection,
} from "../models";
import { Logger, normalizePath } from "../utils";
Expand Down Expand Up @@ -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<Reflection>();

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;

Expand Down
7 changes: 6 additions & 1 deletion src/test/TestLogger.ts
Original file line number Diff line number Diff line change
@@ -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<LogLevel, string> = {
Expand All @@ -11,6 +11,10 @@ const levelMap: Record<LogLevel, string> = {
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) {
Expand All @@ -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);
}
}
6 changes: 4 additions & 2 deletions src/test/converter2.test.ts
Original file line number Diff line number Diff line change
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,15 @@ 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,
program,
sourceFile,
},
]);
check(project);
check(project, app.logger as TestLogger);
});
}

Expand Down
34 changes: 34 additions & 0 deletions src/test/converter2/issues/gh1898.ts
Original file line number Diff line number Diff line change
@@ -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<number> {
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<IteratorResult<number>> {
return { done: false, value: this.index++ };
},
};
}
}
14 changes: 13 additions & 1 deletion src/test/issueTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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(
Expand Down Expand Up @@ -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();
},
};
1 change: 1 addition & 0 deletions src/test/programs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export function getConverter2App() {
excludeExternals: true,
tsconfig: join(getConverter2Base(), "tsconfig.json"),
plugin: [],
validation: true,
});
converter2App.options.freeze();
}
Expand Down

0 comments on commit ea16a7b

Please sign in to comment.