From 489789eb2dac2401b94bdd7a5bc0fbe66fdd4dda Mon Sep 17 00:00:00 2001 From: Michael Withagen Date: Tue, 18 Jun 2019 11:43:08 +0200 Subject: [PATCH 1/5] Introduce new typedef rule option (variable-declaration-ignore-function) Ignores typedef requirement for vars for arrow and non arrow functions. Fixes: #2654 --- src/rules/typedefRule.ts | 50 ++++++++++++++----- .../test.ts.lint | 17 +++++++ .../tslint.json | 5 ++ 3 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 test/rules/typedef/variable-declaration-ignore-function/test.ts.lint create mode 100644 test/rules/typedef/variable-declaration-ignore-function/tslint.json diff --git a/src/rules/typedefRule.ts b/src/rules/typedefRule.ts index 778e3000309..2e80fec60bb 100644 --- a/src/rules/typedefRule.ts +++ b/src/rules/typedefRule.ts @@ -21,6 +21,7 @@ import * as ts from "typescript"; import * as Lint from "../index"; import { codeExamples } from "./code-examples/typedef.examples"; +import { getChildOfKind } from "tsutils"; interface Options { "call-signature"?: boolean; @@ -29,6 +30,7 @@ interface Options { "arrow-parameter"?: boolean; "property-declaration"?: boolean; "variable-declaration"?: boolean; + "variable-declaration-ignore-function"?: boolean; "member-variable-declaration"?: boolean; "object-destructuring"?: boolean; "array-destructuring"?: boolean; @@ -41,6 +43,7 @@ const OPTION_PARAMETER: Option = "parameter"; const OPTION_ARROW_PARAMETER: Option = "arrow-parameter"; const OPTION_PROPERTY_DECLARATION: Option = "property-declaration"; const OPTION_VARIABLE_DECLARATION: Option = "variable-declaration"; +const OPTION_VARIABLE_DECLARATION_IGNORE_FUNCTION: Option = "variable-declaration-ignore-function"; const OPTION_MEMBER_VARIABLE_DECLARATION: Option = "member-variable-declaration"; const OPTION_OBJECT_DESTRUCTURING: Option = "object-destructuring"; const OPTION_ARRAY_DESTRUCTURING: Option = "array-destructuring"; @@ -67,6 +70,7 @@ export class Rule extends Lint.Rules.AbstractRule { * \`"${OPTION_ARROW_PARAMETER}"\` checks type specifier of function parameters for arrow functions. * \`"${OPTION_PROPERTY_DECLARATION}"\` checks return types of interface properties. * \`"${OPTION_VARIABLE_DECLARATION}"\` checks non-binding variable declarations. + * \`"${OPTION_VARIABLE_DECLARATION_IGNORE_FUNCTION}"\` ignore variable declarations for non-arrow and arrow functions. * \`"${OPTION_MEMBER_VARIABLE_DECLARATION}"\` checks member variable declarations. * \`"${OPTION_OBJECT_DESTRUCTURING}"\` checks object destructuring declarations. * \`"${OPTION_ARRAY_DESTRUCTURING}"\` checks array destructuring declarations.`, @@ -81,13 +85,14 @@ export class Rule extends Lint.Rules.AbstractRule { OPTION_ARROW_PARAMETER, OPTION_PROPERTY_DECLARATION, OPTION_VARIABLE_DECLARATION, + OPTION_VARIABLE_DECLARATION_IGNORE_FUNCTION, OPTION_MEMBER_VARIABLE_DECLARATION, OPTION_OBJECT_DESTRUCTURING, OPTION_ARRAY_DESTRUCTURING, ], }, minLength: 0, - maxLength: 7, + maxLength: 10, }, optionExamples: [ [true, OPTION_CALL_SIGNATURE, OPTION_PARAMETER, OPTION_MEMBER_VARIABLE_DECLARATION], @@ -181,7 +186,9 @@ class TypedefWalker extends Lint.AbstractWalker { } } - private checkVariableDeclaration({ parent, name, type }: ts.VariableDeclaration): void { + private checkVariableDeclaration(node: ts.Node): void { + const { parent, name, type } = node; + // variable declarations should always have a grandparent, but check that to be on the safe side. // catch statements will be the parent of the variable declaration // for-in/for-of loops will be the gradparent of the variable declaration @@ -193,20 +200,39 @@ class TypedefWalker extends Lint.AbstractWalker { return; } - const option = (() => { - switch (name.kind) { - case ts.SyntaxKind.ObjectBindingPattern: - return "object-destructuring"; - case ts.SyntaxKind.ArrayBindingPattern: - return "array-destructuring"; - default: - return "variable-declaration"; - } - })(); + let option: Option; + + switch (name.kind) { + case ts.SyntaxKind.ObjectBindingPattern: + option = OPTION_OBJECT_DESTRUCTURING; + break; + + case ts.SyntaxKind.ArrayBindingPattern: + option = OPTION_ARRAY_DESTRUCTURING; + break; + + default: + option = OPTION_VARIABLE_DECLARATION; + } + + if (this.shouldIgnoreVariableDeclaration(node)) { + return; + } this.checkTypeAnnotation(option, name, type, name); } + private shouldIgnoreVariableDeclaration(node: ts.Node): boolean { + const ignoreFunctions: boolean = + this.options[OPTION_VARIABLE_DECLARATION_IGNORE_FUNCTION] === true; + + return !!( + ignoreFunctions && + (getChildOfKind(node, ts.SyntaxKind.ArrowFunction) || + getChildOfKind(node, ts.SyntaxKind.FunctionExpression)) + ); + } + private checkTypeAnnotation( option: Option, location: ts.Node | ts.NodeArray, diff --git a/test/rules/typedef/variable-declaration-ignore-function/test.ts.lint b/test/rules/typedef/variable-declaration-ignore-function/test.ts.lint new file mode 100644 index 00000000000..4f1ded514b8 --- /dev/null +++ b/test/rules/typedef/variable-declaration-ignore-function/test.ts.lint @@ -0,0 +1,17 @@ +// Should all pass +var foo = function(): void {}; +let foo = function(): void {}; +const foo = function(): void {}; + +var foo = (): void => {}; +let foo = (): void => {}; +const foo = (): void => {}; + +class Foo { + foo = (): void => {}; + foo = function(): void {}; +} + +// Should not throw any exception if we do decide to add types. +const foo: () => void = (): void => {}; +const foo: () => void = function(): void {}; diff --git a/test/rules/typedef/variable-declaration-ignore-function/tslint.json b/test/rules/typedef/variable-declaration-ignore-function/tslint.json new file mode 100644 index 00000000000..bb965d329b5 --- /dev/null +++ b/test/rules/typedef/variable-declaration-ignore-function/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "typedef": [true, "variable-declaration", "variable-declaration-ignore-function"] + } +} From 5053bf57633dc2a7c385794d5ec1b226161315a0 Mon Sep 17 00:00:00 2001 From: Michael Withagen Date: Tue, 18 Jun 2019 12:08:27 +0200 Subject: [PATCH 2/5] Fix linting issues --- src/rules/typedefRule.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/rules/typedefRule.ts b/src/rules/typedefRule.ts index 2e80fec60bb..f4b38d11cf5 100644 --- a/src/rules/typedefRule.ts +++ b/src/rules/typedefRule.ts @@ -21,7 +21,6 @@ import * as ts from "typescript"; import * as Lint from "../index"; import { codeExamples } from "./code-examples/typedef.examples"; -import { getChildOfKind } from "tsutils"; interface Options { "call-signature"?: boolean; @@ -187,7 +186,7 @@ class TypedefWalker extends Lint.AbstractWalker { } private checkVariableDeclaration(node: ts.Node): void { - const { parent, name, type } = node; + const { parent, name, type } = node as ts.VariableDeclaration; // variable declarations should always have a grandparent, but check that to be on the safe side. // catch statements will be the parent of the variable declaration @@ -226,10 +225,10 @@ class TypedefWalker extends Lint.AbstractWalker { const ignoreFunctions: boolean = this.options[OPTION_VARIABLE_DECLARATION_IGNORE_FUNCTION] === true; - return !!( + return ( ignoreFunctions && - (getChildOfKind(node, ts.SyntaxKind.ArrowFunction) || - getChildOfKind(node, ts.SyntaxKind.FunctionExpression)) + (utils.getChildOfKind(node, ts.SyntaxKind.ArrowFunction) !== undefined || + utils.getChildOfKind(node, ts.SyntaxKind.FunctionExpression) !== undefined) ); } From 30913706d9c2c8767dd8361aad29ec90bc803b5f Mon Sep 17 00:00:00 2001 From: Michael Withagen Date: Tue, 18 Jun 2019 12:38:22 +0200 Subject: [PATCH 3/5] Include ignore typedef for member var declaration --- src/rules/typedefRule.ts | 10 +++++++++- .../variable-declaration-ignore-function/tslint.json | 7 ++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/rules/typedefRule.ts b/src/rules/typedefRule.ts index f4b38d11cf5..1e939a5799b 100644 --- a/src/rules/typedefRule.ts +++ b/src/rules/typedefRule.ts @@ -157,7 +157,8 @@ class TypedefWalker extends Lint.AbstractWalker { } } - private checkParameter({ parent, name, type }: ts.ParameterDeclaration): void { + private checkParameter(node: ts.Node): void { + const { parent, name, type } = node as ts.ParameterDeclaration; const isArrowFunction = parent.kind === ts.SyntaxKind.ArrowFunction; const option = (() => { @@ -181,6 +182,13 @@ class TypedefWalker extends Lint.AbstractWalker { // If this is an arrow function, it doesn't need to have a typedef on the property declaration // as the typedefs can be on the function's parameters instead if (initializer === undefined || initializer.kind !== ts.SyntaxKind.ArrowFunction) { + if ( + this.options[OPTION_VARIABLE_DECLARATION_IGNORE_FUNCTION] === true && + initializer && + initializer.kind === ts.SyntaxKind.FunctionExpression + ) { + return; + } this.checkTypeAnnotation("member-variable-declaration", name, type, name); } } diff --git a/test/rules/typedef/variable-declaration-ignore-function/tslint.json b/test/rules/typedef/variable-declaration-ignore-function/tslint.json index bb965d329b5..f815790c07b 100644 --- a/test/rules/typedef/variable-declaration-ignore-function/tslint.json +++ b/test/rules/typedef/variable-declaration-ignore-function/tslint.json @@ -1,5 +1,10 @@ { "rules": { - "typedef": [true, "variable-declaration", "variable-declaration-ignore-function"] + "typedef": [ + true, + "variable-declaration", + "member-variable-declaration", + "variable-declaration-ignore-function" + ] } } From 718a1747b31ce8056a4fe75677170bdb4b26c3f0 Mon Sep 17 00:00:00 2001 From: Michael Withagen Date: Tue, 18 Jun 2019 13:17:15 +0200 Subject: [PATCH 4/5] Fix linter issue --- src/rules/typedefRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/typedefRule.ts b/src/rules/typedefRule.ts index 1e939a5799b..103efc1885c 100644 --- a/src/rules/typedefRule.ts +++ b/src/rules/typedefRule.ts @@ -184,7 +184,7 @@ class TypedefWalker extends Lint.AbstractWalker { if (initializer === undefined || initializer.kind !== ts.SyntaxKind.ArrowFunction) { if ( this.options[OPTION_VARIABLE_DECLARATION_IGNORE_FUNCTION] === true && - initializer && + initializer !== undefined && initializer.kind === ts.SyntaxKind.FunctionExpression ) { return; From 355786569ecfc9a61506753faa97d92c61e2dac8 Mon Sep 17 00:00:00 2001 From: Michael Withagen Date: Thu, 4 Jul 2019 10:44:37 +0200 Subject: [PATCH 5/5] Process PR comments, remove comments from test + add test cases --- .../variable-declaration-ignore-function/test.ts.lint | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/rules/typedef/variable-declaration-ignore-function/test.ts.lint b/test/rules/typedef/variable-declaration-ignore-function/test.ts.lint index 4f1ded514b8..d9f013a93a0 100644 --- a/test/rules/typedef/variable-declaration-ignore-function/test.ts.lint +++ b/test/rules/typedef/variable-declaration-ignore-function/test.ts.lint @@ -1,4 +1,3 @@ -// Should all pass var foo = function(): void {}; let foo = function(): void {}; const foo = function(): void {}; @@ -12,6 +11,13 @@ class Foo { foo = function(): void {}; } -// Should not throw any exception if we do decide to add types. const foo: () => void = (): void => {}; const foo: () => void = function(): void {}; + +var noTypeDef = 'Should still fail'; + ~~~~~~~~~ [expected variable-declaration: 'noTypeDef' to have a typedef] + +class NoTypeDef { + public noTypeDef = 'Should still fail'; + ~~~~~~~~~ [expected member-variable-declaration: 'noTypeDef' to have a typedef] +}