add code path analysis to always-return

This will also change the reported location of errors to the inner
function that failed to return, rather than referring to it's enclosing
Promise

I bumped up the the version number (2.0.1 -> 3.0.0) because the changed
location reporting could un-suppress linter errors that have been
suppressed with `// eslint-disable-line promise/always-return`

fixes #8
fixes #18
fixes #24

Author: ahuff44

README.md

@@ -61,13 +61,15 @@ We also allow someone to `throw` inside a `then()` which is essentially the same
 myPromise.then((val) => val * 2));
 myPromise.then(function(val) { return val * 2; });
 myPromise.then(doSomething); // could be either
+myPromise.then((b) => { if (b) { return "yes" } else { return "no" } });
 ```
 
 #### Invalid
 
 ```js
 myPromise.then(function(val) {});
 myPromise.then(() => { doSomething(); });
+myPromise.then((b) => { if (b) { return "yes" } else { forgotToReturn(); } });
 ```
 
 ### `param-names`

package.json

@@ -1,6 +1,6 @@
 {
   "name": "eslint-plugin-promise",
-  "version": "2.0.1",
+  "version": "3.0.0",
   "description": "Enforce best practices for JavaScript promises",
   "keywords": [
     "eslint",

rules/always-return.js

@@ -8,31 +8,88 @@ function isFunctionWithBlockStatement (node) {
   return false
 }
 
-function isReturnOrThrowStatement (node) {
-  return node.type === 'ReturnStatement' || node.type === 'ThrowStatement'
+function isThenExpression (node) {
+  return (
+    node.type === 'CallExpression' &&
+    node.callee.type === 'MemberExpression' &&
+    node.callee.property.name === 'then'
+  )
+}
+
+function isInlineThenFunctionExpression (node) {
+  return (
+    isFunctionWithBlockStatement(node) &&
+    isThenExpression(node.parent)
+  )
+}
+
+function includes (arr, val) {
+  return arr.indexOf(val) !== -1
+}
+
+function last (arr) {
+  return arr[arr.length - 1]
 }
 
 module.exports = {
   create: function (context) {
-    return {
-      MemberExpression: function (node) {
-        var firstArg, body, lastStatement
-
-        if (node.property.name !== 'then' || node.parent.type !== 'CallExpression') {
-          return
-        }
-
-        firstArg = node.parent.arguments[0]
-        if (!firstArg || !isFunctionWithBlockStatement(firstArg)) {
-          return
-        }
-
-        body = firstArg.body.body
-        lastStatement = body[body.length - 1]
-        if (!lastStatement || !isReturnOrThrowStatement(lastStatement)) {
-          context.report(node, 'Each then() should return a value or throw')
-        }
+    var funcInfoStack = []
+    var CPSIDStack = []
+
+    function isEveryBranchReturning (funcInfo) {
+      // We need to check noCurrentCPSIsOnTheCPSStack because of what
+      // seems like a bug in eslint where 'FunctionExpression:exit' events occur
+      // before all of their constituent codePathSegments have fired their
+      // 'onCodePathSegmentEnd' events
+      var currentIDs = funcInfo.codePath.currentSegments.map(x => x.id)
+      var noCurrentCPSIsOnTheCPSStack = !currentIDs.some((id) => includes(CPSIDStack, id))
+
+      var finalIDs = funcInfo.codePath.finalSegments.map(x => x.id)
+      var everyFinalCPSIsReturning = finalIDs.every((id) => includes(funcInfo.explicitlyReturningCPSIDs, id))
+
+      return noCurrentCPSIsOnTheCPSStack && everyFinalCPSIsReturning
+    }
+
+    function onFunctionExpressionExit (node) {
+      if (!isInlineThenFunctionExpression(node)) {
+        return
+      }
+
+      var funcInfo = last(funcInfoStack)
+      if (!isEveryBranchReturning(funcInfo)) {
+        context.report(node, 'Each then() should return a value or throw')
       }
     }
+
+    function markCurrentCodePathSegmentAsReturning () {
+      var funcInfo = last(funcInfoStack)
+      var currentCPSID = last(CPSIDStack)
+      funcInfo.explicitlyReturningCPSIDs.push(currentCPSID)
+    }
+
+    return {
+      onCodePathStart: function (codePath, node) {
+        funcInfoStack.push({
+          codePath: codePath,
+          explicitlyReturningCPSIDs: []
+        })
+      },
+
+      onCodePathEnd: function (codePath, node) {
+        funcInfoStack.pop()
+      },
+
+      onCodePathSegmentEnd: function (segment, node) {
+        CPSIDStack.pop()
+      },
+      onCodePathSegmentStart: function (segment, node) {
+        CPSIDStack.push(segment.id)
+      },
+
+      ReturnStatement: markCurrentCodePathSegmentAsReturning,
+      ThrowStatement: markCurrentCodePathSegmentAsReturning,
+      'FunctionExpression:exit': onFunctionExpressionExit,
+      'ArrowFunctionExpression:exit': onFunctionExpressionExit
+    }
   }
 }

test/always-return.test.js

@@ -9,6 +9,7 @@ ruleTester.run('always-return', rule, {
   valid: [
     { code: 'hey.then(x => x)', parserOptions: parserOptions },
     { code: 'hey.then(x => ({}))', parserOptions: parserOptions },
+    { code: 'hey.then(x => { return; })', parserOptions: parserOptions },
     { code: 'hey.then(x => { return x * 10 })', parserOptions: parserOptions },
     { code: 'hey.then(function() { return 42; })', parserOptions: parserOptions },
     { code: 'hey.then(function() { return new Promise(); })', parserOptions: parserOptions },
@@ -19,7 +20,10 @@ ruleTester.run('always-return', rule, {
     { code: 'hey.then(function(x) { if (x) { return x; } throw new Error("no x"); })', parserOptions: parserOptions },
     { code: 'hey.then(x => { throw new Error("msg"); })', parserOptions: parserOptions },
     { code: 'hey.then(x => { if (!x) { throw new Error("no x"); } return x; })', parserOptions: parserOptions },
-    { code: 'hey.then(x => { if (x) { return x; } throw new Error("no x"); })', parserOptions: parserOptions }
+    { code: 'hey.then(x => { if (x) { return x; } throw new Error("no x"); })', parserOptions: parserOptions },
+    { code: 'hey.then(x => { var f = function() { }; return f; })', parserOptions: parserOptions },
+    { code: 'hey.then(x => { if (x) { return x; } else { return x; } })', parserOptions: parserOptions },
+    { code: 'hey.then(x => { return x; var y = "unreachable"; })', parserOptions: parserOptions }
   ],
 
   invalid: [
@@ -40,9 +44,29 @@ ruleTester.run('always-return', rule, {
       code: 'hey.then(function() { }).then(function() { })',
       errors: [ { message: message }, { message: message } ]
     },
+    {
+      code: 'hey.then(function() { return; }).then(function() { })',
+      errors: [ { message: message } ]
+    },
     {
       code: 'hey.then(function() { doSomethingWicked(); })',
       errors: [ { message: message } ]
+    },
+    {
+      code: 'hey.then(function() { if (x) { return x; } })',
+      errors: [ { message: message } ]
+    },
+    {
+      code: 'hey.then(function() { if (x) { return x; } else { }})',
+      errors: [ { message: message } ]
+    },
+    {
+      code: 'hey.then(function() { if (x) { } else { return x; }})',
+      errors: [ { message: message } ]
+    },
+    {
+      code: 'hey.then(function() { if (x) { return you.then(function() { return x; }); } })',
+      errors: [ { message: message } ]
     }
   ]
 })