/** * @fileoverview Rule to flag `else` after a `return` in `if` * @author Ian Christian Myers */ "use strict"; //------------------------------------------------------------------------------ // Requirements //------------------------------------------------------------------------------ const astUtils = require("./utils/ast-utils"); const FixTracker = require("./utils/fix-tracker"); //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ module.exports = { meta: { type: "suggestion", docs: { description: "disallow `else` blocks after `return` statements in `if` statements", category: "Best Practices", recommended: false, url: "https://eslint.org/docs/rules/no-else-return" }, schema: [{ type: "object", properties: { allowElseIf: { type: "boolean", default: true } }, additionalProperties: false }], fixable: "code", messages: { unexpected: "Unnecessary 'else' after 'return'." } }, create(context) { //-------------------------------------------------------------------------- // Helpers //-------------------------------------------------------------------------- /** * Checks whether the given names can be safely used to declare block-scoped variables * in the given scope. Name collisions can produce redeclaration syntax errors, * or silently change references and modify behavior of the original code. * * This is not a generic function. In particular, it is assumed that the scope is a function scope or * a function's inner scope, and that the names can be valid identifiers in the given scope. * * @param {string[]} names Array of variable names. * @param {eslint-scope.Scope} scope Function scope or a function's inner scope. * @returns {boolean} True if all names can be safely declared, false otherwise. */ function isSafeToDeclare(names, scope) { if (names.length === 0) { return true; } const functionScope = scope.variableScope; /* * If this is a function scope, scope.variables will contain parameters, implicit variables such as "arguments", * all function-scoped variables ('var'), and block-scoped variables defined in the scope. * If this is an inner scope, scope.variables will contain block-scoped variables defined in the scope. * * Redeclaring any of these would cause a syntax error, except for the implicit variables. */ const declaredVariables = scope.variables.filter(({ defs }) => defs.length > 0); if (declaredVariables.some(({ name }) => names.includes(name))) { return false; } // Redeclaring a catch variable would also cause a syntax error. if (scope !== functionScope && scope.upper.type === "catch") { if (scope.upper.variables.some(({ name }) => names.includes(name))) { return false; } } /* * Redeclaring an implicit variable, such as "arguments", would not cause a syntax error. * However, if the variable was used, declaring a new one with the same name would change references * and modify behavior. */ const usedImplicitVariables = scope.variables.filter(({ defs, references }) => defs.length === 0 && references.length > 0); if (usedImplicitVariables.some(({ name }) => names.includes(name))) { return false; } /* * Declaring a variable with a name that was already used to reference a variable from an upper scope * would change references and modify behavior. */ if (scope.through.some(t => names.includes(t.identifier.name))) { return false; } /* * If the scope is an inner scope (not the function scope), an uninitialized `var` variable declared inside * the scope node (directly or in one of its descendants) is neither declared nor 'through' in the scope. * * For example, this would be a syntax error "Identifier 'a' has already been declared": * function foo() { if (bar) { let a; if (baz) { var a; } } } */ if (scope !== functionScope) { const scopeNodeRange = scope.block.range; const variablesToCheck = functionScope.variables.filter(({ name }) => names.includes(name)); if (variablesToCheck.some(v => v.defs.some(({ node: { range } }) => scopeNodeRange[0] <= range[0] && range[1] <= scopeNodeRange[1]))) { return false; } } return true; } /** * Checks whether the removal of `else` and its braces is safe from variable name collisions. * * @param {Node} node The 'else' node. * @param {eslint-scope.Scope} scope The scope in which the node and the whole 'if' statement is. * @returns {boolean} True if it is safe, false otherwise. */ function isSafeFromNameCollisions(node, scope) { if (node.type === "FunctionDeclaration") { // Conditional function declaration. Scope and hoisting are unpredictable, different engines work differently. return false; } if (node.type !== "BlockStatement") { return true; } const elseBlockScope = scope.childScopes.find(({ block }) => block === node); if (!elseBlockScope) { // ecmaVersion < 6, `else` block statement cannot have its own scope, no possible collisions. return true; } /* * elseBlockScope is supposed to merge into its upper scope. elseBlockScope.variables array contains * only block-scoped variables (such as let and const variables or class and function declarations) * defined directly in the elseBlockScope. These are exactly the only names that could cause collisions. */ const namesToCheck = elseBlockScope.variables.map(({ name }) => name); return isSafeToDeclare(namesToCheck, scope); } /** * Display the context report if rule is violated * * @param {Node} node The 'else' node * @returns {void} */ function displayReport(node) { const currentScope = context.getScope(); context.report({ node, messageId: "unexpected", fix: fixer => { if (!isSafeFromNameCollisions(node, currentScope)) { return null; } const sourceCode = context.getSourceCode(); const startToken = sourceCode.getFirstToken(node); const elseToken = sourceCode.getTokenBefore(startToken); const source = sourceCode.getText(node); const lastIfToken = sourceCode.getTokenBefore(elseToken); let fixedSource, firstTokenOfElseBlock; if (startToken.type === "Punctuator" && startToken.value === "{") { firstTokenOfElseBlock = sourceCode.getTokenAfter(startToken); } else { firstTokenOfElseBlock = startToken; } /* * 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 = /^[([/+`-]/u.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 && /^[([/+`-]/u.test(nextToken.value); const nextTokenOnSameLine = nextToken && nextToken.loc.start.line === lastTokenOfElseBlock.loc.start.line; /* * If the else block contents does not end in a semicolon, * and the else block starts with (, [, /, +, ` or -, then it is not * safe to remove the else block, because ASI will not add a semicolon * after the remaining else block contents */ if (nextTokenUnsafe || (nextTokenOnSameLine && nextToken.value !== "}")) { return null; } } if (startToken.type === "Punctuator" && startToken.value === "{") { fixedSource = source.slice(1, -1); } else { fixedSource = source; } /* * Extend the replacement range to include the entire * function to avoid conflicting with no-useless-return. * https://github.com/eslint/eslint/issues/8026 * * Also, to avoid name collisions between two else blocks. */ return new FixTracker(fixer, sourceCode) .retainEnclosingFunction(node) .replaceTextRange([elseToken.range[0], node.range[1]], fixedSource); } }); } /** * Check to see if the node is a ReturnStatement * * @param {Node} node The node being evaluated * @returns {boolean} True if node is a return */ function checkForReturn(node) { return node.type === "ReturnStatement"; } /** * Naive return checking, does not iterate through the whole * BlockStatement because we make the assumption that the ReturnStatement * will be the last node in the body of the BlockStatement. * * @param {Node} node The consequent/alternate node * @returns {boolean} True if it has a return */ function naiveHasReturn(node) { if (node.type === "BlockStatement") { const body = node.body, lastChildNode = body[body.length - 1]; return lastChildNode && checkForReturn(lastChildNode); } return checkForReturn(node); } /** * Check to see if the node is valid for evaluation, * meaning it has an else. * * @param {Node} node The node being evaluated * @returns {boolean} True if the node is valid */ function hasElse(node) { return node.alternate && node.consequent; } /** * If the consequent is an IfStatement, check to see if it has an else * and both its consequent and alternate path return, meaning this is * a nested case of rule violation. If-Else not considered currently. * * @param {Node} node The consequent node * @returns {boolean} True if this is a nested rule violation */ function checkForIf(node) { return node.type === "IfStatement" && hasElse(node) && naiveHasReturn(node.alternate) && naiveHasReturn(node.consequent); } /** * Check the consequent/body node to make sure it is not * a ReturnStatement or an IfStatement that returns on both * code paths. * * @param {Node} node The consequent or body node * @returns {boolean} `true` if it is a Return/If node that always returns. */ function checkForReturnOrIf(node) { return checkForReturn(node) || checkForIf(node); } /** * Check whether a node returns in every codepath. * @param {Node} node The node to be checked * @returns {boolean} `true` if it returns on every codepath. */ function alwaysReturns(node) { if (node.type === "BlockStatement") { // If we have a BlockStatement, check each consequent body node. return node.body.some(checkForReturnOrIf); } /* * If not a block statement, make sure the consequent isn't a * ReturnStatement or an IfStatement with returns on both paths. */ return checkForReturnOrIf(node); } /** * Check the if statement, but don't catch else-if blocks. * @returns {void} * @param {Node} node The node for the if statement to check * @private */ function checkIfWithoutElse(node) { const parent = node.parent; /* * Fixing this would require splitting one statement into two, so no error should * be reported if this node is in a position where only one statement is allowed. */ if (!astUtils.STATEMENT_LIST_PARENTS.has(parent.type)) { return; } const consequents = []; let alternate; for (let currentNode = node; currentNode.type === "IfStatement"; currentNode = currentNode.alternate) { if (!currentNode.alternate) { return; } consequents.push(currentNode.consequent); alternate = currentNode.alternate; } if (consequents.every(alwaysReturns)) { displayReport(alternate); } } /** * Check the if statement * @returns {void} * @param {Node} node The node for the if statement to check * @private */ function checkIfWithElse(node) { const parent = node.parent; /* * Fixing this would require splitting one statement into two, so no error should * be reported if this node is in a position where only one statement is allowed. */ if (!astUtils.STATEMENT_LIST_PARENTS.has(parent.type)) { return; } const alternate = node.alternate; if (alternate && alwaysReturns(node.consequent)) { displayReport(alternate); } } const allowElseIf = !(context.options[0] && context.options[0].allowElseIf === false); //-------------------------------------------------------------------------- // Public API //-------------------------------------------------------------------------- return { "IfStatement:exit": allowElseIf ? checkIfWithoutElse : checkIfWithElse }; } };