Skip to content

Commit

Permalink
completed-docs: exclude object literal methods (palantir#3532)
Browse files Browse the repository at this point in the history 
[bugfix] `completed-docs`: don't require documentation on methods in object literals

Also simplifies some other checks.
  • Loading branch information
@ajafff @adidahiya
ajafff authored and adidahiya committed Dec 4, 2017
1 parent 42fe8ea commit 91f2d25
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 58 deletions.
62 changes: 21 additions & 41 deletions src/rules/completedDocsRule.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/

import { isVariableDeclarationList, isVariableStatement } from "tsutils";
import * as ts from "typescript";

import * as Lint from "../index";
Expand Down Expand Up @@ -319,12 +318,11 @@ function walk(context: Lint.WalkContext<ExclusionsMap>, typeChecker: ts.TypeChec

case ts.SyntaxKind.EnumDeclaration:
checkNode(node as ts.EnumDeclaration, ARGUMENT_ENUMS);
break;

case ts.SyntaxKind.EnumMember:
// Enum members don't have modifiers, so use the parent
// enum declaration when checking the requirements.
checkNode(node as ts.EnumMember, ARGUMENT_ENUM_MEMBERS, node.parent);
for (const member of (node as ts.EnumDeclaration).members) {
// Enum members don't have modifiers, so use the parent
// enum declaration when checking the requirements.
checkNode(member, ARGUMENT_ENUM_MEMBERS, node);
}
break;

case ts.SyntaxKind.FunctionDeclaration:
Expand All @@ -336,7 +334,9 @@ function walk(context: Lint.WalkContext<ExclusionsMap>, typeChecker: ts.TypeChec
break;

case ts.SyntaxKind.MethodDeclaration:
checkNode(node as ts.MethodDeclaration, ARGUMENT_METHODS);
if (node.parent!.kind !== ts.SyntaxKind.ObjectLiteralExpression) {
checkNode(node as ts.MethodDeclaration, ARGUMENT_METHODS);
}
break;

case ts.SyntaxKind.ModuleDeclaration:
Expand All @@ -351,13 +351,23 @@ function walk(context: Lint.WalkContext<ExclusionsMap>, typeChecker: ts.TypeChec
checkNode(node as ts.TypeAliasDeclaration, ARGUMENT_TYPES);
break;

case ts.SyntaxKind.VariableDeclaration:
checkVariable(node as ts.VariableDeclaration);
case ts.SyntaxKind.VariableStatement:
// Only check variables at the namespace/module-level or file-level
// and not variables declared inside functions and other things.
switch (node.parent!.kind) {
case ts.SyntaxKind.SourceFile:
case ts.SyntaxKind.ModuleBlock:
for (const declaration of (node as ts.VariableStatement).declarationList.declarations) {
checkNode(declaration, ARGUMENT_VARIABLES, node);
}
}
break;

case ts.SyntaxKind.GetAccessor:
case ts.SyntaxKind.SetAccessor:
checkAccessor(node as ts.NamedDeclaration);
if (node.parent!.kind !== ts.SyntaxKind.ObjectLiteralExpression) {
checkNode(node as ts.AccessorDeclaration, ARGUMENT_PROPERTIES);
}
}

return ts.forEachChild(node, cb);
Expand Down Expand Up @@ -389,36 +399,6 @@ function walk(context: Lint.WalkContext<ExclusionsMap>, typeChecker: ts.TypeChec
checkComments(node, describeNode(nodeType), comments, requirementNode);
}

function checkVariable(node: ts.VariableDeclaration) {
// Only check variables in variable declaration lists
// and not variables in catch clauses and for loops.
const list = node.parent!;
if (!isVariableDeclarationList(list)) {
return;
}

const statement = list.parent!;
if (!isVariableStatement(statement)) {
return;
}

// Only check variables at the namespace/module-level or file-level
// and not variables declared inside functions and other things.
switch (statement.parent!.kind) {
case ts.SyntaxKind.SourceFile:
case ts.SyntaxKind.ModuleBlock:
checkNode(node, ARGUMENT_VARIABLES, statement);
}
}

function checkAccessor(node: ts.NamedDeclaration): void {
// Properties in object literal expressions do not
// require documentation, so neither do accessors.
if (node.parent!.kind !== ts.SyntaxKind.ObjectLiteralExpression) {
checkNode(node, ARGUMENT_PROPERTIES);
}
}

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);
Expand Down
35 changes: 18 additions & 17 deletions test/rules/completed-docs/types/test.ts.lint
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,54 +50,54 @@ class BadClass {
}

/**
*
*
*/
class EmptyClass {
~~~~~~~~~~~~~~~~~~ [Documentation must exist for classes.]
/**
*
*
*/
emptyDefaultProperty;
~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for properties.]

/**
*
*
*/
public emptyPublicProperty;
~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for public properties.]

/**
*
*
*/
protected emptyProtectedProperty;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for protected properties.]

/**
*
*
*/
private emptyPrivateProperty;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for private properties.]

/**
*
*
*/
emptyDefaultMethod() { }
~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for methods.]

/**
*
*
*/
public emptyPublicMethod() { }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for public methods.]

/**
*
*
*/
protected emptyProtectedMethod() { }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for protected methods.]

/**
*
*
*/
private emptyPrivateMethod() { }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for private methods.]
Expand Down Expand Up @@ -240,7 +240,7 @@ enum BadEnum { }
~~~~~~~~~~~~~~~~ [Documentation must exist for enums.]

/**
*
*
*/
enum EmptyEnum { }
~~~~~~~~~~~~~~~~~~ [Documentation must exist for enums.]
Expand All @@ -267,7 +267,7 @@ function BadFunction() { }
~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for functions.]

/**
*
*
*/
function EmptyFunction() { }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for functions.]
Expand All @@ -281,7 +281,7 @@ interface BadInterface { }
~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for interfaces.]

/**
*
*
*/
interface EmptyInterface { }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for interfaces.]
Expand All @@ -295,7 +295,7 @@ namespace BadNamespace { }
~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for namespaces.]

/**
*
*
*/
namespace EmptyNamespace { }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for namespaces.]
Expand All @@ -309,7 +309,7 @@ type BadType = 1;
~~~~~~~~~~~~~~~~~ [Documentation must exist for types.]

/**
*
*
*/
type EmptyType = 1;
~~~~~~~~~~~~~~~~~~~ [Documentation must exist for types.]
Expand All @@ -323,7 +323,7 @@ type BadType = 1;
~~~~~~~~~~~~~~~~~ [Documentation must exist for types.]

/**
*
*
*/
type EmptyType = 1;
~~~~~~~~~~~~~~~~~~~ [Documentation must exist for types.]
Expand All @@ -337,7 +337,7 @@ const BadVariable = 1;
~~~~~~~~~~~~~~~ [Documentation must exist for variables.]

/**
*
*
*/
const EmptyVariable = 1;
~~~~~~~~~~~~~~~~~ [Documentation must exist for variables.]
Expand All @@ -348,10 +348,11 @@ const EmptyVariable = 1;
const GoodVariable = 1;

/**
* Properties and accessors in object literals should not require documentation.
* Properties, methods and accessors in object literals should not require documentation.
*/
let literal = {
prop: 1,
get accessor() { return literal.prop; },
set accessor(value) { literal.prop = value; }
someMethod() {}
}

0 comments on commit 91f2d25

Please sign in to comment.