Merge pull request #194 from cherryblossom000/193

fix(no-return-wrap): fix it not reporting for arrow functions without braces
eslint-community · Jul 8, 2020 · 41ec09f · 41ec09f
2 parents f9a9ee0 + 12dfa12
commit 41ec09f
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 22 deletions.
diff --git a/__tests__/no-return-wrap.js b/__tests__/no-return-wrap.js
@@ -54,7 +54,20 @@ ruleTester.run('no-return-wrap', rule, {
     },
 
     // not function bind
-    'doThing().then((function() { return Promise.resolve(4) }).toString())'
+    'doThing().then((function() { return Promise.resolve(4) }).toString())',
+
+    {
+      code: 'doThing().then(() => Promise.reject(4))',
+      options: [{ allowReject: true }]
+    },
+
+    // Call expressions that aren't Promise.resolve/reject
+    'doThing().then(function() { return a() })',
+    'doThing().then(function() { return Promise.a() })',
+    'doThing().then(() => { return a() })',
+    'doThing().then(() => { return Promise.a() })',
+    'doThing().then(() => a())',
+    'doThing().then(() => Promise.a())'
   ],
 
   invalid: [
@@ -115,7 +128,7 @@ ruleTester.run('no-return-wrap', rule, {
     // {code: 'doThing().catch(function(x) { return true ? Promise.resolve(4) : Promise.reject(5) })', errors: [{message: rejectMessage }, {message: resolveMessage}]},
     // {code: 'doThing().catch(function(x) { return x && Promise.reject(4) })', errors: [{message: rejectMessage}]}
 
-    // mltiple "ExpressionStatement"
+    // multiple "ExpressionStatement"
     {
       code: `
       fn(function() {
@@ -214,6 +227,16 @@ ruleTester.run('no-return-wrap', rule, {
       }
       `,
       errors: [{ message: resolveMessage }]
+    },
+
+    // issue #193
+    {
+      code: 'doThing().then(() => Promise.resolve(4))',
+      errors: [{ message: resolveMessage }]
+    },
+    {
+      code: 'doThing().then(() => Promise.reject(4))',
+      errors: [{ message: rejectMessage }]
     }
   ]
 })
diff --git a/rules/always-return.js b/rules/always-return.js
@@ -68,7 +68,7 @@ module.exports = {
     // funcInfoStack[i].branchInfoMap is an object representing information
     //   about all branches within the given function
     // funcInfoStack[i].branchInfoMap[j].good is a boolean representing whether
-    //   the given branch explictly `return`s or `throw`s. It starts as `false`
+    //   the given branch explicitly `return`s or `throw`s. It starts as `false`
     //   for every branch and is updated to `true` if a `return` or `throw`
     //   statement is found
     // funcInfoStack[i].branchInfoMap[j].loc is a eslint SourceLocation object

diff --git a/rules/lib/has-promise-callback.js b/rules/lib/has-promise-callback.js
@@ -1,5 +1,5 @@
 /**
- * Library: Has Promis eCallback
+ * Library: Has Promise Callback
  * Makes sure that an Expression node is part of a promise
  * with callback functions (like then() or catch())
  */

diff --git a/rules/no-return-wrap.js b/rules/no-return-wrap.js
@@ -1,6 +1,6 @@
 /**
  * Rule: no-return-wrap function
- * Prevents uneccessary wrapping of results in Promise.resolve
+ * Prevents unnecessary wrapping of results in Promise.resolve
  * or Promise.reject as the Promise will do that for us
  */
 
@@ -49,26 +49,33 @@ module.exports = {
     const options = context.options[0] || {}
     const allowReject = options.allowReject
 
+    /**
+     * Checks a call expression, reporting if necessary.
+     * @param callExpression The call expression.
+     * @param node The node to report.
+     */
+    function checkCallExpression({ callee }, node) {
+      if (
+        isInPromise(context) &&
+        callee.type === 'MemberExpression' &&
+        callee.object.name === 'Promise'
+      ) {
+        if (callee.property.name === 'resolve') {
+          context.report({ node, messageId: 'resolve' })
+        } else if (!allowReject && callee.property.name === 'reject') {
+          context.report({ node, messageId: 'reject' })
+        }
+      }
+    }
+
     return {
       ReturnStatement(node) {
-        if (isInPromise(context)) {
-          if (node.argument) {
-            if (node.argument.type === 'CallExpression') {
-              if (node.argument.callee.type === 'MemberExpression') {
-                if (node.argument.callee.object.name === 'Promise') {
-                  if (node.argument.callee.property.name === 'resolve') {
-                    context.report({ node, messageId: 'resolve' })
-                  } else if (
-                    !allowReject &&
-                    node.argument.callee.property.name === 'reject'
-                  ) {
-                    context.report({ node, messageId: 'reject' })
-                  }
-                }
-              }
-            }
-          }
+        if (node.argument && node.argument.type === 'CallExpression') {
+          checkCallExpression(node.argument, node)
         }
+      },
+      'ArrowFunctionExpression > CallExpression'(node) {
+        checkCallExpression(node, node)
       }
     }
   }