Allow concise ArrowFunctionExpression (named-functions-in-promises) (#64)

* Allow concise ArrowFunctionExpression (named-functions-in-promises)

* Ensure CallExpression is calling a method on the parent object

This is to ensure that the promise handler is defined as a method on the
parent object and not some other object.

* Fix destructuring

* Drop requirement that method is called on this

* Make allowing concise arrow expressions optional

* Document allowConciseArrow option (named-functions-in-promises)

* Address Code Review Comments

- Rename `allowConciseArrow` to `allowSimpleArrowFunction`
- Update documentation for `allowSimpleArrowFunction`
- Move `isConciseArrowFunctionWithCallExpression` to utils
- Add unit test for `isConciseArrowFunctionWithCallExpression`
- Add invalid test cases for arrow functions with bodies when
  `allowSimpleArrowFunction` is set to true

* Update isConciseArrowFunctionWithCallExpression test name

* Add test case for block body

Author: sudowork

docs/rules/named-functions-in-promises.md

@@ -2,6 +2,20 @@
 
 ### Rule name: `named-functions-in-promises`
 
+#### Configuration
+
+```
+ember/named-functions-in-promises: [2, {
+  allowSimpleArrowFunction: false,
+}]
+```
+
+Setting `allowSimpleArrowFunction` to `true` allows arrow function expressions that do not have block bodies.
+These simple arrow functions must also only contain a single function call.
+For example: `.then(user => this._reloadUser(user))`.
+
+#### Description
+
 When you use promises and its handlers, use named functions defined on parent object. Thus, you will be able to test them in isolation using unit tests without any additional mocking.
 
 ```javascript

lib/rules/named-functions-in-promises.js

@@ -12,6 +12,9 @@ module.exports = {
   },
 
   create(context) {
+    const options = context.options[0] || {};
+    const allowSimpleArrowFunction = options.allowSimpleArrowFunction || false;
+
     const message = 'Use named functions defined on objects to handle promises';
 
     const report = function (node) {
@@ -22,14 +25,19 @@ module.exports = {
       CallExpression(node) {
         const firstArg = node.arguments[0];
 
-        if (
-          hasPromiseExpression(node) &&
-          (
+        if (hasPromiseExpression(node)) {
+          if (
+            allowSimpleArrowFunction &&
+            utils.isConciseArrowFunctionWithCallExpression(firstArg)
+          ) {
+            return;
+          }
+          if (
             utils.isFunctionExpression(firstArg) ||
             utils.isArrowFunctionExpression(firstArg)
-          )
-        ) {
-          report(node);
+          ) {
+            report(node);
+          }
         }
       },
     };

lib/utils/utils.js

@@ -10,6 +10,7 @@ module.exports = {
   isArrayExpression,
   isFunctionExpression,
   isArrowFunctionExpression,
+  isConciseArrowFunctionWithCallExpression,
   isNewExpression,
   isCallWithFunctionExpression,
   isThisExpression,
@@ -118,6 +119,20 @@ function isArrowFunctionExpression(node) {
   return node !== undefined && node.type === 'ArrowFunctionExpression';
 }
 
+/**
+ * Check whether or not a node is an ArrowFunctionExpression with concise body
+ * that contains a call expression.
+ *
+ * @param {Object} node The node to check.
+ * @returns {boolean} Whether or not the node is an ArrowFunctionExpression
+ * with concise body.
+ */
+function isConciseArrowFunctionWithCallExpression(node) {
+  return isArrowFunctionExpression(node) &&
+    node.expression &&
+    isCallExpression(node.body);
+}
+
 /**
  * Check whether or not a node is an NewExpression.
  *

tests/lib/rules/named-functions-in-promises.js

@@ -39,6 +39,30 @@ eslintTester.run('named-functions-in-promises', rule, {
     }, {
       code: 'user.save().finally(_finallyDo);',
       parserOptions: { ecmaVersion: 6 },
+    }, {
+      code: 'user.save().then(() => this._reloadUser(user));',
+      parserOptions: { ecmaVersion: 6 },
+      options: [{
+        allowSimpleArrowFunction: true,
+      }],
+    }, {
+      code: 'user.save().catch(err => this._handleError(err));',
+      parserOptions: { ecmaVersion: 6 },
+      options: [{
+        allowSimpleArrowFunction: true,
+      }],
+    }, {
+      code: 'user.save().finally(() => this._finallyDo());',
+      parserOptions: { ecmaVersion: 6 },
+      options: [{
+        allowSimpleArrowFunction: true,
+      }],
+    }, {
+      code: 'user.save().then(() => user.reload());',
+      parserOptions: { ecmaVersion: 6 },
+      options: [{
+        allowSimpleArrowFunction: true,
+      }],
     },
   ],
   invalid: [
@@ -49,23 +73,77 @@ eslintTester.run('named-functions-in-promises', rule, {
         message: 'Use named functions defined on objects to handle promises',
       }],
     }, {
-      code: 'user.save().then(() => user.reload());',
+      code: 'user.save().catch(() => {return error.handle();});',
       parserOptions: { ecmaVersion: 6 },
       errors: [{
         message: 'Use named functions defined on objects to handle promises',
       }],
+    }, {
+      code: 'user.save().finally(() => {return finallyDo();});',
+      parserOptions: { ecmaVersion: 6 },
+      errors: [{
+        message: 'Use named functions defined on objects to handle promises',
+      }],
+    }, {
+      code: 'user.save().then(() => {return user.reload();});',
+      parserOptions: { ecmaVersion: 6 },
+      options: [{
+        allowSimpleArrowFunction: true,
+      }],
+      errors: [{
+        message: 'Use named functions defined on objects to handle promises',
+      }],
     }, {
       code: 'user.save().catch(() => {return error.handle();});',
       parserOptions: { ecmaVersion: 6 },
+      options: [{
+        allowSimpleArrowFunction: true,
+      }],
       errors: [{
         message: 'Use named functions defined on objects to handle promises',
       }],
     }, {
       code: 'user.save().finally(() => {return finallyDo();});',
       parserOptions: { ecmaVersion: 6 },
+      options: [{
+        allowSimpleArrowFunction: true,
+      }],
       errors: [{
         message: 'Use named functions defined on objects to handle promises',
       }],
-    },
+    }, {
+      code: 'user.save().then(() => this._reloadUser(user));',
+      parserOptions: { ecmaVersion: 6 },
+      errors: [{
+        message: 'Use named functions defined on objects to handle promises',
+      }],
+    }, {
+      code: 'user.save().catch(err => this._handleError(err));',
+      parserOptions: { ecmaVersion: 6 },
+      errors: [{
+        message: 'Use named functions defined on objects to handle promises',
+      }],
+    }, {
+      code: 'user.save().finally(() => this._finallyDo());',
+      parserOptions: { ecmaVersion: 6 },
+      errors: [{
+        message: 'Use named functions defined on objects to handle promises',
+      }],
+    }, {
+      code: 'user.save().then(() => user.reload());',
+      parserOptions: { ecmaVersion: 6 },
+      errors: [{
+        message: 'Use named functions defined on objects to handle promises',
+      }],
+    }, {
+      code: 'user.save().then(user => user.name);',
+      parserOptions: { ecmaVersion: 6 },
+      options: [{
+        allowSimpleArrowFunction: true,
+      }],
+      errors: [{
+        message: 'Use named functions defined on objects to handle promises',
+      }],
+    }
   ],
 });

tests/lib/utils/utils-test.js

@@ -94,6 +94,19 @@ describe('isArrowFunctionExpression', () => {
   });
 });
 
+describe('isConciseArrowFunctionExpressionWithCall', () => {
+  const node = parse('test = () => foo()').right;
+  const blockNode = parse('test = () => { foo() }').right;
+
+  it('should check if node is concise arrow function expression with call expression body', () => {
+    assert.ok(utils.isConciseArrowFunctionWithCallExpression(node));
+  });
+
+  it('should check if node does not have block body', () => {
+    assert.ok(!utils.isConciseArrowFunctionWithCallExpression(blockNode));
+  });
+});
+
 describe('isNewExpression', () => {
   const node = parse('new Date()');