diff --git a/docs/rules/no-else-return.md b/docs/rules/no-else-return.md index 3127e0f511fb..f4045d5c3e22 100644 --- a/docs/rules/no-else-return.md +++ b/docs/rules/no-else-return.md @@ -2,6 +2,8 @@ If an `if` block contains a `return` statement, the `else` block becomes unnecessary. Its contents can be placed outside of the block. +(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) automatically fixes problems reported by this rule. + ```js function foo() { if (x) { diff --git a/lib/rules/no-else-return.js b/lib/rules/no-else-return.js index 43564346b043..4227c886910f 100644 --- a/lib/rules/no-else-return.js +++ b/lib/rules/no-else-return.js @@ -17,7 +17,9 @@ module.exports = { recommended: false }, - schema: [] + schema: [], + + fixable: "code" }, create(context) { @@ -33,7 +35,56 @@ module.exports = { * @returns {void} */ function displayReport(node) { - context.report({ node, message: "Unnecessary 'else' after 'return'." }); + context.report({ + node, + message: "Unnecessary 'else' after 'return'.", + fix: fixer => { + const sourceCode = context.getSourceCode(); + const alternateStartToken = sourceCode.getFirstToken(node.parent.alternate); + const elseToken = sourceCode.getTokenBefore(alternateStartToken); + const source = sourceCode.getText(node.parent.alternate); + const lastIfToken = sourceCode.getTokenBefore(elseToken); + let fixedSource, firstTokenOfElseBlock; + + if (alternateStartToken.type === "Punctuator" && alternateStartToken.value === "{") { + firstTokenOfElseBlock = sourceCode.getTokenAfter(alternateStartToken); + } else { + firstTokenOfElseBlock = alternateStartToken; + } + + // If the if block does not have curly braces and does not end in a semicolon + // and the else block starts with (, [, /, +, ` or -, then it is not + // safe to remove the else keyword, because ASI will not add a semicolon + // after the if block + const ifBlockMaybeUnsafe = node.parent.consequent.type !== "BlockStatement" && lastIfToken.value !== ";"; + const elseBlockUnsafe = /^[([/+`-]/.test(firstTokenOfElseBlock.value); + + if (ifBlockMaybeUnsafe && elseBlockUnsafe) { + return null; + } + + const endToken = sourceCode.getLastToken(node); + const lastTokenOfElseBlock = sourceCode.getTokenBefore(endToken); + + if (lastTokenOfElseBlock.value !== ";") { + const nextToken = sourceCode.getTokenAfter(endToken); + + const nextTokenUnsafe = nextToken && /^[([/+`-]/.test(nextToken.value); + const nextTokenOnSameLine = nextToken && nextToken.loc.start.line === lastTokenOfElseBlock.loc.start.line; + + if (nextTokenUnsafe || (nextTokenOnSameLine && nextToken.value !== "}")) { + return null; + } + } + + if (alternateStartToken.type === "Punctuator" && alternateStartToken.value === "{") { + fixedSource = source.slice(1, -1); + } else { + fixedSource = source; + } + return fixer.replaceTextRange([elseToken.start, node.end], fixedSource); + } + }); } /** @@ -121,35 +172,43 @@ module.exports = { return checkForReturnOrIf(node); } + /** + * Check the if statement + * @returns {void} + * @param {Node} node The node for the if statement to check + * @private + */ + function IfStatement(node) { + const parent = context.getAncestors().pop(); + let consequents, + alternate; + + // Only "top-level" if statements are checked, meaning the first `if` + // in a `if-else-if-...` chain. + if (parent.type === "IfStatement" && parent.alternate === node) { + return; + } + + for (consequents = []; node.type === "IfStatement"; node = node.alternate) { + if (!node.alternate) { + return; + } + consequents.push(node.consequent); + alternate = node.alternate; + } + + if (consequents.every(alwaysReturns)) { + displayReport(alternate); + } + } + //-------------------------------------------------------------------------- // Public API //-------------------------------------------------------------------------- return { - IfStatement(node) { - const parent = context.getAncestors().pop(); - let consequents, - alternate; - - // Only "top-level" if statements are checked, meaning the first `if` - // in a `if-else-if-...` chain. - if (parent.type === "IfStatement" && parent.alternate === node) { - return; - } - - for (consequents = []; node.type === "IfStatement"; node = node.alternate) { - if (!node.alternate) { - return; - } - consequents.push(node.consequent); - alternate = node.alternate; - } - - if (consequents.every(alwaysReturns)) { - displayReport(alternate); - } - } + "IfStatement:exit": IfStatement }; diff --git a/tests/lib/rules/no-else-return.js b/tests/lib/rules/no-else-return.js index c3a70c33af69..321be6bff895 100644 --- a/tests/lib/rules/no-else-return.js +++ b/tests/lib/rules/no-else-return.js @@ -30,23 +30,100 @@ ruleTester.run("no-else-return", rule, { "if (0) { if (0) {} else {} } else {}" ], invalid: [ - { code: "function foo() { if (true) { return x; } else { return y; } }", errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] }, - { code: "function foo() { if (true) { var x = bar; return x; } else { var y = baz; return y; } }", errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] }, - { code: "function foo() { if (true) return x; else return y; }", errors: [{ message: "Unnecessary 'else' after 'return'.", type: "ReturnStatement" }] }, - { code: "function foo() { if (true) { if (false) return x; else return y; } else { return z; } }", errors: [{ message: "Unnecessary 'else' after 'return'.", type: "ReturnStatement" }, { message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] }, - { code: "function foo() { if (true) { if (false) { if (true) return x; else w = y; } else { w = x; } } else { return z; } }", errors: [{ message: "Unnecessary 'else' after 'return'.", type: "ExpressionStatement" }] }, - { code: "function foo() { if (true) { if (false) { if (true) return x; else return y; } } else { return z; } }", errors: [{ message: "Unnecessary 'else' after 'return'.", type: "ReturnStatement" }] }, - { code: "function foo() { if (true) { if (false) { if (true) return x; else return y; } return w; } else { return z; } }", errors: [ - { message: "Unnecessary 'else' after 'return'.", type: "ReturnStatement" }, - { message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" } - ] }, - { code: "function foo() { if (true) { if (false) { if (true) return x; else return y; } else { w = x; } } else { return z; } }", errors: [ - { message: "Unnecessary 'else' after 'return'.", type: "ReturnStatement" }, - { message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" } - ] }, - { - code: "function foo() {if (x) { return true; } else if (y) { return true; } else { notAReturn(); } }", + { + code: "function foo1() { if (true) { return x; } else { return y; } }", + output: "function foo1() { if (true) { return x; } return y; }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo2() { if (true) { var x = bar; return x; } else { var y = baz; return y; } }", + output: "function foo2() { if (true) { var x = bar; return x; } var y = baz; return y; }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo3() { if (true) return x; else return y; }", + output: "function foo3() { if (true) return x; return y; }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "ReturnStatement" }] }, + { + code: "function foo4() { if (true) { if (false) return x; else return y; } else { return z; } }", + output: "function foo4() { if (true) { if (false) return x; return y; } return z; }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "ReturnStatement" }, { message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo5() { if (true) { if (false) { if (true) return x; else { w = y; } } else { w = x; } } else { return z; } }", + output: "function foo5() { if (true) { if (false) { if (true) return x; w = y; } else { w = x; } } else { return z; } }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo6() { if (true) { if (false) { if (true) return x; else return y; } } else { return z; } }", + output: "function foo6() { if (true) { if (false) { if (true) return x; return y; } } else { return z; } }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "ReturnStatement" }] + }, + { + code: "function foo7() { if (true) { if (false) { if (true) return x; else return y; } return w; } else { return z; } }", + output: "function foo7() { if (true) { if (false) { if (true) return x; return y; } return w; } return z; }", + errors: [ + { message: "Unnecessary 'else' after 'return'.", type: "ReturnStatement" }, + { message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" } + ] + }, + { + code: "function foo8() { if (true) { if (false) { if (true) return x; else return y; } else { w = x; } } else { return z; } }", + output: "function foo8() { if (true) { if (false) { if (true) return x; return y; } w = x; } else { return z; } }", + errors: [ + { message: "Unnecessary 'else' after 'return'.", type: "ReturnStatement" }, + { message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" } + ] + }, + { + code: "function foo9() {if (x) { return true; } else if (y) { return true; } else { notAReturn(); } }", + output: "function foo9() {if (x) { return true; } else if (y) { return true; } notAReturn(); }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo10() { if (foo) return bar; else (foo).bar(); }", + output: "function foo10() { if (foo) return bar; (foo).bar(); }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "ExpressionStatement" }] + }, + { + code: "function foo11() { if (foo) return bar \nelse { [1, 2, 3].map(foo) } }", + output: "function foo11() { if (foo) return bar \nelse { [1, 2, 3].map(foo) } }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo12() { if (foo) return bar \nelse { baz() } \n[1, 2, 3].map(foo) }", + output: "function foo12() { if (foo) return bar \nelse { baz() } \n[1, 2, 3].map(foo) }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo13() { if (foo) return bar; \nelse { [1, 2, 3].map(foo) } }", + output: "function foo13() { if (foo) return bar; \n [1, 2, 3].map(foo) }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo14() { if (foo) return bar \nelse { baz(); } \n[1, 2, 3].map(foo) }", + output: "function foo14() { if (foo) return bar \n baz(); \n[1, 2, 3].map(foo) }", errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo15() { if (foo) return bar; else { baz() } qaz() }", + output: "function foo15() { if (foo) return bar; else { baz() } qaz() }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo16() { if (foo) return bar \nelse { baz() } qaz() }", + output: "function foo16() { if (foo) return bar \nelse { baz() } qaz() }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo17() { if (foo) return bar \nelse { baz() } \nqaz() }", + output: "function foo17() { if (foo) return bar \n baz() \nqaz() }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "BlockStatement" }] + }, + { + code: "function foo18() { if (foo) return function() {} \nelse [1, 2, 3].map(bar) }", + output: "function foo18() { if (foo) return function() {} \nelse [1, 2, 3].map(bar) }", + errors: [{ message: "Unnecessary 'else' after 'return'.", type: "ExpressionStatement" }] } ] });