feat(rules): add no-test-return-statement (#136)

Adds a rule for not returning inside of a test. Related issue in Jest: jestjs/jest#6516

Author: mattphillips

Diff for: README.md

@@ -90,6 +90,7 @@ for more information about extending configuration files.
 | [no-jest-import][]           | Disallow importing `jest`                                         | ![recommended][] |                     |
 | [no-large-snapshots][]       | Disallow large snapshots                                          |                  |                     |
 | [no-test-prefixes][]         | Disallow using `f` & `x` prefixes to define focused/skipped tests |                  | ![fixable-green][]  |
+| [no-test-return-statement][] | Disallow explicitly returning from tests                          |                  |                     |
 | [prefer-expect-assertions][] | Suggest using `expect.assertions()` OR `expect.hasAssertions()`   |                  |                     |
 | [prefer-to-be-null][]        | Suggest using `toBeNull()`                                        |                  | ![fixable-green][]  |
 | [prefer-to-be-undefined][]   | Suggest using `toBeUndefined()`                                   |                  | ![fixable-green][]  |
@@ -114,6 +115,7 @@ for more information about extending configuration files.
 [no-jest-import]: docs/rules/no-jest-import.md
 [no-large-snapshots]: docs/rules/no-large-snapshots.md
 [no-test-prefixes]: docs/rules/no-test-prefixes.md
+[no-test-return-statement]: docs/rules/no-test-return-statement.md
 [prefer-expect-assertions]: docs/rules/prefer-expect-assertions.md
 [prefer-to-be-null]: docs/rules/prefer-to-be-null.md
 [prefer-to-be-undefined]: docs/rules/prefer-to-be-undefined.md

Diff for: docs/rules/no-test-return-statement.md

@@ -0,0 +1,47 @@
+# Disallow explicitly returning from tests (no-test-return-statement)
+
+Tests in Jest should be void and not return values.
+
+If you are returning Promises then you should update the test to use
+`async/await`.
+
+## Rule details
+
+This rule triggers a warning if you use a return statement inside of a test
+body.
+
+```js
+/*eslint jest/no-test-return-statement: "error"*/
+
+// valid:
+
+it('noop', function() {});
+
+test('noop', () => {});
+
+test('one arrow', () => expect(1).toBe(1));
+
+test('empty');
+
+test('one', () => {
+  expect(1).toBe(1);
+});
+
+it('one', function() {
+  expect(1).toBe(1);
+});
+
+it('returning a promise', async () => {
+  await new Promise(res => setTimeout(res, 100));
+  expect(1).toBe(1);
+});
+
+// invalid:
+test('return an expect', () => {
+  return expect(1).toBe(1);
+});
+
+it('returning a promise', function() {
+  return new Promise(res => setTimeout(res, 100)).then(() => expect(1).toBe(1));
+});
+```

Diff for: index.js

@@ -10,6 +10,7 @@ const noJasmineGlobals = require('./rules/no-jasmine-globals');
 const noJestImport = require('./rules/no-jest-import');
 const noLargeSnapshots = require('./rules/no-large-snapshots');
 const noTestPrefixes = require('./rules/no-test-prefixes');
+const noTestReturnStatement = require('./rules/no-test-return-statement');
 const preferToBeNull = require('./rules/prefer-to-be-null');
 const preferToBeUndefined = require('./rules/prefer-to-be-undefined');
 const preferToHaveLength = require('./rules/prefer-to-have-length');
@@ -75,6 +76,7 @@ module.exports = {
     'no-jest-import': noJestImport,
     'no-large-snapshots': noLargeSnapshots,
     'no-test-prefixes': noTestPrefixes,
+    'no-test-return-statement': noTestReturnStatement,
     'prefer-to-be-null': preferToBeNull,
     'prefer-to-be-undefined': preferToBeUndefined,
     'prefer-to-have-length': preferToHaveLength,

Diff for: rules/__tests__/no-test-return-statement.test.js

@@ -0,0 +1,55 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const rule = require('../no-test-return-statement');
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2015 } });
+
+ruleTester.run('no-test-prefixes', rule, {
+  valid: [
+    'it("noop", function () {});',
+    'test("noop", () => {});',
+    'test("one", () => expect(1).toBe(1));',
+    'test("empty")',
+    `
+    test("one", () => {
+      expect(1).toBe(1);
+    });
+    `,
+    `
+    it("one", function () {
+      expect(1).toBe(1);
+    });
+    `,
+  ],
+  invalid: [
+    {
+      code: `
+      test("one", () => {
+        return expect(1).toBe(1);
+      });
+      `,
+      errors: [
+        {
+          message: 'Jest tests should not return a value.',
+          column: 9,
+          line: 3,
+        },
+      ],
+    },
+    {
+      code: `
+      it("one", function () {
+        return expect(1).toBe(1);
+      });
+      `,
+      errors: [
+        {
+          message: 'Jest tests should not return a value.',
+          column: 9,
+          line: 3,
+        },
+      ],
+    },
+  ],
+});

Diff for: rules/no-test-return-statement.js

@@ -0,0 +1,43 @@
+'use strict';
+
+const getDocsUrl = require('./util').getDocsUrl;
+const isFunction = require('./util').isFunction;
+const isTestCase = require('./util').isTestCase;
+
+const MESSAGE = 'Jest tests should not return a value.';
+const RETURN_STATEMENT = 'ReturnStatement';
+const BLOCK_STATEMENT = 'BlockStatement';
+
+const getBody = args => {
+  if (
+    args.length > 1 &&
+    isFunction(args[1]) &&
+    args[1].body.type === BLOCK_STATEMENT
+  ) {
+    return args[1].body.body;
+  }
+  return [];
+};
+
+module.exports = {
+  meta: {
+    docs: {
+      url: getDocsUrl(__filename),
+    },
+  },
+  create(context) {
+    return {
+      CallExpression(node) {
+        if (!isTestCase(node)) return;
+        const body = getBody(node.arguments);
+        const returnStmt = body.find(t => t.type === RETURN_STATEMENT);
+        if (!returnStmt) return;
+
+        context.report({
+          message: MESSAGE,
+          node: returnStmt,
+        });
+      },
+    };
+  },
+};