diff --git a/CHANGELOG.md b/CHANGELOG.md index 66c855711fb1..64501c5e99d8 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/packages/eslint-plugin-jest/src/index.js b/packages/eslint-plugin-jest/src/index.js index 83b270b3ca10..ccc9cdb59aa3 100644 --- a/packages/eslint-plugin-jest/src/index.js +++ b/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, }, }; diff --git a/packages/eslint-plugin-jest/src/rules/__tests__/valid_expect_in_promise.test.js b/packages/eslint-plugin-jest/src/rules/__tests__/valid_expect_in_promise.test.js new file mode 100644 index 000000000000..73bff20c5eee --- /dev/null +++ b/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()})', + ], +}); diff --git a/packages/eslint-plugin-jest/src/rules/valid_expect_in_promise.js b/packages/eslint-plugin-jest/src/rules/valid_expect_in_promise.js new file mode 100644 index 000000000000..71ebc003f5ce --- /dev/null +++ b/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); + } + }, + }; +};