From f53ec359b7d95795f1da58463b73fc4987bbf554 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 24 Jan 2017 21:02:44 -0800 Subject: [PATCH] Rename `no-let-undefined` to `no-unnecessary-initializer` (#2106) * Rename `no-let-undefined` to `no-unnecessary-initializer` * And handle 'var', destructuring, and parameter initializers * fix for parameter that can't be made optional * Fail for *any* parameter with an unnecessary 'undefined' initializer * Fix typo --- src/rules/noLetUndefinedRule.ts | 57 --------- src/rules/noUnnecessaryInitializerRule.ts | 121 ++++++++++++++++++ test/rules/no-let-undefined/test.ts.fix | 10 -- test/rules/no-let-undefined/test.ts.lint | 13 -- test/rules/no-let-undefined/tslint.json | 8 -- .../test.js.lint | 0 .../no-unnecessary-initializer/test.ts.fix | 16 +++ .../no-unnecessary-initializer/test.ts.lint | 25 ++++ .../no-unnecessary-initializer/tslint.json | 8 ++ 9 files changed, 170 insertions(+), 88 deletions(-) delete mode 100644 src/rules/noLetUndefinedRule.ts create mode 100644 src/rules/noUnnecessaryInitializerRule.ts delete mode 100644 test/rules/no-let-undefined/test.ts.fix delete mode 100644 test/rules/no-let-undefined/test.ts.lint delete mode 100644 test/rules/no-let-undefined/tslint.json rename test/rules/{no-let-undefined => no-unnecessary-initializer}/test.js.lint (100%) create mode 100644 test/rules/no-unnecessary-initializer/test.ts.fix create mode 100644 test/rules/no-unnecessary-initializer/test.ts.lint create mode 100644 test/rules/no-unnecessary-initializer/tslint.json diff --git a/src/rules/noLetUndefinedRule.ts b/src/rules/noLetUndefinedRule.ts deleted file mode 100644 index beb2d1f3fb6..00000000000 --- a/src/rules/noLetUndefinedRule.ts +++ /dev/null @@ -1,57 +0,0 @@ -/** - * @license - * Copyright 2017 Palantir Technologies, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import * as ts from "typescript"; - -import * as Lint from "../index"; - -export class Rule extends Lint.Rules.AbstractRule { - /* tslint:disable:object-literal-sort-keys */ - public static metadata: Lint.IRuleMetadata = { - ruleName: "no-let-undefined", - description: "Forbids a 'let' statement to be initialized to 'undefined'.", - hasFix: true, - optionsDescription: "Not configurable.", - options: null, - optionExamples: ["true"], - type: "style", - typescriptOnly: false, - }; - /* tslint:enable:object-literal-sort-keys */ - - public static FAILURE_STRING = "Unnecessary initialization to 'undefined'."; - - public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new Walker(sourceFile, this.getOptions())); - } -} - -class Walker extends Lint.RuleWalker { - public visitVariableDeclaration(node: ts.VariableDeclaration) { - if (Lint.isNodeFlagSet(node.parent!, ts.NodeFlags.Let) && isUndefined(node.initializer)) { - const fix = this.createFix(this.deleteFromTo( - Lint.childOfKind(node, ts.SyntaxKind.EqualsToken)!.pos, - node.end)); - this.addFailureAtNode(node, Rule.FAILURE_STRING, fix); - } - super.visitVariableDeclaration(node); - } -} - -function isUndefined(node: ts.Node | undefined): boolean { - return node !== undefined && node.kind === ts.SyntaxKind.Identifier && (node as ts.Identifier).text === "undefined"; -} diff --git a/src/rules/noUnnecessaryInitializerRule.ts b/src/rules/noUnnecessaryInitializerRule.ts new file mode 100644 index 00000000000..f722b3f1ed3 --- /dev/null +++ b/src/rules/noUnnecessaryInitializerRule.ts @@ -0,0 +1,121 @@ +/** + * @license + * Copyright 2017 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as ts from "typescript"; + +import * as Lint from "../index"; + +export class Rule extends Lint.Rules.AbstractRule { + /* tslint:disable:object-literal-sort-keys */ + public static metadata: Lint.IRuleMetadata = { + ruleName: "no-unnecessary-initializer", + description: "Forbids a 'var'/'let' statement or destructuring initializer to be initialized to 'undefined'.", + hasFix: true, + optionsDescription: "Not configurable.", + options: null, + optionExamples: ["true"], + type: "style", + typescriptOnly: false, + }; + /* tslint:enable:object-literal-sort-keys */ + + public static FAILURE_STRING = "Unnecessary initialization to 'undefined'."; + public static FAILURE_STRING_PARAMETER = + "Use an optional parameter instead of initializing to 'undefined'. " + + "Also, the type declaration does not need to include '| undefined'."; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithWalker(new Walker(sourceFile, this.getOptions())); + } +} + +class Walker extends Lint.RuleWalker { + public visitVariableDeclaration(node: ts.VariableDeclaration) { + if (isBindingPattern(node.name)) { + for (const elem of node.name.elements) { + if (elem.kind === ts.SyntaxKind.BindingElement) { + this.checkInitializer(elem); + } + } + } else if (!Lint.isNodeFlagSet(node.parent!, ts.NodeFlags.Const)) { + this.checkInitializer(node); + } + + super.visitVariableDeclaration(node); + } + + public visitMethodDeclaration(node: ts.MethodDeclaration) { + this.checkSignature(node); + super.visitMethodDeclaration(node); + } + public visitFunctionDeclaration(node: ts.FunctionDeclaration) { + this.checkSignature(node); + super.visitFunctionDeclaration(node); + } + public visitConstructorDeclaration(node: ts.ConstructorDeclaration) { + this.checkSignature(node); + super.visitConstructorDeclaration(node); + } + + private checkSignature({ parameters }: ts.MethodDeclaration | ts.FunctionDeclaration | ts.ConstructorDeclaration) { + parameters.forEach((parameter, i) => { + if (isUndefined(parameter.initializer)) { + if (parametersAllOptionalAfter(parameters, i)) { + // No fix since they may want to remove '| undefined' from the type. + this.addFailureAtNode(parameter, Rule.FAILURE_STRING_PARAMETER); + } else { + this.failWithFix(parameter); + } + } + }); + } + + private checkInitializer(node: ts.VariableDeclaration | ts.BindingElement) { + if (isUndefined(node.initializer)) { + this.failWithFix(node); + } + } + + private failWithFix(node: ts.VariableDeclaration | ts.BindingElement | ts.ParameterDeclaration) { + const fix = this.createFix(this.deleteFromTo( + Lint.childOfKind(node, ts.SyntaxKind.EqualsToken)!.pos, + node.end)); + this.addFailureAtNode(node, Rule.FAILURE_STRING, fix); + } +} + +function parametersAllOptionalAfter(parameters: ts.ParameterDeclaration[], idx: number): boolean { + for (let i = idx + 1; i < parameters.length; i++) { + if (parameters[i].questionToken) { + return true; + } + if (!parameters[i].initializer) { + return false; + } + } + return true; +} + +function isUndefined(node: ts.Node | undefined): boolean { + return node !== undefined && + node.kind === ts.SyntaxKind.Identifier && + (node as ts.Identifier).originalKeywordKind === ts.SyntaxKind.UndefinedKeyword; +} + +function isBindingPattern(node: ts.Node): node is ts.ArrayBindingPattern | ts.ObjectBindingPattern { + return node.kind === ts.SyntaxKind.ArrayBindingPattern || node.kind === ts.SyntaxKind.ObjectBindingPattern; +} diff --git a/test/rules/no-let-undefined/test.ts.fix b/test/rules/no-let-undefined/test.ts.fix deleted file mode 100644 index 6a9080a8d3b..00000000000 --- a/test/rules/no-let-undefined/test.ts.fix +++ /dev/null @@ -1,10 +0,0 @@ -let x: string | undefined; - -for (let y: number | undefined; y < 2; y++) {} - -let z; - -const x = undefined; - -function f(x: string | undefined = undefined) {} - diff --git a/test/rules/no-let-undefined/test.ts.lint b/test/rules/no-let-undefined/test.ts.lint deleted file mode 100644 index 676e899f224..00000000000 --- a/test/rules/no-let-undefined/test.ts.lint +++ /dev/null @@ -1,13 +0,0 @@ -let x: string | undefined = undefined; - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] - -for (let y: number | undefined = undefined; y < 2; y++) {} - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] - -let z; - -const x = undefined; - -function f(x: string | undefined = undefined) {} - -[0]: Unnecessary initialization to 'undefined'. diff --git a/test/rules/no-let-undefined/tslint.json b/test/rules/no-let-undefined/tslint.json deleted file mode 100644 index 5cbf82dce32..00000000000 --- a/test/rules/no-let-undefined/tslint.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "rules": { - "no-let-undefined": [true] - }, - "jsRules": { - "no-let-undefined": [true] - } -} diff --git a/test/rules/no-let-undefined/test.js.lint b/test/rules/no-unnecessary-initializer/test.js.lint similarity index 100% rename from test/rules/no-let-undefined/test.js.lint rename to test/rules/no-unnecessary-initializer/test.js.lint diff --git a/test/rules/no-unnecessary-initializer/test.ts.fix b/test/rules/no-unnecessary-initializer/test.ts.fix new file mode 100644 index 00000000000..a8143aba8d9 --- /dev/null +++ b/test/rules/no-unnecessary-initializer/test.ts.fix @@ -0,0 +1,16 @@ +let a: string | undefined; +var b: string | undefined; + +for (let c: number | undefined; c < 2; c++) {} + +let d; + +const e = undefined; + +function f(x: string | undefined, bar: number) {} +function f2(x: string | undefined = undefined, bar: number = 1) {} + +declare function get(): T +const { g } = get<{ g?: number }>(); +const [h] = get(); + diff --git a/test/rules/no-unnecessary-initializer/test.ts.lint b/test/rules/no-unnecessary-initializer/test.ts.lint new file mode 100644 index 00000000000..66d9dfc0ba4 --- /dev/null +++ b/test/rules/no-unnecessary-initializer/test.ts.lint @@ -0,0 +1,25 @@ +let a: string | undefined = undefined; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +var b: string | undefined = undefined; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +for (let c: number | undefined = undefined; c < 2; c++) {} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +let d; + +const e = undefined; + +function f(x: string | undefined = undefined, bar: number) {} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +function f2(x: string | undefined = undefined, bar: number = 1) {} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [1] + +declare function get(): T +const { g = undefined } = get<{ g?: number }>(); + ~~~~~~~~~~~~~ [0] +const [h = undefined] = get(); + ~~~~~~~~~~~~~ [0] + +[0]: Unnecessary initialization to 'undefined'. +[1]: Use an optional parameter instead of initializing to 'undefined'. Also, the type declaration does not need to include '| undefined'. diff --git a/test/rules/no-unnecessary-initializer/tslint.json b/test/rules/no-unnecessary-initializer/tslint.json new file mode 100644 index 00000000000..263c0cbe0c3 --- /dev/null +++ b/test/rules/no-unnecessary-initializer/tslint.json @@ -0,0 +1,8 @@ +{ + "rules": { + "no-unnecessary-initializer": [true] + }, + "jsRules": { + "no-unnecessary-initializer": [true] + } +}