diff --git a/src/rules/__tests__/valid-expect.test.js b/src/rules/__tests__/valid-expect.test.ts similarity index 71% rename from src/rules/__tests__/valid-expect.test.js rename to src/rules/__tests__/valid-expect.test.ts index a10caea02..eff761e31 100644 --- a/src/rules/__tests__/valid-expect.test.js +++ b/src/rules/__tests__/valid-expect.test.ts @@ -1,7 +1,7 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../valid-expect'; -const ruleTester = new RuleTester({ +const ruleTester = new TSESLint.RuleTester({ parserOptions: { ecmaVersion: 8, }, @@ -15,12 +15,12 @@ ruleTester.run('valid-expect', rule, { 'expect(undefined).not.toBeDefined();', 'test("valid-expect", () => { return expect(Promise.resolve(2)).resolves.toBeDefined(); });', 'test("valid-expect", () => { return expect(Promise.reject(2)).rejects.toBeDefined(); });', - 'test("valid-expect", () => { return expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', - 'test("valid-expect", () => { return expect(Promise.resolve(2)).not.rejects.toBeDefined(); });', - 'test("valid-expect", function () { return expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', - 'test("valid-expect", function () { return expect(Promise.resolve(2)).not.rejects.toBeDefined(); });', - 'test("valid-expect", function () { return Promise.resolve(expect(Promise.resolve(2)).not.resolves.toBeDefined()); });', - 'test("valid-expect", function () { return Promise.resolve(expect(Promise.resolve(2)).not.rejects.toBeDefined()); });', + 'test("valid-expect", () => { return expect(Promise.resolve(2)).resolves.not.toBeDefined(); });', + 'test("valid-expect", () => { return expect(Promise.resolve(2)).rejects.not.toBeDefined(); });', + 'test("valid-expect", function () { return expect(Promise.resolve(2)).resolves.not.toBeDefined(); });', + 'test("valid-expect", function () { return expect(Promise.resolve(2)).rejects.not.toBeDefined(); });', + 'test("valid-expect", function () { return Promise.resolve(expect(Promise.resolve(2)).resolves.not.toBeDefined()); });', + 'test("valid-expect", function () { return Promise.resolve(expect(Promise.resolve(2)).rejects.not.toBeDefined()); });', { code: 'test("valid-expect", () => expect(Promise.resolve(2)).resolves.toBeDefined());', @@ -28,10 +28,52 @@ ruleTester.run('valid-expect', rule, { }, '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", () => expect(Promise.reject(2)).not.resolves.toBeDefined());', + // 'test("valid-expect", () => expect(Promise.reject(2)).not.rejects.toBeDefined());', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined(); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).rejects.not.toBeDefined(); });', + 'test("valid-expect", async function () { await expect(Promise.reject(2)).resolves.not.toBeDefined(); });', + 'test("valid-expect", async function () { await expect(Promise.reject(2)).rejects.not.toBeDefined(); });', + 'test("valid-expect", async () => { await Promise.resolve(expect(Promise.reject(2)).rejects.not.toBeDefined()); });', + 'test("valid-expect", async () => { await Promise.reject(expect(Promise.reject(2)).rejects.not.toBeDefined()); });', + 'test("valid-expect", async () => { await Promise.all([expect(Promise.reject(2)).rejects.not.toBeDefined(), expect(Promise.reject(2)).rejects.not.toBeDefined()]); });', + 'test("valid-expect", async () => { await Promise.race([expect(Promise.reject(2)).rejects.not.toBeDefined(), expect(Promise.reject(2)).rejects.not.toBeDefined()]); });', + 'test("valid-expect", async () => { await Promise.allSettled([expect(Promise.reject(2)).rejects.not.toBeDefined(), expect(Promise.reject(2)).rejects.not.toBeDefined()]); });', + 'test("valid-expect", async () => { await Promise.any([expect(Promise.reject(2)).rejects.not.toBeDefined(), expect(Promise.reject(2)).rejects.not.toBeDefined()]); });', + 'test("valid-expect", async () => { return expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")); });', + 'test("valid-expect", async () => { return expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")).then(() => console.log("another valid case")); });', + 'test("valid-expect", async () => { return expect(Promise.reject(2)).resolves.not.toBeDefined().catch(() => console.log("valid-case")); });', + 'test("valid-expect", async () => { return expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")).catch(() => console.log("another valid case")); });', + 'test("valid-expect", async () => { return expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => { expect(someMock).toHaveBeenCalledTimes(1); }); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")).then(() => console.log("another valid case")); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined().catch(() => console.log("valid-case")); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")).catch(() => console.log("another valid case")); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => { expect(someMock).toHaveBeenCalledTimes(1); }); });', + { + code: `test("valid-expect", () => { + return expect(functionReturningAPromise()).resolves.toEqual(1).then(() => { + return expect(Promise.resolve(2)).resolves.toBe(1); + }); + });`, + }, + { + code: `test("valid-expect", () => { + return expect(functionReturningAPromise()).resolves.toEqual(1).then(async () => { + await expect(Promise.resolve(2)).resolves.toBe(1); + }); + });`, + }, + { + code: `test("valid-expect", () => { + return expect(functionReturningAPromise()).resolves.toEqual(1).then(() => expect(Promise.resolve(2)).resolves.toBe(1)); + });`, + }, + ], + invalid: [ + /* '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(); });', @@ -52,28 +94,7 @@ ruleTester.run('valid-expect', rule, { 'test("valid-expect", async () => { await expect(Promise.reject(2)).not.resolves.toBeDefined().catch(() => console.log("valid-case")); });', 'test("valid-expect", async () => { await expect(Promise.reject(2)).not.resolves.toBeDefined().then(() => console.log("valid-case")).catch(() => console.log("another valid case")); });', 'test("valid-expect", async () => { await expect(Promise.reject(2)).not.resolves.toBeDefined().then(() => { expect(someMock).toHaveBeenCalledTimes(1); }); });', - { - code: `test("valid-expect", () => { - return expect(functionReturningAPromise()).resolves.toEqual(1).then(() => { - return expect(Promise.resolve(2)).resolves.toBe(1); - }); - });`, - }, - { - code: `test("valid-expect", () => { - return expect(functionReturningAPromise()).resolves.toEqual(1).then(async () => { - await expect(Promise.resolve(2)).resolves.toBe(1); - }); - });`, - }, - { - code: `test("valid-expect", () => { - return expect(functionReturningAPromise()).resolves.toEqual(1).then(() => expect(Promise.resolve(2)).resolves.toBe(1)); - });`, - }, - ], - - invalid: [ + */ { code: 'expect().toBe(true);', errors: [{ endColumn: 8, column: 7, messageId: 'noArgs' }], @@ -215,10 +236,10 @@ ruleTester.run('valid-expect', rule, { }, ], }, - // expect().not.resolves + // expect().resolves.not { code: - 'test("valid-expect", () => { expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', + 'test("valid-expect", () => { expect(Promise.resolve(2)).resolves.not.toBeDefined(); });', errors: [ { column: 30, @@ -241,10 +262,10 @@ ruleTester.run('valid-expect', rule, { }, ], }, - // expect().not.rejects + // expect().rejects.not { code: - 'test("valid-expect", () => { expect(Promise.resolve(2)).not.rejects.toBeDefined(); });', + 'test("valid-expect", () => { expect(Promise.resolve(2)).rejects.not.toBeDefined(); });', errors: [ { column: 30, @@ -269,7 +290,7 @@ ruleTester.run('valid-expect', rule, { }, { code: - 'test("valid-expect", async () => { expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', + 'test("valid-expect", async () => { expect(Promise.resolve(2)).resolves.not.toBeDefined(); });', errors: [ { column: 36, @@ -281,9 +302,9 @@ ruleTester.run('valid-expect', rule, { }, // alwaysAwait:false, one not awaited { - code: `test("valid-expect", async () => { - expect(Promise.resolve(2)).not.resolves.toBeDefined(); - expect(Promise.resolve(1)).rejects.toBeDefined(); + code: `test("valid-expect", async () => { + expect(Promise.resolve(2)).resolves.not.toBeDefined(); + expect(Promise.resolve(1)).rejects.toBeDefined(); });`, errors: [ { @@ -304,9 +325,9 @@ ruleTester.run('valid-expect', rule, { }, // alwaysAwait: true, one returned { - code: `test("valid-expect", async () => { - await expect(Promise.resolve(2)).not.resolves.toBeDefined(); - expect(Promise.resolve(1)).rejects.toBeDefined(); + code: `test("valid-expect", async () => { + await expect(Promise.resolve(2)).resolves.not.toBeDefined(); + expect(Promise.resolve(1)).rejects.toBeDefined(); });`, errors: [ { @@ -323,9 +344,9 @@ ruleTester.run('valid-expect', rule, { */ // both not awaited { - code: `test("valid-expect", async () => { - expect(Promise.resolve(2)).not.resolves.toBeDefined(); - return expect(Promise.resolve(1)).rejects.toBeDefined(); + code: `test("valid-expect", async () => { + expect(Promise.resolve(2)).resolves.not.toBeDefined(); + return expect(Promise.resolve(1)).rejects.toBeDefined(); });`, options: [{ alwaysAwait: true }], errors: [ @@ -345,9 +366,9 @@ ruleTester.run('valid-expect', rule, { }, // 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(); + code: `test("valid-expect", async () => { + expect(Promise.resolve(2)).resolves.not.toBeDefined(); + return expect(Promise.resolve(1)).rejects.toBeDefined(); });`, errors: [ { @@ -361,9 +382,9 @@ ruleTester.run('valid-expect', rule, { }, // one not awaited { - code: `test("valid-expect", async () => { - await expect(Promise.resolve(2)).not.resolves.toBeDefined(); - return expect(Promise.resolve(1)).rejects.toBeDefined(); + code: `test("valid-expect", async () => { + await expect(Promise.resolve(2)).resolves.not.toBeDefined(); + return expect(Promise.resolve(1)).rejects.toBeDefined(); });`, options: [{ alwaysAwait: true }], errors: [ @@ -380,8 +401,8 @@ ruleTester.run('valid-expect', rule, { * Promise.x(expect()) usages */ { - code: `test("valid-expect", () => { - Promise.resolve(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + code: `test("valid-expect", () => { + Promise.resolve(expect(Promise.resolve(2)).resolves.not.toBeDefined()); });`, errors: [ { @@ -394,8 +415,8 @@ ruleTester.run('valid-expect', rule, { ], }, { - code: `test("valid-expect", () => { - Promise.reject(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + code: `test("valid-expect", () => { + Promise.reject(expect(Promise.resolve(2)).resolves.not.toBeDefined()); });`, errors: [ { @@ -408,8 +429,8 @@ ruleTester.run('valid-expect', rule, { ], }, { - code: `test("valid-expect", () => { - Promise.x(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + code: `test("valid-expect", () => { + Promise.x(expect(Promise.resolve(2)).resolves.not.toBeDefined()); });`, errors: [ { @@ -423,8 +444,8 @@ ruleTester.run('valid-expect', rule, { }, // alwaysAwait option changes error message { - code: `test("valid-expect", () => { - Promise.resolve(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + code: `test("valid-expect", () => { + Promise.resolve(expect(Promise.resolve(2)).resolves.not.toBeDefined()); });`, options: [{ alwaysAwait: true }], errors: [ @@ -438,11 +459,11 @@ ruleTester.run('valid-expect', rule, { }, // Promise method accepts arrays and returns 1 error { - code: `test("valid-expect", () => { + code: `test("valid-expect", () => { Promise.all([ - expect(Promise.resolve(2)).not.resolves.toBeDefined(), - expect(Promise.resolve(3)).not.resolves.toBeDefined(), - ]); + expect(Promise.resolve(2)).resolves.not.toBeDefined(), + expect(Promise.resolve(3)).resolves.not.toBeDefined(), + ]); });`, errors: [ { @@ -457,11 +478,11 @@ ruleTester.run('valid-expect', rule, { }, // Promise.any([expect1, expect2]) returns one error { - code: `test("valid-expect", () => { + code: `test("valid-expect", () => { Promise.x([ - expect(Promise.resolve(2)).not.resolves.toBeDefined(), - expect(Promise.resolve(3)).not.resolves.toBeDefined(), - ]); + expect(Promise.resolve(2)).resolves.not.toBeDefined(), + expect(Promise.resolve(3)).resolves.not.toBeDefined(), + ]); });`, errors: [ { @@ -476,10 +497,10 @@ ruleTester.run('valid-expect', rule, { }, // { - code: `test("valid-expect", () => { + code: `test("valid-expect", () => { const assertions = [ - expect(Promise.resolve(2)).not.resolves.toBeDefined(), - expect(Promise.resolve(3)).not.resolves.toBeDefined(), + expect(Promise.resolve(2)).resolves.not.toBeDefined(), + expect(Promise.resolve(3)).resolves.not.toBeDefined(), ] });`, errors: [ @@ -505,6 +526,13 @@ ruleTester.run('valid-expect', rule, { { code: 'expect(Promise.resolve(2)).resolves.toBe;', errors: [ + { + line: 1, + column: 1, + endLine: 1, + endColumn: 42, + messageId: 'asyncMustBeAwaited', + }, { column: 37, endColumn: 41, @@ -514,7 +542,7 @@ ruleTester.run('valid-expect', rule, { ], }, { - code: `test("valid-expect", () => { + code: `test("valid-expect", () => { return expect(functionReturningAPromise()).resolves.toEqual(1).then(() => { expect(Promise.resolve(2)).resolves.toBe(1); }); @@ -531,7 +559,7 @@ ruleTester.run('valid-expect', rule, { ], }, { - code: `test("valid-expect", () => { + code: `test("valid-expect", () => { return expect(functionReturningAPromise()).resolves.toEqual(1).then(async () => { await expect(Promise.resolve(2)).resolves.toBe(1); expect(Promise.resolve(4)).resolves.toBe(4); diff --git a/src/rules/util.js b/src/rules/util.js index 60ff20a2a..d2d1c555d 100644 --- a/src/rules/util.js +++ b/src/rules/util.js @@ -22,21 +22,11 @@ export const expectResolvesCase = node => node.parent.parent.type === 'MemberExpression' && methodName(node) === 'resolves'; -export const expectNotResolvesCase = node => - expectNotCase(node) && - node.parent.parent.type === 'MemberExpression' && - methodName(node.parent) === 'resolves'; - export const expectRejectsCase = node => expectCase(node) && node.parent.parent.type === 'MemberExpression' && 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) || diff --git a/src/rules/valid-expect.js b/src/rules/valid-expect.js deleted file mode 100644 index 84e6f2ffa..000000000 --- a/src/rules/valid-expect.js +++ /dev/null @@ -1,314 +0,0 @@ -/* - * This implementation is ported from from eslint-plugin-jasmine. - * MIT license, Tom Vincent. - */ - -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 getParentIfThenified = node => { - const grandParentNode = node.parent && node.parent.parent; - - if ( - grandParentNode && - grandParentNode.type === 'CallExpression' && - grandParentNode.callee && - grandParentNode.callee.type === 'MemberExpression' && - ['then', 'catch'].includes(grandParentNode.callee.property.name) && - grandParentNode.parent - ) { - // Just in case `then`s are chained look one above. - return getParentIfThenified(grandParentNode); - } - - return node; -}; - -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: { - docs: { - url: getDocsUrl(__filename), - }, - messages: { - multipleArgs: 'More than one argument was passed to expect().', - noArgs: 'No arguments were passed to expect().', - noAssertions: 'No assertion was called on expect().', - invalidProperty: - '"{{ 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: [ - { - 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; - }; - - /** - * Promise method that accepts an array of promises, - * ( eg. Promise.all), will throw warnings for the each - * unawaited or non-returned promise. To avoid throwing - * multiple warnings, we check if there is a warning in - * the given location. - */ - const promiseArrayExceptionExists = loc => { - const key = promiseArrayExceptionKey(loc); - return !!arrayExceptions[key]; - }; - - return { - CallExpression(node) { - // checking "expect()" arguments - if (expectCase(node)) { - if (node.arguments.length > 1) { - const secondArgumentLocStart = node.arguments[1].loc.start; - const lastArgumentLocEnd = - node.arguments[node.arguments.length - 1].loc.end; - - context.report({ - loc: { - end: { - column: lastArgumentLocEnd.column - 1, - line: lastArgumentLocEnd.line, - }, - start: secondArgumentLocStart, - }, - messageId: 'multipleArgs', - node, - }); - } else if (node.arguments.length === 0) { - const expectLength = node.callee.name.length; - context.report({ - loc: { - end: { - column: node.loc.start.column + expectLength + 1, - line: node.loc.start.line, - }, - start: { - column: node.loc.start.column + expectLength, - line: node.loc.start.line, - }, - }, - messageId: 'noArgs', - node, - }); - } - - // something was called on `expect()` - if ( - node.parent && - node.parent.type === 'MemberExpression' && - node.parent.parent - ) { - let parentNode = node.parent; - let parentProperty = parentNode.property; - let propertyName = parentProperty.name; - let grandParent = parentNode.parent; - - // a property is accessed, get the next node - if (grandParent.type === 'MemberExpression') { - // a modifier is used, just get the next one - if (expectProperties.includes(propertyName)) { - grandParent = grandParent.parent; - } else { - // only a few properties are allowed - context.report({ - // For some reason `endColumn` isn't set in tests if `loc` is - // not added - loc: parentProperty.loc, - messageId: 'invalidProperty', - data: { propertyName }, - node: parentProperty, - }); - } - - // this next one should be the matcher - parentNode = parentNode.parent; - parentProperty = parentNode.property; - propertyName = parentProperty.name; - } - - // matcher was not called - if (grandParent.type === 'ExpressionStatement') { - context.report({ - // For some reason `endColumn` isn't set in tests if `loc` is not - // added - loc: parentProperty.loc, - data: { propertyName }, - messageId: expectProperties.includes(propertyName) - ? 'propertyWithoutMatcher' - : 'matcherOnPropertyNotCalled', - node: parentProperty, - }); - } - } - } - - if ( - expectResolvesCase(node) || - expectRejectsCase(node) || - expectNotResolvesCase(node) || - expectNotRejectsCase(node) - ) { - let parentNode = getClosestParentCallExpressionNode(node); - if (parentNode) { - /** - * If parent node is an array expression, we'll report the warning, - * for the array object, not for each individual assertion. - */ - const isParentArrayExpression = - parentNode.parent.type === 'ArrayExpression'; - - const { options } = context; - const allowReturn = !options[0] || !options[0].alwaysAwait; - const orReturned = allowReturn ? ' or returned' : ''; - let messageId = 'asyncMustBeAwaited'; - - /** - * An async assertion can be chained with `then` or `catch` statements. - * In that case our target CallExpression node is the one with - * the last `then` or `catch` statement. - */ - parentNode = getParentIfThenified(parentNode); - - // Promise.x([expect()]) || Promise.x(expect()) - if (promiseArgumentTypes.includes(parentNode.parent.type)) { - const promiseNode = getPromiseCallExpressionNode( - parentNode.parent, - ); - - if (promiseNode) { - parentNode = promiseNode; - messageId = 'promisesWithAsyncAssertionsMustBeAwaited'; - } - } - - if ( - // If node is not awaited or returned - !checkIfValidReturn(parentNode.parent, allowReturn) && - // if we didn't warn user already - !promiseArrayExceptionExists(parentNode.loc) - ) { - context.report({ - loc: parentNode.loc, - data: { - orReturned, - }, - messageId, - node, - }); - - if (isParentArrayExpression) { - pushPromiseArrayException(parentNode.loc); - } - } - } - } - }, - - // nothing called on "expect()" - 'CallExpression:exit'(node) { - if ( - node.callee.name === 'expect' && - node.parent.type === 'ExpressionStatement' - ) { - context.report({ - // For some reason `endColumn` isn't set in tests if `loc` is not - // added - loc: node.loc, - messageId: 'noAssertions', - node, - }); - } - }, - }; - }, -}; diff --git a/src/rules/valid-expect.ts b/src/rules/valid-expect.ts new file mode 100644 index 000000000..d602aaa89 --- /dev/null +++ b/src/rules/valid-expect.ts @@ -0,0 +1,292 @@ +/* + * This implementation is ported from from eslint-plugin-jasmine. + * MIT license, Tom Vincent. + */ + +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import { + ModifierName, + createRule, + getAccessorValue, + isExpectCall, + isExpectMember, + isSupportedAccessor, + parseExpectCall, +} from './tsUtils'; + +/** + * 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: TSESTree.Node) => { + if ( + node && + node.type === AST_NODE_TYPES.ArrayExpression && + node.parent && + node.parent.type === AST_NODE_TYPES.CallExpression + ) { + node = node.parent; + } + + if ( + node.type === AST_NODE_TYPES.CallExpression && + node.callee && + node.callee.type === AST_NODE_TYPES.MemberExpression && + isSupportedAccessor(node.callee.object) && + getAccessorValue(node.callee.object) === 'Promise' && + node.parent + ) { + return node; + } + + return null; +}; + +const findPromiseCallExpressionNode = (node: TSESTree.Node) => + node.parent && + node.parent.parent && + [AST_NODE_TYPES.CallExpression, AST_NODE_TYPES.ArrayExpression].includes( + node.parent.type, + ) + ? getPromiseCallExpressionNode(node.parent) + : null; + +const getParentIfThenified = (node: TSESTree.Node): TSESTree.Node => { + const grandParentNode = node.parent && node.parent.parent; + + if ( + grandParentNode && + grandParentNode.type === AST_NODE_TYPES.CallExpression && + grandParentNode.callee && + isExpectMember(grandParentNode.callee) && + ['then', 'catch'].includes( + getAccessorValue(grandParentNode.callee.property), + ) && + grandParentNode.parent + ) { + // Just in case `then`s are chained look one above. + return getParentIfThenified(grandParentNode); + } + + return node; +}; + +const isAcceptableReturnNode = ( + node: TSESTree.Node, + allowReturn: boolean, +): node is + | TSESTree.ArrowFunctionExpression + | TSESTree.AwaitExpression + | TSESTree.ReturnStatement => + (allowReturn && node.type === AST_NODE_TYPES.ReturnStatement) || + [ + AST_NODE_TYPES.ArrowFunctionExpression, + AST_NODE_TYPES.AwaitExpression, + ].includes(node.type); + +const promiseArrayExceptionKey = ({ start, end }: TSESTree.SourceLocation) => + `${start.line}:${start.column}-${end.line}:${end.column}`; + +type MessageIds = + | 'multipleArgs' + | 'noArgs' + | 'noAssertions' + | 'invalidProperty' + | 'propertyWithoutMatcher' + | 'matcherOnPropertyNotCalled' + | 'asyncMustBeAwaited' + | 'promisesWithAsyncAssertionsMustBeAwaited'; + +export default createRule<[{ alwaysAwait?: boolean }], MessageIds>({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Enforce valid `expect()` usage', + recommended: 'error', + }, + messages: { + multipleArgs: 'More than one argument was passed to expect().', + noArgs: 'No arguments were passed to expect().', + noAssertions: 'No assertion was called on expect().', + invalidProperty: + '"{{ 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 }}.', + }, + type: 'suggestion', + schema: [ + { + type: 'object', + properties: { + alwaysAwait: { + type: 'boolean', + default: false, + }, + }, + additionalProperties: false, + }, + ], + }, + defaultOptions: [{ alwaysAwait: false }], + create(context, [{ alwaysAwait }]) { + // Context state + const arrayExceptions = new Set(); + + const pushPromiseArrayException = (loc: TSESTree.SourceLocation) => + arrayExceptions.add(promiseArrayExceptionKey(loc)); + + /** + * Promise method that accepts an array of promises, + * (eg. Promise.all), will throw warnings for the each + * unawaited or non-returned promise. To avoid throwing + * multiple warnings, we check if there is a warning in + * the given location. + */ + const promiseArrayExceptionExists = (loc: TSESTree.SourceLocation) => + arrayExceptions.has(promiseArrayExceptionKey(loc)); + + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { expect, modifier, matcher } = parseExpectCall(node); + + if (expect.arguments.length > 1) { + const secondArgumentLocStart = expect.arguments[1].loc.start; + const lastArgumentLocEnd = + expect.arguments[node.arguments.length - 1].loc.end; + + context.report({ + loc: { + end: { + column: lastArgumentLocEnd.column - 1, + line: lastArgumentLocEnd.line, + }, + start: secondArgumentLocStart, + }, + messageId: 'multipleArgs', + node, + }); + } else if (expect.arguments.length === 0) { + const expectLength = getAccessorValue(expect.callee).length; + context.report({ + loc: { + end: { + column: node.loc.start.column + expectLength + 1, + line: node.loc.start.line, + }, + start: { + column: node.loc.start.column + expectLength, + line: node.loc.start.line, + }, + }, + messageId: 'noArgs', + node, + }); + } + + // something was called on `expect()` + if (!matcher) { + if (modifier) { + context.report({ + data: { propertyName: modifier.name }, // todo: rename to 'modifierName' + messageId: 'propertyWithoutMatcher', // todo: rename to 'modifierWithoutMatcher' + node: modifier.node.property, + }); + } + + return; + } + + if (matcher.node.parent && isExpectMember(matcher.node.parent)) { + context.report({ + messageId: 'invalidProperty', // todo: rename to 'invalidModifier' + data: { propertyName: matcher.name }, // todo: rename to 'matcherName' (or modifierName?) + node: matcher.node.property, + }); + + return; + } + + if (!matcher.arguments) { + context.report({ + data: { propertyName: matcher.name }, // todo: rename to 'matcherName' + messageId: 'matcherOnPropertyNotCalled', // todo: rename to 'matcherNotCalled' + node: matcher.node.property, + }); + } + + const parentNode = matcher.node.parent; + + if ( + !modifier || + !parentNode || + !parentNode.parent || + modifier.name === ModifierName.not + ) { + return; + } + /** + * If parent node is an array expression, we'll report the warning, + * for the array object, not for each individual assertion. + */ + const isParentArrayExpression = + parentNode.parent.type === AST_NODE_TYPES.ArrayExpression; + const orReturned = alwaysAwait ? '' : ' or returned'; + /** + * An async assertion can be chained with `then` or `catch` statements. + * In that case our target CallExpression node is the one with + * the last `then` or `catch` statement. + */ + const targetNode = getParentIfThenified(parentNode); + const finalNode = + findPromiseCallExpressionNode(targetNode) || targetNode; + if ( + finalNode.parent && + // If node is not awaited or returned + !isAcceptableReturnNode(finalNode.parent, !alwaysAwait) && + // if we didn't warn user already + !promiseArrayExceptionExists(finalNode.loc) + ) { + context.report({ + loc: finalNode.loc, + data: { + orReturned, + }, + messageId: + finalNode === targetNode + ? 'asyncMustBeAwaited' + : 'promisesWithAsyncAssertionsMustBeAwaited', + node, + }); + + if (isParentArrayExpression) { + pushPromiseArrayException(finalNode.loc); + } + } + }, + + // nothing called on "expect()" + 'CallExpression:exit'(node: TSESTree.CallExpression) { + if ( + isExpectCall(node) && + node.parent.type === AST_NODE_TYPES.ExpressionStatement + ) { + context.report({ messageId: 'noAssertions', node }); + } + }, + }; + }, +});