diff --git a/docs/rules/valid-expect.md b/docs/rules/valid-expect.md index b01785ddc..ac8b19cb3 100644 --- a/docs/rules/valid-expect.md +++ b/docs/rules/valid-expect.md @@ -20,8 +20,56 @@ or when a matcher function was not called, e.g.: expect(true).toBeDefined; ``` +or when an async assertion was not `await`ed or returned, e.g.: + +```js +expect(Promise.resolve('Hi!')).resolves.toBe('Hi!'); +``` + This rule is enabled by default. +## Options + +```js +{ + type: 'object', + properties: { + alwaysAwait: { + type: 'boolean', + default: false, + }, + }, + additionalProperties: false, +} +``` + +### `alwaysAwait` + +Enforces to use `await` inside block statements. Using `return` will trigger a +warning. Returning one line statements with arrow functions is _always allowed_. + +Examples of **incorrect** code for the { "alwaysAwait": **true** } option: + +```js +// alwaysAwait: true +test('test1', async () => { + await expect(Promise.resolve(2)).resolves.toBeDefined(); + return expect(Promise.resolve(1)).resolves.toBe(1); // `return` statement will trigger a warning +}); +``` + +Examples of **correct** code for the { "alwaysAwait": **true** } option: + +```js +// alwaysAwait: true +test('test1', async () => { + await expect(Promise.resolve(2)).resolves.toBeDefined(); + await expect(Promise.resolve(1)).resolves.toBe(1); +}); + +test('test2', () => expect(Promise.resolve(2)).resolves.toBe(2)); +``` + ### Default configuration The following patterns are considered warnings: @@ -33,6 +81,12 @@ expect('something', 'else'); expect('something'); expect(true).toBeDefined; expect(Promise.resolve('hello')).resolves; +expect(Promise.resolve('hello')).resolves.toEqual('hello'); +Promise.resolve(expect(Promise.resolve('hello')).resolves.toEqual('hello')); +Promise.all([ + expect(Promise.resolve('hello')).resolves.toEqual('hello'), + expect(Promise.resolve('hi')).resolves.toEqual('hi'), +]); ``` The following patterns are not warnings: @@ -41,5 +95,12 @@ The following patterns are not warnings: expect('something').toEqual('something'); expect([1, 2, 3]).toEqual([1, 2, 3]); expect(true).toBeDefined(); -expect(Promise.resolve('hello')).resolves.toEqual('hello'); +await expect(Promise.resolve('hello')).resolves.toEqual('hello'); +await Promise.resolve( + expect(Promise.resolve('hello')).resolves.toEqual('hello'), +); +await Promise.all( + expect(Promise.resolve('hello')).resolves.toEqual('hello'), + expect(Promise.resolve('hi')).resolves.toEqual('hi'), +); ``` diff --git a/src/rules/__tests__/no-alias-methods.test.js b/src/rules/__tests__/no-alias-methods.test.js index 7a36fd3ee..326844cc4 100644 --- a/src/rules/__tests__/no-alias-methods.test.js +++ b/src/rules/__tests__/no-alias-methods.test.js @@ -17,6 +17,7 @@ ruleTester.run('no-alias-methods', rule, { 'expect(a).toHaveNthReturnedWith()', 'expect(a).toThrow()', 'expect(a).rejects;', + 'expect(a);', ], invalid: [ diff --git a/src/rules/__tests__/no-truthy-falsy.test.js b/src/rules/__tests__/no-truthy-falsy.test.js index 76d797741..e0b124449 100644 --- a/src/rules/__tests__/no-truthy-falsy.test.js +++ b/src/rules/__tests__/no-truthy-falsy.test.js @@ -13,6 +13,7 @@ ruleTester.run('no-truthy-falsy', rule, { 'expect("anything").not.toEqual(true);', 'expect(Promise.resolve({})).resolves.toBe(true);', 'expect(Promise.reject({})).rejects.toBe(true);', + 'expect(a);', ], invalid: [ diff --git a/src/rules/__tests__/prefer-called-with.js b/src/rules/__tests__/prefer-called-with.test.js similarity index 98% rename from src/rules/__tests__/prefer-called-with.js rename to src/rules/__tests__/prefer-called-with.test.js index 91560f72d..5a641a648 100644 --- a/src/rules/__tests__/prefer-called-with.js +++ b/src/rules/__tests__/prefer-called-with.test.js @@ -15,6 +15,7 @@ ruleTester.run('prefer-called-with', rule, { 'expect(fn).not.toHaveBeenCalledWith();', 'expect(fn).toBeCalledTimes(0);', 'expect(fn).toHaveBeenCalledTimes(0);', + 'expect(fn);', ], invalid: [ diff --git a/src/rules/__tests__/prefer-strict-equal.test.js b/src/rules/__tests__/prefer-strict-equal.test.js index ff5d874b4..bd4624e28 100644 --- a/src/rules/__tests__/prefer-strict-equal.test.js +++ b/src/rules/__tests__/prefer-strict-equal.test.js @@ -7,6 +7,7 @@ ruleTester.run('prefer-strict-equal', rule, { valid: [ 'expect(something).toStrictEqual(somethingElse);', "a().toEqual('b')", + 'expect(a);', ], invalid: [ { diff --git a/src/rules/__tests__/prefer-to-contain.test.js b/src/rules/__tests__/prefer-to-contain.test.js index a67444878..54e20a4c7 100644 --- a/src/rules/__tests__/prefer-to-contain.test.js +++ b/src/rules/__tests__/prefer-to-contain.test.js @@ -23,6 +23,7 @@ ruleTester.run('prefer-to-contain', rule, { `expect(a.test(b)).resolves.toEqual(true)`, `expect(a.test(b)).resolves.not.toEqual(true)`, `expect(a).not.toContain(b)`, + 'expect(a);', ], invalid: [ { diff --git a/src/rules/__tests__/prefer-to-have-length.test.js b/src/rules/__tests__/prefer-to-have-length.test.js index c5be836ba..a5981e7ff 100644 --- a/src/rules/__tests__/prefer-to-have-length.test.js +++ b/src/rules/__tests__/prefer-to-have-length.test.js @@ -10,6 +10,7 @@ ruleTester.run('prefer-to-have-length', rule, { 'expect(result).toBe(true);', `expect(user.getUserName(5)).resolves.toEqual('Paul')`, `expect(user.getUserName(5)).rejects.toEqual('Paul')`, + 'expect(a);', ], invalid: [ diff --git a/src/rules/__tests__/require-tothrow-message.test.js b/src/rules/__tests__/require-tothrow-message.test.js index c91a39cdb..4a3a6e445 100644 --- a/src/rules/__tests__/require-tothrow-message.test.js +++ b/src/rules/__tests__/require-tothrow-message.test.js @@ -57,6 +57,7 @@ ruleTester.run('require-tothrow-message', rule, { await expect(throwErrorAsync()).resolves.not.toThrow(); await expect(throwErrorAsync()).resolves.not.toThrowError(); })`, + 'expect(a);', ], invalid: [ diff --git a/src/rules/__tests__/valid-expect.test.js b/src/rules/__tests__/valid-expect.test.js index 8b3192375..92ac5cf2b 100644 --- a/src/rules/__tests__/valid-expect.test.js +++ b/src/rules/__tests__/valid-expect.test.js @@ -1,7 +1,11 @@ import { RuleTester } from 'eslint'; import rule from '../valid-expect'; -const ruleTester = new RuleTester(); +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 8, + }, +}); ruleTester.run('valid-expect', rule, { valid: [ @@ -9,8 +13,67 @@ ruleTester.run('valid-expect', rule, { 'expect(true).toBeDefined();', 'expect([1, 2, 3]).toEqual([1, 2, 3]);', 'expect(undefined).not.toBeDefined();', - 'expect(Promise.resolve(2)).resolves.toBeDefined();', - 'expect(Promise.reject(2)).rejects.toBeDefined();', + { + code: + 'test("valid-expect", () => { return expect(Promise.resolve(2)).resolves.toBeDefined(); });', + options: [{ alwaysAwait: false }], + }, + { + code: + 'test("valid-expect", () => { return expect(Promise.reject(2)).rejects.toBeDefined(); });', + options: [{ alwaysAwait: false }], + }, + { + code: + 'test("valid-expect", () => { return expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', + options: [{ alwaysAwait: false }], + }, + { + code: + 'test("valid-expect", () => { return expect(Promise.resolve(2)).not.rejects.toBeDefined(); });', + options: [{ alwaysAwait: false }], + }, + { + code: + 'test("valid-expect", function () { return expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', + options: [{ alwaysAwait: false }], + }, + { + code: + 'test("valid-expect", function () { return expect(Promise.resolve(2)).not.rejects.toBeDefined(); });', + options: [{ alwaysAwait: false }], + }, + { + code: + 'test("valid-expect", function () { return Promise.resolve(expect(Promise.resolve(2)).not.resolves.toBeDefined()); });', + options: [{ alwaysAwait: false }], + }, + { + code: + 'test("valid-expect", function () { return Promise.resolve(expect(Promise.resolve(2)).not.rejects.toBeDefined()); });', + options: [{ alwaysAwait: false }], + }, + { + code: + 'test("valid-expect", () => expect(Promise.resolve(2)).resolves.toBeDefined());', + options: [{ alwaysAwait: true }], + }, + 'test("valid-expect", () => expect(Promise.resolve(2)).resolves.toBeDefined());', + 'test("valid-expect", () => expect(Promise.reject(2)).rejects.toBeDefined());', + 'test("valid-expect", () => expect(Promise.reject(2)).not.resolves.toBeDefined());', + 'test("valid-expect", () => expect(Promise.reject(2)).not.rejects.toBeDefined());', + 'test("valid-expect", () => expect(Promise.reject(2)).resolves.not.toBeDefined());', + 'test("valid-expect", () => expect(Promise.reject(2)).rejects.not.toBeDefined());', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).not.resolves.toBeDefined(); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).not.rejects.toBeDefined(); });', + 'test("valid-expect", async function () { await expect(Promise.reject(2)).not.resolves.toBeDefined(); });', + 'test("valid-expect", async function () { await expect(Promise.reject(2)).not.rejects.toBeDefined(); });', + 'test("valid-expect", async () => { await Promise.resolve(expect(Promise.reject(2)).not.rejects.toBeDefined()); });', + 'test("valid-expect", async () => { await Promise.reject(expect(Promise.reject(2)).not.rejects.toBeDefined()); });', + 'test("valid-expect", async () => { await Promise.all([expect(Promise.reject(2)).not.rejects.toBeDefined(), expect(Promise.reject(2)).not.rejects.toBeDefined()]); });', + 'test("valid-expect", async () => { await Promise.race([expect(Promise.reject(2)).not.rejects.toBeDefined(), expect(Promise.reject(2)).rejects.not.toBeDefined()]); });', + 'test("valid-expect", async () => { await Promise.allSettled([expect(Promise.reject(2)).not.rejects.toBeDefined(), expect(Promise.reject(2)).rejects.not.toBeDefined()]); });', + 'test("valid-expect", async () => { await Promise.any([expect(Promise.reject(2)).not.rejects.toBeDefined(), expect(Promise.reject(2)).rejects.not.toBeDefined()]); });', ], invalid: [ @@ -103,5 +166,341 @@ ruleTester.run('valid-expect', rule, { }, ], }, + /** + * .resolves & .rejects checks + */ + // Inline usages + { + code: 'expect(Promise.resolve(2)).resolves.toBeDefined();', + errors: [ + { + column: 1, + endColumn: 50, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + { + code: 'expect(Promise.resolve(2)).rejects.toBeDefined();', + errors: [ + { + column: 1, + endColumn: 49, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + // alwaysAwait option changes error message + { + code: 'expect(Promise.resolve(2)).resolves.toBeDefined();', + options: [{ alwaysAwait: true }], + errors: [ + { + column: 1, + endColumn: 50, + message: 'Async assertions must be awaited.', + }, + ], + }, + + // expect().resolves + { + code: + 'test("valid-expect", () => { expect(Promise.resolve(2)).resolves.toBeDefined(); });', + errors: [ + { + column: 30, + endColumn: 79, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + // expect().not.resolves + { + code: + 'test("valid-expect", () => { expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', + errors: [ + { + column: 30, + endColumn: 83, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + // expect().rejects + { + code: + 'test("valid-expect", () => { expect(Promise.resolve(2)).rejects.toBeDefined(); });', + errors: [ + { + column: 30, + endColumn: 78, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + // expect().not.rejects + { + code: + 'test("valid-expect", () => { expect(Promise.resolve(2)).not.rejects.toBeDefined(); });', + errors: [ + { + column: 30, + endColumn: 82, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + // usages in async function + { + code: + 'test("valid-expect", async () => { expect(Promise.resolve(2)).resolves.toBeDefined(); });', + errors: [ + { + column: 36, + endColumn: 85, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + { + code: + 'test("valid-expect", async () => { expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', + errors: [ + { + column: 36, + endColumn: 89, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + // alwaysAwait:false, one not awaited + { + code: `test("valid-expect", async () => { + expect(Promise.resolve(2)).not.resolves.toBeDefined(); + expect(Promise.resolve(1)).rejects.toBeDefined(); + });`, + errors: [ + { + line: 2, + column: 11, + endColumn: 64, + message: 'Async assertions must be awaited or returned.', + }, + { + line: 3, + column: 11, + endColumn: 59, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + // alwaysAwait: true, one returned + { + code: `test("valid-expect", async () => { + await expect(Promise.resolve(2)).not.resolves.toBeDefined(); + expect(Promise.resolve(1)).rejects.toBeDefined(); + });`, + errors: [ + { + line: 3, + column: 11, + endColumn: 59, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + /** + * Multiple async assertions + */ + // both not awaited + { + code: `test("valid-expect", async () => { + expect(Promise.resolve(2)).not.resolves.toBeDefined(); + return expect(Promise.resolve(1)).rejects.toBeDefined(); + });`, + options: [{ alwaysAwait: true }], + errors: [ + { + line: 2, + column: 11, + endColumn: 64, + message: 'Async assertions must be awaited.', + }, + { + line: 3, + column: 18, + endColumn: 66, + message: 'Async assertions must be awaited.', + }, + ], + }, + // alwaysAwait:true, one not awaited, one returned + { + code: `test("valid-expect", async () => { + expect(Promise.resolve(2)).not.resolves.toBeDefined(); + return expect(Promise.resolve(1)).rejects.toBeDefined(); + });`, + options: [{ alwaysAwait: false }], + errors: [ + { + line: 2, + column: 11, + endColumn: 64, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + // one not awaited + { + code: `test("valid-expect", async () => { + await expect(Promise.resolve(2)).not.resolves.toBeDefined(); + return expect(Promise.resolve(1)).rejects.toBeDefined(); + });`, + options: [{ alwaysAwait: true }], + errors: [ + { + line: 3, + column: 18, + endColumn: 66, + message: 'Async assertions must be awaited.', + }, + ], + }, + + /** + * Promise.x(expect()) usages + */ + { + code: `test("valid-expect", () => { + Promise.resolve(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + });`, + errors: [ + { + line: 2, + column: 11, + endColumn: 81, + message: + 'Promises which return async assertions must be awaited or returned.', + }, + ], + }, + { + code: `test("valid-expect", () => { + Promise.reject(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + });`, + errors: [ + { + line: 2, + column: 11, + endColumn: 80, + message: + 'Promises which return async assertions must be awaited or returned.', + }, + ], + }, + { + code: `test("valid-expect", () => { + Promise.x(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + });`, + errors: [ + { + line: 2, + column: 11, + endColumn: 75, + message: + 'Promises which return async assertions must be awaited or returned.', + }, + ], + }, + // alwaysAwait option changes error message + { + code: `test("valid-expect", () => { + Promise.resolve(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + });`, + options: [{ alwaysAwait: true }], + errors: [ + { + line: 2, + column: 11, + endColumn: 81, + message: 'Promises which return async assertions must be awaited.', + }, + ], + }, + // Promise method accepts arrays and returns 1 error + { + code: `test("valid-expect", () => { + Promise.all([ + expect(Promise.resolve(2)).not.resolves.toBeDefined(), + expect(Promise.resolve(3)).not.resolves.toBeDefined(), + ]); + });`, + errors: [ + { + line: 2, + column: 11, + endLine: 5, + endColumn: 13, + message: + 'Promises which return async assertions must be awaited or returned.', + }, + ], + }, + // Promise.any([expect1, expect2]) returns one error + { + code: `test("valid-expect", () => { + Promise.x([ + expect(Promise.resolve(2)).not.resolves.toBeDefined(), + expect(Promise.resolve(3)).not.resolves.toBeDefined(), + ]); + });`, + errors: [ + { + line: 2, + column: 11, + endLine: 5, + endColumn: 13, + message: + 'Promises which return async assertions must be awaited or returned.', + }, + ], + }, + // + { + code: `test("valid-expect", () => { + const assertions = [ + expect(Promise.resolve(2)).not.resolves.toBeDefined(), + expect(Promise.resolve(3)).not.resolves.toBeDefined(), + ] + });`, + errors: [ + { + line: 3, + column: 13, + endLine: 3, + endColumn: 66, + message: 'Async assertions must be awaited or returned.', + }, + { + line: 4, + column: 13, + endLine: 4, + endColumn: 66, + message: 'Async assertions must be awaited or returned.', + }, + ], + }, + // Code coverage for line 29 + { + code: 'expect(Promise.resolve(2)).resolves.toBe;', + errors: [ + { + column: 37, + endColumn: 41, + message: '"toBe" was not called.', + }, + ], + }, ], }); diff --git a/src/rules/no-alias-methods.js b/src/rules/no-alias-methods.js index b27d2ed7c..80a5cc0a3 100644 --- a/src/rules/no-alias-methods.js +++ b/src/rules/no-alias-methods.js @@ -1,4 +1,4 @@ -import { expectCase, getDocsUrl, method } from './util'; +import { expectCaseWithParent, getDocsUrl, method } from './util'; export default { meta: { @@ -30,7 +30,7 @@ export default { return { CallExpression(node) { - if (!expectCase(node)) { + if (!expectCaseWithParent(node)) { return; } diff --git a/src/rules/no-truthy-falsy.js b/src/rules/no-truthy-falsy.js index 4fcbe2df0..27cdf715f 100644 --- a/src/rules/no-truthy-falsy.js +++ b/src/rules/no-truthy-falsy.js @@ -1,8 +1,8 @@ import { - expectCase, + expectCaseWithParent, expectNotCase, - expectRejectCase, - expectResolveCase, + expectRejectsCase, + expectResolvesCase, getDocsUrl, method, } from './util'; @@ -21,10 +21,10 @@ export default { return { CallExpression(node) { if ( - expectCase(node) || + expectCaseWithParent(node) || expectNotCase(node) || - expectResolveCase(node) || - expectRejectCase(node) + expectResolvesCase(node) || + expectRejectsCase(node) ) { const targetNode = node.parent.parent.type === 'MemberExpression' ? node.parent : node; diff --git a/src/rules/prefer-called-with.js b/src/rules/prefer-called-with.js index 182375d87..0e4f99554 100644 --- a/src/rules/prefer-called-with.js +++ b/src/rules/prefer-called-with.js @@ -1,4 +1,9 @@ -import { expectCase, expectNotCase, getDocsUrl, method } from './util'; +import { + expectCaseWithParent, + expectNotCase, + getDocsUrl, + method, +} from './util'; export default { meta: { @@ -14,7 +19,7 @@ export default { return { CallExpression(node) { // Could check resolves/rejects here but not a likely idiom. - if (expectCase(node) && !expectNotCase(node)) { + if (expectCaseWithParent(node) && !expectNotCase(node)) { const methodNode = method(node); const { name } = methodNode; if (name === 'toBeCalled' || name === 'toHaveBeenCalled') { diff --git a/src/rules/prefer-to-contain.js b/src/rules/prefer-to-contain.js index 205a81d7c..ba59f6c5b 100644 --- a/src/rules/prefer-to-contain.js +++ b/src/rules/prefer-to-contain.js @@ -1,8 +1,8 @@ import { argument, - expectCase, - expectRejectCase, - expectResolveCase, + expectCaseWithParent, + expectRejectsCase, + expectResolvesCase, getDocsUrl, method, } from './util'; @@ -91,8 +91,8 @@ export default { return { CallExpression(node) { if ( - !(expectResolveCase(node) || expectRejectCase(node)) && - expectCase(node) && + !(expectResolvesCase(node) || expectRejectsCase(node)) && + expectCaseWithParent(node) && (isEqualityNegation(node) || isValidEqualityCheck(node)) && isValidIncludesMethod(node) ) { diff --git a/src/rules/prefer-to-have-length.js b/src/rules/prefer-to-have-length.js index 905b800b8..bc8792602 100644 --- a/src/rules/prefer-to-have-length.js +++ b/src/rules/prefer-to-have-length.js @@ -1,8 +1,8 @@ import { - expectCase, + expectCaseWithParent, expectNotCase, - expectRejectCase, - expectResolveCase, + expectRejectsCase, + expectResolvesCase, getDocsUrl, method, } from './util'; @@ -24,10 +24,10 @@ export default { if ( !( expectNotCase(node) || - expectResolveCase(node) || - expectRejectCase(node) + expectResolvesCase(node) || + expectRejectsCase(node) ) && - expectCase(node) && + expectCaseWithParent(node) && (method(node).name === 'toBe' || method(node).name === 'toEqual') && node.arguments[0].property && node.arguments[0].property.name === 'length' diff --git a/src/rules/require-tothrow-message.js b/src/rules/require-tothrow-message.js index 449a348a9..5e609fe5d 100644 --- a/src/rules/require-tothrow-message.js +++ b/src/rules/require-tothrow-message.js @@ -1,4 +1,4 @@ -import { argument, expectCase, getDocsUrl, method } from './util'; +import { argument, expectCaseWithParent, getDocsUrl, method } from './util'; export default { meta: { @@ -13,7 +13,7 @@ export default { create(context) { return { CallExpression(node) { - if (!expectCase(node)) { + if (!expectCaseWithParent(node)) { return; } diff --git a/src/rules/util.js b/src/rules/util.js index 4d1eaecd4..44b41f0e4 100644 --- a/src/rules/util.js +++ b/src/rules/util.js @@ -4,8 +4,10 @@ import { version } from '../../package.json'; const REPO_URL = 'https://github.com/jest-community/eslint-plugin-jest'; export const expectCase = node => - node.callee.name === 'expect' && - node.arguments.length === 1 && + node && node.callee && node.callee.name === 'expect'; + +export const expectCaseWithParent = node => + expectCase(node) && node.parent && node.parent.type === 'MemberExpression' && node.parent.parent; @@ -15,18 +17,32 @@ export const expectNotCase = node => node.parent.parent.type === 'MemberExpression' && methodName(node) === 'not'; -export const expectResolveCase = node => +export const expectResolvesCase = node => expectCase(node) && node.parent.parent.type === 'MemberExpression' && - methodName(node) === 'resolve'; + methodName(node) === 'resolves'; + +export const expectNotResolvesCase = node => + expectNotCase(node) && + node.parent.parent.type === 'MemberExpression' && + methodName(node.parent) === 'resolves'; -export const expectRejectCase = node => +export const expectRejectsCase = node => expectCase(node) && node.parent.parent.type === 'MemberExpression' && - methodName(node) === 'reject'; + methodName(node) === 'rejects'; + +export const expectNotRejectsCase = node => + expectNotCase(node) && + node.parent.parent.type === 'MemberExpression' && + methodName(node.parent) === 'rejects'; export const expectToBeCase = (node, arg) => - !(expectNotCase(node) || expectResolveCase(node) || expectRejectCase(node)) && + !( + expectNotCase(node) || + expectResolvesCase(node) || + expectRejectsCase(node) + ) && expectCase(node) && methodName(node) === 'toBe' && argument(node) && @@ -45,7 +61,11 @@ export const expectNotToBeCase = (node, arg) => (argument2(node).name === 'undefined' && arg === undefined)); export const expectToEqualCase = (node, arg) => - !(expectNotCase(node) || expectResolveCase(node) || expectRejectCase(node)) && + !( + expectNotCase(node) || + expectResolvesCase(node) || + expectRejectsCase(node) + ) && expectCase(node) && methodName(node) === 'toEqual' && argument(node) && diff --git a/src/rules/valid-expect.js b/src/rules/valid-expect.js index 21d5fb30e..ec46e7ddc 100644 --- a/src/rules/valid-expect.js +++ b/src/rules/valid-expect.js @@ -3,9 +3,77 @@ * MIT license, Tom Vincent. */ -import { getDocsUrl } from './util'; +import { + expectCase, + expectNotRejectsCase, + expectNotResolvesCase, + expectRejectsCase, + expectResolvesCase, + getDocsUrl, +} from './util'; const expectProperties = ['not', 'resolves', 'rejects']; +const promiseArgumentTypes = ['CallExpression', 'ArrayExpression']; + +/** + * expect statement can have chained matchers. + * We are looking for the closest CallExpression + * to find `expect().x.y.z.t()` usage. + * + * @Returns CallExpressionNode + */ +const getClosestParentCallExpressionNode = node => { + if (!node || !node.parent || !node.parent.parent) { + return null; + } + + if (node.parent.type === 'CallExpression') { + return node.parent; + } + return getClosestParentCallExpressionNode(node.parent); +}; + +/** + * Async assertions might be called in Promise + * methods like `Promise.x(expect1)` or `Promise.x([expect1, expect2])`. + * If that's the case, Promise node have to be awaited or returned. + * + * @Returns CallExpressionNode + */ +const getPromiseCallExpressionNode = node => { + if ( + node && + node.type === 'ArrayExpression' && + node.parent && + node.parent.type === 'CallExpression' + ) { + node = node.parent; + } + + if ( + node.type === 'CallExpression' && + node.callee && + node.callee.type === 'MemberExpression' && + node.callee.object.name === 'Promise' && + node.parent + ) { + return node; + } + + return null; +}; + +const checkIfValidReturn = (parentCallExpressionNode, allowReturn) => { + const validParentNodeTypes = ['ArrowFunctionExpression', 'AwaitExpression']; + if (allowReturn) { + validParentNodeTypes.push('ReturnStatement'); + } + + return validParentNodeTypes.includes(parentCallExpressionNode.type); +}; + +const promiseArrayExceptionKey = ({ start, end }) => + `${start.line}:${start.column}-${end.line}:${end.column}`; export default { meta: { @@ -20,16 +88,41 @@ export default { '"{{ propertyName }}" is not a valid property of expect.', propertyWithoutMatcher: '"{{ propertyName }}" needs to call a matcher.', matcherOnPropertyNotCalled: '"{{ propertyName }}" was not called.', + asyncMustBeAwaited: 'Async assertions must be awaited{{ orReturned }}.', + promisesWithAsyncAssertionsMustBeAwaited: + 'Promises which return async assertions must be awaited{{ orReturned }}.', }, - schema: [], + schema: [ + { + type: 'object', + properties: { + alwaysAwait: { + type: 'boolean', + default: false, + }, + }, + additionalProperties: false, + }, + ], }, create(context) { + // Context state + const arrayExceptions = {}; + + const pushPromiseArrayException = loc => { + const key = promiseArrayExceptionKey(loc); + arrayExceptions[key] = true; + }; + + const promiseArrayExceptionExists = loc => { + const key = promiseArrayExceptionKey(loc); + return !!arrayExceptions[key]; + }; + return { CallExpression(node) { - const calleeName = node.callee.name; - - if (calleeName === 'expect') { - // checking "expect()" arguments + // checking "expect()" arguments + if (expectCase(node)) { if (node.arguments.length > 1) { const secondArgumentLocStart = node.arguments[1].loc.start; const lastArgumentLocEnd = @@ -47,7 +140,7 @@ export default { node, }); } else if (node.arguments.length === 0) { - const expectLength = calleeName.length; + const expectLength = node.callee.name.length; context.report({ loc: { end: { @@ -114,6 +207,53 @@ export default { } } } + + if ( + expectResolvesCase(node) || + expectRejectsCase(node) || + expectNotResolvesCase(node) || + expectNotRejectsCase(node) + ) { + let parentNode = getClosestParentCallExpressionNode(node); + if (parentNode) { + const { options } = context; + const allowReturn = !options[0] || !options[0].alwaysAwait; + const isParentArrayExpression = + parentNode.parent.type === 'ArrayExpression'; + const orReturned = allowReturn ? ' or returned' : ''; + let messageId = 'asyncMustBeAwaited'; + + // Promise.x([expect()]) || Promise.x(expect()) + if (promiseArgumentTypes.includes(parentNode.parent.type)) { + const promiseNode = getPromiseCallExpressionNode( + parentNode.parent, + ); + + if (promiseNode) { + parentNode = promiseNode; + messageId = 'promisesWithAsyncAssertionsMustBeAwaited'; + } + } + + if ( + !checkIfValidReturn(parentNode.parent, allowReturn) && + !promiseArrayExceptionExists(parentNode.loc) + ) { + context.report({ + loc: parentNode.loc, + data: { + orReturned, + }, + messageId, + node, + }); + + if (isParentArrayExpression) { + pushPromiseArrayException(parentNode.loc); + } + } + } + } }, // nothing called on "expect()"