--- /dev/null
+/**
+ * @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
+
+ };
+
+ }
+};
projects / dotfiles / .git / blobdiff