Repository: lo1tuma/eslint-plugin-mocha
Pull request #190 (2019-03-05 14:08:48)

Implement no-return-from-async rule
While upgrading a codebase to use async/await I've found it to be a bit confusing when the two Promise styles are mixed so a Promise is returned from an `async` test:

```js
it('should foo', async function () {
  await bar();
  return expect(somePromise).to.eventually.be.rejectedWith('bla');
});
```

I've implemented a rule that prohibits mixing these two styles, similar to to the `no-return-and-callback`. And pretty much all copy/pasted from it because of the similarities :)


Patch:
diff --git a/docs/rules/README.md b/docs/rules/README.md
index 2bfd26d..2c7ba16 100644
--- a/docs/rules/README.md
+++ b/docs/rules/README.md
@@ -11,6 +11,7 @@
 * [no-nested-tests](no-nested-tests.md) - disallow tests to be nested within other tests
 * [no-pending-tests](no-pending-tests.md) - disallow pending/unimplemented mocha tests
 * [no-return-and-callback](no-return-and-callback.md) - disallow returning in a test or hook function that uses a callback
+* [no-return-from-async](no-return-from-async.md) - disallow returning from an async test or hook
 * [no-setup-in-describe](no-setup-in-describe.md) - disallow calling functions and dot operators directly in describe blocks
 * [no-sibling-hooks](no-sibling-hooks.md) - disallow duplicate uses of a hook at the same level inside a describe
 * [no-skipped-tests](no-skipped-tests.md) - disallow skipped mocha tests (fixable)
diff --git a/docs/rules/no-return-from-async.md b/docs/rules/no-return-from-async.md
new file mode 100644
index 0000000..da63bc8
--- /dev/null
+++ b/docs/rules/no-return-from-async.md
@@ -0,0 +1,44 @@
+# Disallow returning from an async test or hook (no-return-from-async)
+
+Mocha's tests or hooks (like `before`) may be asynchronous by returning a Promise. When such a Promise-returning function is defined using [an ES7 `async` function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function) it can be confusing when combined with an explicit `return` of a Promise, as it's mixing the two styles.
+
+## Rule Details
+
+This rule looks for every test and hook (`before`, `after`, `beforeEach` and `afterEach`) and reports when the function is async and returns a value. Returning a non-Promise value is fine from Mocha's perspective, though it is ignored, but helps the linter catch more error cases.
+
+The following patterns are considered warnings:
+
+```js
+describe('suite', function () {
+    before('title', async function() {
+        return foo;
+    });
+
+    it('title', async function() {
+        return bar().then(function() {
+            quux();
+        });
+    });
+});
+```
+
+These patterns would not be considered warnings:
+
+```js
+describe('suite', function () {
+    before('title', async function() {
+        await foo();
+    });
+
+    it('title', function() {
+        if (bailEarly) {
+            return;
+        }
+        await bar();
+    });
+});
+```
+
+## When Not To Use It
+
+* If you use another library which exposes a similar API as mocha (e.g. `before`, `after`), you should turn this rule off, because it would raise warnings.
diff --git a/index.js b/index.js
index 76cb7c1..06961f3 100644
--- a/index.js
+++ b/index.js
@@ -9,6 +9,7 @@ module.exports = {
         'no-synchronous-tests': require('./lib/rules/no-synchronous-tests'),
         'no-global-tests': require('./lib/rules/no-global-tests'),
         'no-return-and-callback': require('./lib/rules/no-return-and-callback'),
+        'no-return-from-async': require('./lib/rules/no-return-from-async'),
         'valid-test-description': require('./lib/rules/valid-test-description'),
         'valid-suite-description': require('./lib/rules/valid-suite-description'),
         'no-mocha-arrows': require('./lib/rules/no-mocha-arrows'),
diff --git a/lib/rules/handle-done-callback.js b/lib/rules/handle-done-callback.js
index 4eca3c5..bd7cdfc 100644
--- a/lib/rules/handle-done-callback.js
+++ b/lib/rules/handle-done-callback.js
@@ -4,10 +4,6 @@ const R = require('ramda');
 const astUtils = require('../util/ast');
 
 module.exports = function (context) {
-    function hasParentMochaFunctionCall(functionExpression) {
-        return astUtils.isTestCase(functionExpression.parent) || astUtils.isHookCall(functionExpression.parent);
-    }
-
     function isAsyncFunction(functionExpression) {
         return functionExpression.params.length === 1;
     }
@@ -40,7 +36,7 @@ module.exports = function (context) {
     }
 
     function check(node) {
-        if (hasParentMochaFunctionCall(node) && isAsyncFunction(node)) {
+        if (astUtils.hasParentMochaFunctionCall(node) && isAsyncFunction(node)) {
             checkAsyncMochaFunction(node);
         }
     }
diff --git a/lib/rules/no-return-and-callback.js b/lib/rules/no-return-and-callback.js
index 79e5a1b..bc1198a 100644
--- a/lib/rules/no-return-and-callback.js
+++ b/lib/rules/no-return-and-callback.js
@@ -1,14 +1,7 @@
 'use strict';
 
-const R = require('ramda');
 const astUtils = require('../util/ast');
 
-const findReturnStatement = R.find(R.propEq('type', 'ReturnStatement'));
-
-function hasParentMochaFunctionCall(functionExpression) {
-    return astUtils.isTestCase(functionExpression.parent) || astUtils.isHookCall(functionExpression.parent);
-}
-
 function reportIfShortArrowFunction(context, node) {
     if (node.body.type !== 'BlockStatement') {
         context.report({
@@ -20,17 +13,6 @@ function reportIfShortArrowFunction(context, node) {
     return false;
 }
 
-function isExplicitUndefined(node) {
-    return node && node.type === 'Identifier' && node.name === 'undefined';
-}
-
-function isReturnOfUndefined(node) {
-    const argument = node.argument;
-    const isImplicitUndefined = argument === null;
-
-    return isImplicitUndefined || isExplicitUndefined(argument);
-}
-
 function isFunctionCallWithName(node, name) {
     return node.type === 'CallExpression' &&
         node.callee.type === 'Identifier' &&
@@ -40,7 +22,7 @@ function isFunctionCallWithName(node, name) {
 function isAllowedReturnStatement(node, doneName) {
     const argument = node.argument;
 
-    if (isReturnOfUndefined(node) || argument.type === 'Literal') {
+    if (astUtils.isReturnOfUndefined(node) || argument.type === 'Literal') {
         return true;
     }
 
@@ -48,7 +30,7 @@ function isAllowedReturnStatement(node, doneName) {
 }
 
 function reportIfFunctionWithBlock(context, node, doneName) {
-    const returnStatement = findReturnStatement(node.body.body);
+    const returnStatement = astUtils.findReturnStatement(node.body.body);
     if (returnStatement && !isAllowedReturnStatement(returnStatement, doneName)) {
         context.report({
             node: returnStatement,
@@ -59,7 +41,7 @@ function reportIfFunctionWithBlock(context, node, doneName) {
 
 module.exports = function (context) {
     function check(node) {
-        if (node.params.length === 0 || !hasParentMochaFunctionCall(node)) {
+        if (node.params.length === 0 || !astUtils.hasParentMochaFunctionCall(node)) {
             return;
         }
 
diff --git a/lib/rules/no-return-from-async.js b/lib/rules/no-return-from-async.js
new file mode 100644
index 0000000..9fa453d
--- /dev/null
+++ b/lib/rules/no-return-from-async.js
@@ -0,0 +1,51 @@
+'use strict';
+
+const astUtils = require('../util/ast');
+
+function reportIfShortArrowFunction(context, node) {
+    if (node.body.type !== 'BlockStatement') {
+        context.report({
+            node: node.body,
+            message: 'Confusing implicit return in a test with an async function'
+        });
+        return true;
+    }
+    return false;
+}
+
+function isAllowedReturnStatement(node) {
+    const argument = node.argument;
+
+    if (astUtils.isReturnOfUndefined(node) || argument.type === 'Literal') {
+        return true;
+    }
+
+    return false;
+}
+
+function reportIfFunctionWithBlock(context, node) {
+    const returnStatement = astUtils.findReturnStatement(node.body.body);
+    if (returnStatement && !isAllowedReturnStatement(returnStatement)) {
+        context.report({
+            node: returnStatement,
+            message: 'Unexpected use of `return` in a test with an async function'
+        });
+    }
+}
+
+module.exports = function (context) {
+    function check(node) {
+        if (!node.async || !astUtils.hasParentMochaFunctionCall(node)) {
+            return;
+        }
+
+        if (!reportIfShortArrowFunction(context, node)) {
+            reportIfFunctionWithBlock(context, node);
+        }
+    }
+
+    return {
+        FunctionExpression: check,
+        ArrowFunctionExpression: check
+    };
+};
diff --git a/lib/util/ast.js b/lib/util/ast.js
index 6f9a561..88d3063 100644
--- a/lib/util/ast.js
+++ b/lib/util/ast.js
@@ -77,6 +77,23 @@ function isStringLiteral(node) {
     return node && node.type === 'Literal' && typeof node.value === 'string';
 }
 
+function hasParentMochaFunctionCall(functionExpression) {
+    return isTestCase(functionExpression.parent) || isHookCall(functionExpression.parent);
+}
+
+function isExplicitUndefined(node) {
+    return node && node.type === 'Identifier' && node.name === 'undefined';
+}
+
+function isReturnOfUndefined(node) {
+    const argument = node.argument;
+    const isImplicitUndefined = argument === null;
+
+    return isImplicitUndefined || isExplicitUndefined(argument);
+}
+
+const findReturnStatement = R.find(R.propEq('type', 'ReturnStatement'));
+
 module.exports = {
     isDescribe,
     isHookIdentifier,
@@ -85,5 +102,8 @@ module.exports = {
     getNodeName,
     isMochaFunctionCall,
     isHookCall,
-    isStringLiteral
+    isStringLiteral,
+    hasParentMochaFunctionCall,
+    findReturnStatement,
+    isReturnOfUndefined
 };


Test patch:
diff --git a/lib/rules/no-synchronous-tests.js b/lib/rules/no-synchronous-tests.js
index 3d10efa..ec2ad49 100644
--- a/lib/rules/no-synchronous-tests.js
+++ b/lib/rules/no-synchronous-tests.js
@@ -5,11 +5,6 @@ const astUtil = require('../util/ast');
 
 const asyncMethods = [ 'async', 'callback', 'promise' ];
 
-function hasParentMochaFunctionCall(functionExpression) {
-    return astUtil.isTestCase(functionExpression.parent) ||
-            astUtil.isHookIdentifier(functionExpression.parent.callee);
-}
-
 function hasAsyncCallback(functionExpression) {
     return functionExpression.params.length === 1;
 }
@@ -44,7 +39,7 @@ module.exports = function (context) {
     const allowedAsyncMethods = R.isNil(options.allowed) ? asyncMethods : options.allowed;
 
     function check(node) {
-        if (hasParentMochaFunctionCall(node)) {
+        if (astUtil.hasParentMochaFunctionCall(node)) {
             // For each allowed async test method, check if it is used in the test
             const testAsyncMethods = allowedAsyncMethods.map(function (method) {
                 switch (method) {
diff --git a/test/rules/no-return-from-async.js b/test/rules/no-return-from-async.js
new file mode 100644
index 0000000..859c137
--- /dev/null
+++ b/test/rules/no-return-from-async.js
@@ -0,0 +1,135 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const rules = require('../../').rules;
+const ruleTester = new RuleTester();
+const message = 'Unexpected use of `return` in a test with an async function';
+const es6parserOptions = {
+    sourceType: 'module',
+    ecmaVersion: 8
+};
+
+ruleTester.run('no-return-from-async', rules['no-return-from-async'], {
+
+    valid: [
+        {
+            code: 'it("title", async function() {});',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it("title", async function() { async function other() { return foo.then(function () {}); } });',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it("title", async function() { const bar = async () => { return foo.then(function () {}); }; });',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it("title", async function() { const bar = { async a() { return foo.then(function () {}); } }; });',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it("title", async () => {});',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it.only("title", async function() {});',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'before("title", async function() {});',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'after("title", async function() {});',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'async function foo() { return foo.then(function () {}); }',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'var foo = async function() { return foo.then(function () {}); }',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'notMocha("title", async function() { return foo.then(function () {}); })',
+            parserOptions: es6parserOptions
+        },
+        // Allowed return statements
+        {
+            code: 'it("title", async function() { return; });',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it("title", async function() { return undefined; });',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it("title", async function() { return null; });',
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it("title", async function() { return "3"; });',
+            parserOptions: es6parserOptions
+        }
+    ],
+
+    invalid: [
+        {
+            code: 'it("title", async function() { return foo; });',
+            errors: [ { message, column: 32, line: 1 } ],
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it("title", async function() { return foo.then(function() {}).catch(function() {}); });',
+            errors: [ { message, column: 32, line: 1 } ],
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it("title", async function() { var foo = bar(); return foo.then(function() {}); });',
+            errors: [ { message, column: 49, line: 1 } ],
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it("title", async () => { return foo.then(function() {}).catch(function() {}); });',
+            errors: [ { message, column: 27, line: 1 } ],
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it("title", async () => foo.then(function() {}));',
+            errors: [ { message: 'Confusing implicit return in a test with an async function', column: 25, line: 1 } ],
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'it.only("title", async function() { return foo.then(function () {}); });',
+            errors: [ { message, column: 37, line: 1 } ],
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'before("title", async function() { return foo.then(function() {}); });',
+            errors: [ { message, column: 36, line: 1 } ],
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'beforeEach("title", async function() { return foo.then(function() {}); });',
+            errors: [ { message, column: 40, line: 1 } ],
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'after("title", async function() { return foo.then(function() {}); });',
+            errors: [ { message, column: 35, line: 1 } ],
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'afterEach("title", async function() { return foo.then(function() {}); });',
+            errors: [ { message, column: 39, line: 1 } ],
+            parserOptions: es6parserOptions
+        },
+        {
+            code: 'afterEach("title", async function() { return foo; });',
+            errors: [ { message, column: 39, line: 1 } ],
+            parserOptions: es6parserOptions
+        }
+    ]
+});