From 5bd58d2708da7f24c9cbf201e48ae3dac977a21c Mon Sep 17 00:00:00 2001 From: Debsmita Date: Mon, 4 Feb 2019 21:08:10 +0530 Subject: [PATCH] Added no-unnecessary-else Rule --- src/configs/all.ts | 1 + src/configs/recommended.ts | 1 + src/rules/noUnnecessaryElseRule.ts | 96 ++++++++++++++++++ test/rules/no-unnecessary-else/test.ts.lint | 107 ++++++++++++++++++++ test/rules/no-unnecessary-else/tslint.json | 6 ++ 5 files changed, 211 insertions(+) create mode 100644 src/rules/noUnnecessaryElseRule.ts create mode 100644 test/rules/no-unnecessary-else/test.ts.lint create mode 100644 test/rules/no-unnecessary-else/tslint.json diff --git a/src/configs/all.ts b/src/configs/all.ts index a5658c6382c..203d9d3401c 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -151,6 +151,7 @@ export const rules = { "unnecessary-constructor": true, "use-default-type-parameter": true, "use-isnan": true, + "no-unnecessary-else": true, // Maintainability diff --git a/src/configs/recommended.ts b/src/configs/recommended.ts index a692d17e862..0119157386f 100644 --- a/src/configs/recommended.ts +++ b/src/configs/recommended.ts @@ -91,6 +91,7 @@ export const rules = { "no-string-throw": true, "no-switch-case-fall-through": false, "no-trailing-whitespace": true, + "no-unnecessary-else": true, "no-unnecessary-initializer": true, "no-unsafe-finally": true, "no-unused-expression": true, diff --git a/src/rules/noUnnecessaryElseRule.ts b/src/rules/noUnnecessaryElseRule.ts new file mode 100644 index 00000000000..07dcead75e7 --- /dev/null +++ b/src/rules/noUnnecessaryElseRule.ts @@ -0,0 +1,96 @@ +/** + * @license + * Copyright 2018 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 { + public static metadata: Lint.IRuleMetadata = { + description: Lint.Utils.dedent` + Disallows else block when the If block contains control flow statements, such as \`return\`, \`continue\`, + \`break\` and \`throws\`.`, + descriptionDetails: "", + optionExamples: [true], + options: null, + optionsDescription: "Not configurable.", + rationale: Lint.Utils.dedent` + When control flow statements, + such as \`return\`, \`continue\`, \`break\` and \`throws\` are written inside \`if\` block, + Then \`else\` block becomes unnecessary.`, + ruleName: "no-unnecessary-else", + type: "functionality", + typescriptOnly: false, + }; + + public static FAILURE_STRING(name: string): string { + return `If block contains \`${name}\` statement. Consider replacing the contents of else block outside of the block.`; + } + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithFunction(sourceFile, walk); + } +} + +type JumpStatement = + | ts.BreakStatement + | ts.ContinueStatement + | ts.ThrowStatement + | ts.ReturnStatement; + +function walk(ctx: Lint.WalkContext): void { + let inElse = false; + ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void { + switch (node.kind) { + case ts.SyntaxKind.IfStatement: + const { thenStatement, elseStatement } = node as ts.IfStatement; + if (elseStatement !== undefined) { + inElse = true; + } + ts.forEachChild(thenStatement, cb); + break; + + case ts.SyntaxKind.BreakStatement: + case ts.SyntaxKind.ContinueStatement: + case ts.SyntaxKind.ThrowStatement: + case ts.SyntaxKind.ReturnStatement: + if (inElse) { + ctx.addFailureAtNode( + node, + Rule.FAILURE_STRING(printJumpKind(node as JumpStatement)), + ); + inElse = false; + } + break; + + default: + return ts.forEachChild(node, cb); + } + }); +} + +function printJumpKind(node: JumpStatement): string { + switch (node.kind) { + case ts.SyntaxKind.BreakStatement: + return "break"; + case ts.SyntaxKind.ContinueStatement: + return "continue"; + case ts.SyntaxKind.ThrowStatement: + return "throw"; + case ts.SyntaxKind.ReturnStatement: + return "return"; + } +} diff --git a/test/rules/no-unnecessary-else/test.ts.lint b/test/rules/no-unnecessary-else/test.ts.lint new file mode 100644 index 00000000000..e6b3cdc7c8e --- /dev/null +++ b/test/rules/no-unnecessary-else/test.ts.lint @@ -0,0 +1,107 @@ +const testReturn = () => { + if () { + return; + ~~~~~~~ [return] + } else { + return; + } +} + +//valid +const testReturn = () => { + if () { + return; + } + return; +} + +const testReturn = () => { + if () { + return; + ~~~~~~~ [return] + } else if { + if () { + return ; + } else { + return ; + } + } + return; +} + +const testReturn = () => { + if () { + return; + } + if () { + return ; + ~~~~~~~~ [return] + } else { + return ; + } +} + +//valid +const testReturn = () => { + if () { + return; + } + if () { + return ; + } + return ; +} + +function testThrow () { + if () { + throw "error"; + ~~~~~~~~~~~~~~ [throw] + } else { + } +} + +//valid +function testThrow () { + if () { + throw "error"; + } +} + +function testContinue () { + for ( ) { + if () { + continue ; + ~~~~~~~~~~ [continue] + } else { + } + } +} + +//valid +function testContinue () { + for ( ) { + if () { + continue ; + } + } +} + +function testBreak () { + if () { + break ; + ~~~~~~~ [break] + } else { + } +} + +//valid +function testBreak () { + if () { + break ; + } +} + +[return]: If block contains `return` statement. Consider replacing the contents of else block outside of the block. +[throw]: If block contains `throw` statement. Consider replacing the contents of else block outside of the block. +[break]: If block contains `break` statement. Consider replacing the contents of else block outside of the block. +[continue]: If block contains `continue` statement. Consider replacing the contents of else block outside of the block. diff --git a/test/rules/no-unnecessary-else/tslint.json b/test/rules/no-unnecessary-else/tslint.json new file mode 100644 index 00000000000..d5a4a8c2e79 --- /dev/null +++ b/test/rules/no-unnecessary-else/tslint.json @@ -0,0 +1,6 @@ +{ + "rules": { + "no-unnecessary-else": true + } + } + \ No newline at end of file