fix: no-useless-return false negative in try statement

Promote up the useless returns in BlockStatement inside TryStatement when ESLint is going up in the tree during traversing the AST. Fixes eslint#7481
guilhermejcgois · Nov 27, 2022 · 88107a1 · 88107a1
1 parent e6a865d
commit 88107a1
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 16 deletions.
diff --git a/lib/rules/no-useless-return.js b/lib/rules/no-useless-return.js
@@ -38,6 +38,15 @@ function isRemovable(node) {
     return astUtils.STATEMENT_LIST_PARENTS.has(node.parent.type);
 }
 
+/**
+ * Checks if given node is a try statement or not.
+ * @param {ASTNode} node The node to check.
+ * @returns {boolean} `true` if the node is a try statement.
+ */
+function isTryStatement(node) {
+    return node.type === "TryStatement";
+}
+
 /**
  * Checks whether the given return statement is in a `finally` block or not.
  * @param {ASTNode} node The return statement node to check.
@@ -49,14 +58,27 @@ function isInFinally(node) {
         currentNode && currentNode.parent && !astUtils.isFunction(currentNode);
         currentNode = currentNode.parent
     ) {
-        if (currentNode.parent.type === "TryStatement" && currentNode.parent.finalizer === currentNode) {
+        if (isTryStatement(currentNode.parent) && currentNode.parent.finalizer === currentNode) {
             return true;
         }
     }
 
     return false;
 }
 
+/**
+ * Checks whether the given node is the last one in the parentNode.
+ * @param {ASTNode} node The node to check.
+ * @param {ASTNode} parentNode The node that possibly contains the given `node`.
+ * @param {any} sourceCode The source code
+ * @returns {boolean} `true` if the node is the last one in the `parentNode` children.
+ */
+function isLastNode(node, parentNode, sourceCode) {
+    const lastNodeInParent = parentNode.body[parentNode.body.length - 1];
+
+    return astUtils.equalTokens(node, lastNodeInParent, sourceCode);
+}
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -274,6 +296,22 @@ module.exports = {
                 }
                 scopeInfo.uselessReturns.push(node);
             },
+            "BlockStatement:exit"(node) {
+                if (
+                    !isTryStatement(node.parent) ||
+                    !scopeInfo.upper ||
+                    !scopeInfo.uselessReturns
+                ) {
+                    return;
+                }
+
+                const lastUseselessReturn = scopeInfo.uselessReturns[scopeInfo.uselessReturns.length - 1];
+
+
+                if (lastUseselessReturn && isLastNode(lastUseselessReturn, node, sourceCode)) {
+                    scopeInfo.upper.uselessReturns.push(lastUseselessReturn);
+                }
+            },
 
             /*
              * Registers for all statement nodes except FunctionDeclaration, BlockStatement, BreakStatement.

diff --git a/tests/lib/rules/no-useless-return.js b/tests/lib/rules/no-useless-return.js
@@ -391,41 +391,44 @@ ruleTester.run("no-useless-return", rule, {
          * FIXME: Re-add this case (removed due to https://github.com/eslint/eslint/issues/7481):
          * https://github.com/eslint/eslint/blob/261d7287820253408ec87c344beccdba2fe829a4/tests/lib/rules/no-useless-return.js#L308-L329
          */
-
         {
             code: `
               function foo() {
-                try {} finally {}
-                return;
+                try {
+                  foo();
+                  return;
+                } catch (err) {
+                  return 5;
+                }
               }
             `,
             output: `
               function foo() {
-                try {} finally {}
-
+                try {
+                  foo();
+
+                } catch (err) {
+                  return 5;
+                }
               }
             `
         },
         {
             code: `
               function foo() {
                 try {
-                  return 5;
-                } finally {
-                  function bar() {
-                    return;
-                  }
+                  return;
+                } catch (err) {
+                  foo();
                 }
               }
             `,
             output: `
               function foo() {
                 try {
-                  return 5;
-                } finally {
-                  function bar() {
-
-                  }
+
+                } catch (err) {
+                  foo();
                 }
               }
             `