no-else-return.js

/**
 * @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
//------------------------------------------------------------------------------

/** @type {import('../shared/types').Rule} */
module.exports = {
    meta: {
        type: "suggestion",

        docs: {
            description: "Disallow `else` blocks after `return` statements in `if` statements",
            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

        };

    }
};