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

README.md

@@ -89,6 +89,7 @@ for more information about extending configuration files.
 | [prefer-to-be-undefined](docs/rules/prefer-to-be-undefined.md)     | Suggest using `toBeUndefined()`                                 |                                                                         | ![fixable](https://img.shields.io/badge/-fixable-green.svg) |
 | [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` |                                                                         |                                                             |
 | [valid-expect](docs/rules/valid-expect.md)                         | Enforce valid `expect()` usage                                  | ![recommended](https://img.shields.io/badge/-recommended-lightgrey.svg) |                                                             |
+| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md)   | Enforce having return statement when testing with promises      |                                                                         |                                                             |
 
 ## Credit
 

docs/rules/valid-expect-in-promise.md

@@ -0,0 +1,33 @@
+# Enforce having return statement when testing with promises (valid-expect-in-promise)
+
+Ensure to return promise when having assertions in `then` or `catch` block of
+promise
+
+## Rule details
+
+This rule triggers a warning if,
+
+* test is having assertions in `then` or `catch` block of a promise
+* and that promise is not returned from the test
+
+### Default configuration
+
+The following pattern is considered warning:
+
+```js
+it('promise test', () => {
+  somePromise.then(data => {
+    expect(data).toEqual('foo');
+  });
+});
+```
+
+The following pattern is not warning:
+
+```js
+it('promise test', () => {
+  return somePromise.then(data => {
+    expect(data).toEqual('foo');
+  });
+});
+```

index.js

@@ -9,6 +9,7 @@ const preferToBeUndefined = require('./rules/prefer_to_be_undefined');
 const preferToHaveLength = require('./rules/prefer_to_have_length');
 const validExpect = require('./rules/valid_expect');
 const preferExpectAssertions = require('./rules/prefer_expect_assertions');
+const validExpectInPromise = require('./rules/valid_expect_in_promise');
 
 const snapshotProcessor = require('./processors/snapshot-processor');
 
@@ -64,5 +65,6 @@ module.exports = {
     'prefer-to-have-length': preferToHaveLength,
     'valid-expect': validExpect,
     'prefer-expect-assertions': preferExpectAssertions,
+    'valid-expect-in-promise': validExpectInPromise,
   },
 };

rules/__tests__/valid_expect_in_promise.test.js

@@ -0,0 +1,152 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const rules = require('../..').rules;
+
+const ruleTester = new RuleTester({
+  parserOptions: {
+    ecmaVersion: 8,
+  },
+});
+
+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,
+        },
+      ],
+    },
+    {
+      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: [
+    'it("it1", () => { return somePromise.then(() => {expect(someThing).toEqual(true)})})',
+
+    '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()})',
+
+    'it("it1", async () => { await Promise.resolve().then(' +
+      'function() {expect(someThing).toEqual(true)})})',
+
+    'it("it1", async () => ' +
+      '{ await somePromise.then(() => {expect(someThing).toEqual(true)})})',
+
+    'it("it1", async () => { await getSomeThing().getPromise().then(' +
+      'function() {expect(someThing).toEqual(true)})})',
+
+    "it('it1', () => {return somePromise." +
+      'then(() => {expect(someThing).toEqual(true);})' +
+      '.then(() => {expect(someThing).toEqual(true);})});',
+
+    "it('it1', () => {return somePromise." +
+      'then(() => {expect(someThing).toEqual(true);})' +
+      '.catch(() => {expect(someThing).toEqual(false);})});',
+  ],
+});

rules/valid_expect_in_promise.js

@@ -0,0 +1,97 @@
+'use strict';
+
+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, node) => {
+  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 isParentThenOrReturn = node => {
+  try {
+    return (
+      node.parent.parent.type == 'ReturnStatement' ||
+      node.parent.parent.property.name === 'then' ||
+      node.parent.parent.property.name === 'catch'
+    );
+  } catch (e) {
+    return false;
+  }
+};
+
+const verifyExpectWithReturn = (argument, node, context) => {
+  if (
+    argument &&
+    isFunction(argument.type) &&
+    isBodyCallExpression(argument.body)
+  ) {
+    const calleeInThenOrCatch = argument.body.body[0].expression.callee.object;
+    if (isExpectCall(calleeInThenOrCatch)) {
+      if (!isParentThenOrReturn(node)) {
+        reportReturnRequired(context, node);
+      }
+    }
+  }
+};
+
+const isAwaitExpression = node => {
+  return node.parent.parent && node.parent.parent.type == 'AwaitExpression';
+};
+
+module.exports = context => {
+  return {
+    MemberExpression(node) {
+      if (
+        node.type == 'MemberExpression' &&
+        isThenOrCatch(node) &&
+        node.parent.type == 'CallExpression' &&
+        !isAwaitExpression(node)
+      ) {
+        const parent = node.parent;
+        const arg1 = parent.arguments[0];
+        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);
+      }
+    },
+  };
+};