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
130 changes: 101 additions & 29 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 @@ -77,7 +78,7 @@ export type Privacy =

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 @@ -267,17 +268,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(
Expand All @@ -295,7 +295,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 @@ -363,7 +363,7 @@ 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);
}
}

Expand All @@ -375,45 +375,58 @@ function walk(context: Lint.WalkContext<ExclusionsMap>, typeChecker: ts.TypeChec
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 correspondingAccessor = getCorrespondingAccessor(node);

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(typeChecker);
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(
Expand All @@ -429,6 +442,57 @@ function walk(context: Lint.WalkContext<ExclusionsMap>, typeChecker: ts.TypeChec
}
}

function getCorrespondingAccessor(node: ts.AccessorDeclaration) {
const propertyName = tsutils.getPropertyName(node.name);
if (propertyName === undefined) {
return undefined;
}

const parent = node.parent as ts.ClassDeclaration | ts.ClassExpression;
const correspondingKindCheck =
node.kind === ts.SyntaxKind.GetAccessor ? isSetAccessor : isGetAccessor;

for (const member of parent.members) {
if (!correspondingKindCheck(member)) {
continue;
}

if (tsutils.getPropertyName(member.name) === propertyName) {
return member;
}
}

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 All @@ -450,3 +514,11 @@ function describeModifier(kind: ts.SyntaxKind) {
function describeNode(nodeType: DocType): string {
return nodeType.replace("-", " ");
}

function isGetAccessor(node: ts.Node): node is ts.GetAccessorDeclaration {
return node.kind === ts.SyntaxKind.GetAccessor;
}

function isSetAccessor(node: ts.Node): node is ts.SetAccessorDeclaration {
return node.kind === ts.SyntaxKind.SetAccessor;
}
122 changes: 120 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 Expand Up @@ -116,3 +116,121 @@ export class OnlyHasSetAccessorWithNoComments {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for properties.]

}

export class BothAccessorWithDifferentNamesNoComments {

/**
* ...
*/
private myField: string;

get myGetAccessor() { return this.myField; }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for properties.]

set mySetAccessor(value: string) { this.myField = value; }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for properties.]

}

const computedName = "myComputedName";

export class ComputedNamesWithCommentsOnGetOnly {

/**
* ...
*/
private myField: string;

/**
* ...
*/
get [computedName](): string { return this.myField; }

set [computedName](value: string) { this.myField = value; }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for properties.]

}

export class ComputedNamesWithCommentsOnSetOnly {

/**
* ...
*/
private myField: string;

get [computedName](): string { return this.myField; }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for properties.]

/**
* ...
*/
set [computedName](value: string) { this.myField = value; }

}

export class OnlyComputedGetAccessorNameWithNoComments {

/**
* ...
*/
private myField: string;

get [computedName](): string { return this.myField; }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for properties.]

}

export class OnlyComputedGetAccessorNameWithComments {

/**
* ...
*/
private myField: string;

/**
* ...
*/
get [computedName](): string { return this.myField; }

}

export class OnlyComputedSetAccessorNameWithNoComments {

/**
* ...
*/
private myField: string;

set [computedName](value: string) { this.myField = value; }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for properties.]

}

export class OnlyComputedSetAccessorNameWithComments {

/**
* ...
*/
private myField: string;

/**
* ...
*/
set [computedName](value: string) { this.myField = value; }

}

export class SameComputedNamesWithNoComments {

/**
* ...
*/
private myField: string;

get [computedName](): string { return this.myField; }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for properties.]

set [computedName](value: string) { this.myField = value; }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for properties.]

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

// TypeScript API doesn't give a symbol for this before version 2.8.0
// https://github.com/Microsoft/TypeScript/issues/14257
[Symbol.iterator]() {}
#if typescript >= 2.8.0
~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for methods.]
#endif
}

export enum Ddd { }
Expand Down
13 changes: 13 additions & 0 deletions test/rules/completed-docs/tags/content/test.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,20 @@ const ContentSingleLineValidVariable = 4;
/**
* ...
*/
const CommentBodyVariableDots = 5;

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

/** content */
const CommentBodyVariableSingleLineContent = 7;

/** ...content */
const CommentBodyVariableSingleLineDotsAndContent = 8;
const CommentBodyVariableRofl = 5;
~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for variables.]

/** ... */
const CommentBodyVariableRofl = 5;
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