Introduce a lint rule to report error when testing promises. If a exp…

CHANGELOG.md

@@ -21,6 +21,7 @@
 * `[expect]` Keep the stack trace unchanged when `PrettyFormatPluginError` is thrown by pretty-format ([#4787](https://github.com/facebook/jest/pull/4787))
 
 ### Features
+* `[eslint-plugin-jest]` Add `valid-expect-in-promise` lint rule. ([#4844](https://github.com/facebook/jest/pull/4844))
 * `[eslint-plugin-jest]` Add `prefer-to-have-length` lint rule. ([#4771](https://github.com/facebook/jest/pull/4771))
 * `[jest-environment-jsdom]` [**BREAKING**] Upgrade to JSDOM@11 ([#4770](https://github.com/facebook/jest/pull/4770))
 * `[jest-environment-*]` [**BREAKING**] Add Async Test Environment APIs, dispose is now teardown ([#4506](https://github.com/facebook/jest/pull/4506))

packages/eslint-plugin-jest/src/index.js

@@ -12,6 +12,7 @@ import noFocusedTests from './rules/no_focused_tests';
 import noIdenticalTitle from './rules/no_identical_title';
 import preferToHaveLength from './rules/prefer_to_have_length';
 import validExpect from './rules/valid_expect';
+import validExpectInPromise from './rules/valid_expect_in_promise';
 
 module.exports = {
   configs: {
@@ -22,6 +23,7 @@ module.exports = {
         'jest/no-identical-title': 'error',
         'jest/prefer-to-have-length': 'warn',
         'jest/valid-expect': 'error',
+        'jest/valid-expect-in-promise': 'error',
       },
     },
   },
@@ -54,5 +56,6 @@ module.exports = {
     'no-identical-title': noIdenticalTitle,
     'prefer-to-have-length': preferToHaveLength,
     'valid-expect': validExpect,
+    'valid-expect-in-promise': validExpectInPromise,
   },
 };

packages/eslint-plugin-jest/src/rules/__tests__/valid_expect_in_promise.test.js

@@ -0,0 +1,142 @@
+/**
+ * Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+import {RuleTester} from 'eslint';
+const {rules} = require('../../');
+
+const ruleTester = new RuleTester();
+const expectedMsg =
+  'Promise should be returned to test its fulfillment or rejection';
+
+ruleTester.run('valid-expect-in-promise', rules['valid-expect-in-promise'], {
+  invalid: [
+    {
+      code:
+        'it("it1", () => { somePromise.then(' +
+        '() => {expect(someThing).toEqual(true)})})',
+      errors: [
+        {
+          column: 19,
+          endColumn: 76,
+          message: expectedMsg,
+        },
+      ],
+      parserOptions: {ecmaVersion: 6},
+    },
+    {
+      code:
+        'it("it1", function() { getSomeThing().getPromise().then(' +
+        'function() {expect(someThing).toEqual(true)})})',
+      errors: [
+        {
+          column: 24,
+          endColumn: 102,
+          message: expectedMsg,
+        },
+      ],
+    },
+    {
+      code:
+        'it("it1", function() { Promise.resolve().then(' +
+        'function() {expect(someThing).toEqual(true)})})',
+      errors: [
+        {
+          column: 24,
+          endColumn: 92,
+          message: expectedMsg,
+        },
+      ],
+    },
+    {
+      code:
+        'it("it1", function() { somePromise.catch(' +
+        'function() {expect(someThing).toEqual(true)})})',
+      errors: [
+        {
+          column: 24,
+          endColumn: 87,
+          message: expectedMsg,
+        },
+      ],
+    },
+    {
+      code:
+        'it("it1", function() { somePromise.then(' +
+        'function() { expect(someThing).toEqual(true)})})',
+      errors: [
+        {
+          column: 24,
+          endColumn: 87,
+          message: expectedMsg,
+        },
+      ],
+    },
+    {
+      code:
+        'it("it1", function() { Promise.resolve().then(' +
+        'function() { /*fulfillment*/ expect(someThing).toEqual(true)}, ' +
+        'function() { /*rejection*/ expect(someThing).toEqual(true)})})',
+      errors: [
+        {
+          column: 24,
+          endColumn: 170,
+          message: expectedMsg,
+        },
+        {
+          column: 24,
+          endColumn: 170,
+          message: expectedMsg,
+        },
+      ],
+    },
+    {
+      code:
+        'it("it1", function() { Promise.resolve().then(' +
+        'function() { /*fulfillment*/}, ' +
+        'function() { /*rejection*/ expect(someThing).toEqual(true)})})',
+      errors: [
+        {
+          column: 24,
+          endColumn: 138,
+          message: expectedMsg,
+        },
+      ],
+    },
+  ],
+
+  valid: [
+    {
+      code:
+        'it("it1", () => { return somePromise.then(() => {expect(someThing).toEqual(true)})})',
+      parserOptions: {sourceType: 'module'},
+    },
+
+    'it("it1", function() { return somePromise.catch(' +
+      'function() {expect(someThing).toEqual(true)})})',
+
+    'it("it1", function() { somePromise.then(' +
+      'function() {doSomeThingButNotExpect()})})',
+
+    'it("it1", function() { return getSomeThing().getPromise().then(' +
+      'function() {expect(someThing).toEqual(true)})})',
+
+    'it("it1", function() { return Promise.resolve().then(' +
+      'function() {expect(someThing).toEqual(true)})})',
+
+    'it("it1", function() { return Promise.resolve().then(' +
+      'function() { /*fulfillment*/ expect(someThing).toEqual(true)}, ' +
+      'function() { /*rejection*/ expect(someThing).toEqual(true)})})',
+
+    'it("it1", function() { return Promise.resolve().then(' +
+      'function() { /*fulfillment*/}, ' +
+      'function() { /*rejection*/ expect(someThing).toEqual(true)})})',
+
+    'it("it1", function() { return somePromise.then()})',
+  ],
+});

packages/eslint-plugin-jest/src/rules/valid_expect_in_promise.js

@@ -0,0 +1,96 @@
+/**
+ * Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *
+ * @flow
+ */
+
+import type {EslintContext, MemberExpression} from './types';
+
+const reportMsg =
+  'Promise should be returned to test its fulfillment or rejection';
+
+const isThenOrCatch = node => {
+  return node.property.name == 'then' || node.property.name == 'catch';
+};
+
+const isFunction = type => {
+  return type == 'FunctionExpression' || type == 'ArrowFunctionExpression';
+};
+
+const isBodyCallExpression = argumentBody => {
+  try {
+    return argumentBody.body[0].expression.type == 'CallExpression';
+  } catch (e) {
+    return false;
+  }
+};
+
+const isExpectCall = calleeObject => {
+  try {
+    return calleeObject.callee.name == 'expect';
+  } catch (e) {
+    return false;
+  }
+};
+
+const reportReturnRequired = (
+  context: EslintContext,
+  node: MemberExpression,
+) => {
+  context.report({
+    loc: {
+      end: {
+        column: node.parent.parent.loc.end.column,
+        line: node.parent.parent.loc.end.line,
+      },
+      start: node.parent.parent.loc.start,
+    },
+    message: reportMsg,
+    node,
+  });
+};
+
+const verifyExpectWithReturn = (argument, node, context) => {
+  if (
+    argument &&
+    isFunction(argument.type) &&
+    //$FlowFixMe
+    isBodyCallExpression(argument.body)
+  ) {
+    const calleeInThenOrCatch = argument.body.body[0].expression.callee.object;
+    if (isExpectCall(calleeInThenOrCatch)) {
+      if (node.parent.parent.type != 'ReturnStatement') {
+        reportReturnRequired(context, node);
+      }
+    }
+  }
+};
+
+export default (context: EslintContext) => {
+  return {
+    MemberExpression(node: MemberExpression) {
+      if (
+        node.type == 'MemberExpression' &&
+        isThenOrCatch(node) &&
+        node.parent.type == 'CallExpression'
+      ) {
+        const parent = node.parent;
+        // $FlowFixMe
+        const arg1 = parent.arguments[0];
+        // $FlowFixMe
+        const arg2 = parent.arguments[1];
+
+        // then block can have two args, fulfillment & rejection
+        // then block can have one args, fulfillment
+        // catch block can have one args, rejection
+        // ref: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise
+        verifyExpectWithReturn(arg1, node, context);
+        verifyExpectWithReturn(arg2, node, context);
+      }
+    },
+  };
+};