Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Removed use of type checker from completed-docs #3557

Merged
merged 11 commits into from
Feb 23, 2019
99 changes: 80 additions & 19 deletions src/rules/completedDocsRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* limitations under the License.
*/

import * as tsutils from "tsutils";
import * as ts from "typescript";

import * as Lint from "../index";
Expand Down Expand Up @@ -79,7 +80,7 @@ export type Visibility = All
| typeof VISIBILITY_EXPORTED
| typeof VISIBILITY_INTERNAL;

export class Rule extends Lint.Rules.TypedRule {
export class Rule extends Lint.Rules.AbstractRule {
public static FAILURE_STRING_EXIST = "Documentation must exist for ";

public static defaultArguments: IInputExclusionDescriptors = {
Expand Down Expand Up @@ -281,17 +282,16 @@ export class Rule extends Lint.Rules.TypedRule {

type: "style",
typescriptOnly: false,
requiresTypeInfo: true,
};
/* tslint:enable:object-literal-sort-keys */

private readonly exclusionFactory = new ExclusionFactory();

public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
const options = this.getOptions();
const exclusionsMap = this.getExclusionsMap(options.ruleArguments);

return this.applyWithFunction(sourceFile, walk, exclusionsMap, program.getTypeChecker());
return this.applyWithFunction(sourceFile, walk, exclusionsMap);
}

private getExclusionsMap(ruleArguments: Array<DocType | IInputExclusionDescriptors>): ExclusionsMap {
Expand All @@ -307,7 +307,7 @@ const modifierAliases: { [i: string]: string } = {
export: "exported",
};

function walk(context: Lint.WalkContext<ExclusionsMap>, typeChecker: ts.TypeChecker) {
function walk(context: Lint.WalkContext<ExclusionsMap>) {
return ts.forEachChild(context.sourceFile, cb);

function cb(node: ts.Node): void {
Expand Down Expand Up @@ -366,43 +366,66 @@ function walk(context: Lint.WalkContext<ExclusionsMap>, typeChecker: ts.TypeChec
case ts.SyntaxKind.GetAccessor:
case ts.SyntaxKind.SetAccessor:
if (node.parent!.kind !== ts.SyntaxKind.ObjectLiteralExpression) {
checkNode(node as ts.AccessorDeclaration, ARGUMENT_PROPERTIES);
checkAccessorNode(node as ts.AccessorDeclaration);
}
}

return ts.forEachChild(node, cb);
}

function checkNode(node: ts.NamedDeclaration, nodeType: DocType, requirementNode: ts.Node = node): void {
if (!nodeIsExcluded(node, nodeType, requirementNode) && !nodeHasDocs(node)) {
addDocumentationFailure(node, describeNode(nodeType), requirementNode);
}
}

function checkAccessorNode(node: ts.AccessorDeclaration): void {
if (nodeIsExcluded(node, ARGUMENT_PROPERTIES, node) || nodeHasDocs(node)) {
return;
}

const parent = node.parent as ts.ClassDeclaration | ts.ClassExpression;
const correspondingKind = node.kind === ts.SyntaxKind.GetAccessor
? ts.SyntaxKind.SetAccessor
: ts.SyntaxKind.GetAccessor;
const correspondingAccessor = getNodeOfKind(parent.members, correspondingKind);
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

if (correspondingAccessor === undefined || !nodeHasDocs(correspondingAccessor)) {
addDocumentationFailure(node, ARGUMENT_PROPERTIES, node);
}
}

function nodeIsExcluded(node: ts.NamedDeclaration, nodeType: DocType, requirementNode: ts.Node): boolean {
const { name } = node;
if (name === undefined) {
return;
return true;
}

const exclusions = context.options.get(nodeType);
if (exclusions === undefined) {
return;
return true;
}

for (const exclusion of exclusions) {
if (exclusion.excludes(requirementNode)) {
return;
return true;
}
}

const symbol = typeChecker.getSymbolAtLocation(name);
if (symbol === undefined) {
return;
}

const comments = symbol.getDocumentationComment();
checkComments(node, describeNode(nodeType), comments, requirementNode);
return false;
}

function checkComments(node: ts.Node, nodeDescriptor: string, comments: ts.SymbolDisplayPart[], requirementNode: ts.Node) {
if (comments.map((comment: ts.SymbolDisplayPart) => comment.text).join("").trim() === "") {
addDocumentationFailure(node, nodeDescriptor, requirementNode);
function nodeHasDocs(node: ts.Node): boolean {
const docs = getApparentJsDoc(node);
if (docs === undefined) {
return false;
}

const comments = docs
.map((doc) => doc.comment)
.filter((comment) => comment !== undefined && comment.trim() !== "");

return comments.length !== 0;
}

function addDocumentationFailure(node: ts.Node, nodeType: string, requirementNode: ts.Node): void {
Expand All @@ -414,6 +437,44 @@ function walk(context: Lint.WalkContext<ExclusionsMap>, typeChecker: ts.TypeChec
}
}

function getNodeOfKind(nodes: ts.NodeArray<ts.Node>, kind: ts.SyntaxKind) {
for (const node of nodes) {
if (node.kind === kind) {
return node;
}
}

return undefined;
}

/**
* @remarks See https://github.com/ajafff/tsutils/issues/16
*/
function getApparentJsDoc(node: ts.Node): ts.JSDoc[] | undefined {
if (ts.isVariableDeclaration(node)) {
if (variableIsAfterFirstInDeclarationList(node)) {
return undefined;
}

node = node.parent!;
}

if (ts.isVariableDeclarationList(node)) {
node = node.parent!;
}

return tsutils.getJsDoc(node);
}

function variableIsAfterFirstInDeclarationList(node: ts.VariableDeclaration): boolean {
const parent = node.parent;
if (parent === undefined) {
return false;
}

return ts.isVariableDeclarationList(parent) && node !== parent.declarations[0];
}

function describeDocumentationFailure(node: ts.Node, nodeType: string): string {
let description = Rule.FAILURE_STRING_EXIST;

Expand Down
4 changes: 2 additions & 2 deletions test/rules/completed-docs/accessors/test.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class CommentsOnGetOnly {
*/
get myAccessor(): string { return this.myField; }

set myAccessor(value: string) { this.myField = value; } // Comments for the setter are inherited from the getter.
set myAccessor(value: string) { this.myField = value; }

}

Expand All @@ -56,7 +56,7 @@ export class CommentsOnSetOnly {
*/
private myField: string;

get myAccessor(): string { return this.myField; } // Comments from the getter are inherited from the setter.
get myAccessor(): string { return this.myField; }

/**
* The set accessor.
Expand Down
3 changes: 1 addition & 2 deletions test/rules/completed-docs/defaults/test.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ export class Aaa {
public set prop(value) { this.bbb = value; }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for public properties.]

// TODO: TypeScript API doesn't give us a symbol for this, so we must ignore it.
// https://github.com/Microsoft/TypeScript/issues/14257
[Symbol.iterator]() {}
~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for methods.]
}

export enum Ddd { }
Expand Down
13 changes: 12 additions & 1 deletion test/rules/completed-docs/tags/content/test.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,15 @@ const ContentValidVariable = 4;
/**
* ...
*/
const CommentBodyVariableRofl = 5;
const CommentBodyVariableDots = 5;

/**
* ...content
*/
const CommentBodyVariableDotsAndContent = 6;

/** content */
const CommentBodyVariableSingleLineContent = 7;

/** ...content */
const CommentBodyVariableSingleLineDotsAndContent = 8;
10 changes: 10 additions & 0 deletions test/rules/completed-docs/variables/test.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ export const exportedVariable = 1;
const internalVariable = 2;
~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for variables.]

/**
* (documentation here will be read)
*/
const firstVariableInList = true,
/**
* (documentation here will not be read)
*/
secondVariableInList = false;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for variables.]

// Variables in `catch` clauses should not be checked.
try {
} catch (ex) {
Expand Down