fix(always-return): false positives for logical expr (#363)

eslint-community · Sep 29, 2022 · a60d1cb · a60d1cb
1 parent 9b3ef57
commit a60d1cb
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 24 deletions.
diff --git a/__tests__/always-return.js b/__tests__/always-return.js
@@ -42,6 +42,12 @@ ruleTester.run('always-return', rule, {
           }
         })
       })`,
+    `hey.then(({x, y}) => {
+        if (y) {
+          throw new Error(x || y)
+        }
+        return x
+      })`,
   ],
 
   invalid: [
@@ -106,5 +112,23 @@ ruleTester.run('always-return', rule, {
       })()`,
       errors: [{ message }],
     },
+    {
+      code: `
+      hey.then(({x, y}) => {
+        if (y) {
+          throw new Error(x || y)
+        }
+      })`,
+      errors: [{ message }],
+    },
+    {
+      code: `
+      hey.then(({x, y}) => {
+        if (y) {
+          return x
+        }
+      })`,
+      errors: [{ message }],
+    },
   ],
 })
diff --git a/rules/always-return.js b/rules/always-return.js
@@ -34,24 +34,6 @@ function isInlineThenFunctionExpression(node) {
   )
 }
 
-function hasParentReturnStatement(node) {
-  // istanbul ignore else -- not reachable given not checking `Program`
-  if (node && node.parent && node.parent.type) {
-    // if the parent is a then, and we haven't returned anything, fail
-    if (isThenCallExpression(node.parent)) {
-      return false
-    }
-
-    if (node.parent.type === 'ReturnStatement') {
-      return true
-    }
-    return hasParentReturnStatement(node.parent)
-  }
-
-  // istanbul ignore next -- not reachable given not checking `Program`
-  return false
-}
-
 function peek(arr) {
   return arr[arr.length - 1]
 }
@@ -106,8 +88,8 @@ module.exports = {
     }
 
     return {
-      ReturnStatement: markCurrentBranchAsGood,
-      ThrowStatement: markCurrentBranchAsGood,
+      'ReturnStatement:exit': markCurrentBranchAsGood,
+      'ThrowStatement:exit': markCurrentBranchAsGood,
 
       onCodePathSegmentStart(segment, node) {
         const funcInfo = peek(funcInfoStack)
@@ -138,10 +120,6 @@ module.exports = {
           const id = segment.id
           const branch = funcInfo.branchInfoMap[id]
           if (!branch.good) {
-            if (hasParentReturnStatement(branch.node)) {
-              return
-            }
-
             context.report({
               message: 'Each then() should return a value or throw',
               node: branch.node,