Implement no-return-from-async rule

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)

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.

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'),

lib/rules/no-return-from-async.js

@@ -0,0 +1,69 @@
+'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({
+            node: node.body,
+            message: 'Confusing implicit return in a test with an async function'
+        });
+        return true;
+    }
+    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 isAllowedReturnStatement(node) {
+    const argument = node.argument;
+
+    if (isReturnOfUndefined(node) || argument.type === 'Literal') {
+        return true;
+    }
+
+    return false;
+}
+
+function reportIfFunctionWithBlock(context, node) {
+    const returnStatement = 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 || !hasParentMochaFunctionCall(node)) {
+            return;
+        }
+
+        if (!reportIfShortArrowFunction(context, node)) {
+            reportIfFunctionWithBlock(context, node);
+        }
+    }
+
+    return {
+        FunctionExpression: check,
+        ArrowFunctionExpression: check
+    };
+};

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
+        }
+    ]
+});