diff --git a/src/rules/no-conditional-expect.test.ts b/src/rules/no-conditional-expect.test.ts index b1bcb23..e5c684a 100644 --- a/src/rules/no-conditional-expect.test.ts +++ b/src/rules/no-conditional-expect.test.ts @@ -1,22 +1,36 @@ -import dedent from 'dedent' import rule from '../../src/rules/no-conditional-expect' -import { runRuleTester } from '../utils/rule-tester' +import { javascript, runRuleTester } from '../utils/rule-tester' const messageId = 'conditionalExpect' runRuleTester('common tests', rule, { invalid: [], valid: [ - ` + javascript` test('foo', () => { expect(1).toBe(2); }); `, - ` + javascript` test('foo', () => { expect(!true).toBe(false); }); `, + { + code: javascript` + test('foo', () => { + const expected = arr.map((x) => { + if (typeof x === 'string') { + return expect.arrayContaining([x]) + } else { + return b + } + }) + + expect([1, 2, 3]).toEqual(expected); + }); + `, + }, ], }) @@ -154,7 +168,7 @@ runRuleTester('logical conditions', rule, { `, // Global aliases { - code: dedent` + code: javascript` it('foo', () => { process.env.FAIL && setNum(1); @@ -331,7 +345,6 @@ runRuleTester('catch conditions', rule, { code: ` test('foo', () => { try { - } catch (err) { expect(err).toMatch('Error'); } @@ -397,7 +410,7 @@ runRuleTester('catch conditions', rule, { runRuleTester('promises', rule, { invalid: [ { - code: dedent` + code: javascript` test('works', async () => { await Promise.resolve() .then(() => { throw new Error('oh noes!'); }) @@ -407,7 +420,7 @@ runRuleTester('promises', rule, { errors: [{ messageId }], }, { - code: dedent` + code: javascript` test('works', async () => { await Promise.resolve() .then(() => { throw new Error('oh noes!'); }) @@ -421,7 +434,7 @@ runRuleTester('promises', rule, { errors: [{ messageId }], }, { - code: dedent` + code: javascript` test('works', async () => { await Promise.resolve() .catch(error => expect(error).toBeInstanceOf(Error)) @@ -432,7 +445,7 @@ runRuleTester('promises', rule, { errors: [{ messageId }], }, { - code: dedent` + code: javascript` test('works', async () => { await Promise.resolve() .catch(error => expect(error).toBeInstanceOf(Error)) @@ -444,7 +457,7 @@ runRuleTester('promises', rule, { errors: [{ messageId }], }, { - code: dedent` + code: javascript` test('works', async () => { await somePromise .then(() => { throw new Error('oh noes!'); }) @@ -454,7 +467,7 @@ runRuleTester('promises', rule, { errors: [{ messageId }], }, { - code: dedent` + code: javascript` test('works', async () => { await somePromise.catch(error => expect(error).toBeInstanceOf(Error)); }); diff --git a/src/rules/valid-expect.test.ts b/src/rules/valid-expect.test.ts index 7d6ef31..af5862f 100644 --- a/src/rules/valid-expect.test.ts +++ b/src/rules/valid-expect.test.ts @@ -215,6 +215,11 @@ runRuleTester('valid-expect', rule, { valid: [ { code: 'expectPayButtonToBeEnabled()' }, { code: 'expect("something").toBe("else")' }, + { code: 'expect.anything()' }, + { code: 'expect.arrayContaining()' }, + { code: 'expect.not.arrayContaining()' }, + { code: 'expect.objectContaining(expected)' }, + { code: 'expect.not.objectContaining(expected)' }, { code: 'expect("something").toBe(expect.anything())' }, { code: 'expect("something").toEqual({ foo: expect.anything() })' }, { code: 'expect("something").toBe(expect.arrayContaining([1, 2, 3]))' }, diff --git a/src/utils/parseFnCall.test.ts b/src/utils/parseFnCall.test.ts index edb1699..2c9998a 100644 --- a/src/utils/parseFnCall.test.ts +++ b/src/utils/parseFnCall.test.ts @@ -260,36 +260,6 @@ runRuleTester('expect', rule, { code: dedent` import { expect } from '@playwright/test'; - expect.assertions(); - `, - errors: [ - { - column: 1, - data: expectedParsedFnCallResultData({ - args: [], - group: 'expect', - head: { - local: 'expect', - node: 'expect', - original: null, - }, - matcher: 'assertions', - matcherArgs: [], - matcherName: 'assertions', - members: ['assertions'], - modifiers: [], - name: 'expect', - type: 'expect', - }), - line: 3, - messageId: 'details', - }, - ], - }, - { - code: dedent` - import { expect } from '@playwright/test'; - expect(x).toBe(y); `, errors: [ @@ -380,58 +350,6 @@ runRuleTester('expect', rule, { code: dedent` import { expect } from '@playwright/test'; - expect.anything(); - expect.not.arrayContaining(); - `, - errors: [ - { - column: 1, - data: expectedParsedFnCallResultData({ - args: [], - group: 'expect', - head: { - local: 'expect', - node: 'expect', - original: null, - }, - matcher: 'anything', - matcherArgs: [], - matcherName: 'anything', - members: ['anything'], - modifiers: [], - name: 'expect', - type: 'expect', - }), - line: 3, - messageId: 'details', - }, - { - column: 1, - data: expectedParsedFnCallResultData({ - args: [], - group: 'expect', - head: { - local: 'expect', - node: 'expect', - original: null, - }, - matcher: 'arrayContaining', - matcherArgs: [], - matcherName: 'arrayContaining', - members: ['not', 'arrayContaining'], - modifiers: ['not'], - name: 'expect', - type: 'expect', - }), - line: 4, - messageId: 'details', - }, - ], - }, - { - code: dedent` - import { expect } from '@playwright/test'; - expect; expect(x); expect(x).toBe; @@ -1187,5 +1105,18 @@ runTSRuleTester('typescript', rule, { }, "it('is not a function', () => {});", 'dedent()', + 'expect.assertions()', + 'expect.anything()', + 'expect.arrayContaining()', + 'expect.objectContaining(expected)', + 'expect.not.objectContaining(expected)', + { + code: dedent` + import { expect } from '@playwright/test'; + + expect.assertions(); + expect.anything(); + `, + }, ], }) diff --git a/src/utils/parseFnCall.ts b/src/utils/parseFnCall.ts index 9a0dbf9..dd9b385 100644 --- a/src/utils/parseFnCall.ts +++ b/src/utils/parseFnCall.ts @@ -94,21 +94,51 @@ export const isSupportedAccessor = ( ): node is AccessorNode => isIdentifier(node, value) || isStringNode(node, value) -function getNodeChain(node: ESTree.Node): AccessorNode[] | null { - if (isSupportedAccessor(node)) { - return [node] +class Chain { + #nodes: AccessorNode[] | null = null + #leaves: WeakSet = new WeakSet() + + constructor(node: ESTree.Node) { + this.#nodes = this.#buildChain(node) + } + + isLeaf(node: AccessorNode): boolean { + return this.#leaves.has(node) } - switch (node.type) { - case 'TaggedTemplateExpression': - return getNodeChain(node.tag) - case 'MemberExpression': - return joinChains(getNodeChain(node.object), getNodeChain(node.property)) - case 'CallExpression': - return getNodeChain(node.callee) + get nodes() { + return this.#nodes } - return null + #buildChain(node: ESTree.Node, insideCall = false): AccessorNode[] | null { + if (isSupportedAccessor(node)) { + // If we are inside a call expression, then the current node is a leaf, + // that is, the end of the sub-chain. For example, in + // `expect.soft(x).not.toBe()`, `soft` and `toBe` are leaves. + if (insideCall) { + this.#leaves.add(node) + } + + return [node] + } + + switch (node.type) { + case 'TaggedTemplateExpression': + return this.#buildChain(node.tag) + + case 'MemberExpression': + return joinChains( + this.#buildChain(node.object), + this.#buildChain(node.property, insideCall), + ) + + case 'CallExpression': + return this.#buildChain(node.callee, true) + + default: + return null + } + } } const resolvePossibleAliasedGlobal = ( @@ -166,12 +196,13 @@ function determinePlaywrightFnGroup(name: string): FnGroup { export const modifiers = new Set(['not', 'resolves', 'rejects']) const findModifiersAndMatcher = ( + chain: Chain, members: KnownMemberExpressionProperty[], -): ModifiersAndMatcher | string => { + stage: ExpectParseStage, +): ModifiersAndMatcher | string | null => { const modifiers: KnownMemberExpressionProperty[] = [] for (const member of members) { - // Otherwise, it should be a modifier const name = getStringValue(member) if (name === 'soft' || name === 'poll') { @@ -187,6 +218,12 @@ const findModifiersAndMatcher = ( return 'modifier-unknown' } } else if (name !== 'not') { + // If we're in the "modifiers" stage and we find an unknown modifier, + // then it's actually an asymmetric matcher which we don't care about. + if (stage === 'modifiers') { + return null + } + // Check if the member is being called, which means it is the matcher // (and also the end of the entire "expect" call chain). if ( @@ -205,6 +242,13 @@ const findModifiersAndMatcher = ( return 'modifier-unknown' } + // When we find a leaf node, we're done with the modifiers and are moving + // on to the matchers. + if (chain.isLeaf(member)) { + stage = 'matchers' + } + + // Add the modifier to the list of modifiers modifiers.push(member) } @@ -263,10 +307,22 @@ export interface ParsedExpectFnCall export type ParsedFnCall = ParsedGeneralFnCall | ParsedExpectFnCall +type ExpectParseStage = 'matchers' | 'modifiers' + const parseExpectCall = ( + chain: Chain, call: Omit, -): ParsedExpectFnCall | string => { - const modifiersAndMatcher = findModifiersAndMatcher(call.members) + stage: ExpectParseStage, +): ParsedExpectFnCall | string | null => { + const modifiersAndMatcher = findModifiersAndMatcher( + chain, + call.members, + stage, + ) + + if (!modifiersAndMatcher) { + return null + } if (typeof modifiersAndMatcher === 'string') { return modifiersAndMatcher @@ -316,13 +372,10 @@ function parse( context: Rule.RuleContext, node: ESTree.CallExpression, ): ParsedFnCall | string | null { - const chain = getNodeChain(node) - - if (!chain?.length) { - return null - } + const chain = new Chain(node) + if (!chain.nodes?.length) return null - const [first, ...rest] = chain + const [first, ...rest] = chain.nodes const resolved = resolveToPlaywrightFn(context, first) if (!resolved) return null @@ -355,14 +408,20 @@ function parse( const group = determinePlaywrightFnGroup(name) if (group === 'expect') { + let stage: ExpectParseStage = chain.isLeaf(parsedFnCall.head.node) + ? 'matchers' + : 'modifiers' + // If using `test.expect` style, the `rest` array will start with `expect` // and we need to remove it to ensure the chain accurately represents the // `expect` call chain. if (isIdentifier(rest[0], 'expect')) { + stage = chain.isLeaf(rest[0]) ? 'matchers' : 'modifiers' parsedFnCall.members.shift() } - const result = parseExpectCall(parsedFnCall) + const result = parseExpectCall(chain, parsedFnCall, stage) + if (!result) return null // If the `expect` call chain is not valid, only report on the topmost node // since all members in the chain are likely to get flagged for some reason @@ -384,8 +443,8 @@ function parse( // Check that every link in the chain except the last is a member expression if ( - chain - .slice(0, chain.length - 1) + chain.nodes + .slice(0, chain.nodes.length - 1) .some((n) => getParent(n)?.type !== 'MemberExpression') ) { return null diff --git a/src/utils/rule-tester.ts b/src/utils/rule-tester.ts index e66476a..4a40988 100644 --- a/src/utils/rule-tester.ts +++ b/src/utils/rule-tester.ts @@ -1,3 +1,4 @@ +import dedent from 'dedent' import { RuleTester } from 'eslint' import { describe, it } from 'vitest' @@ -38,3 +39,5 @@ export function runTSRuleTester(...args: Parameters) { } export const test = (input: string) => `test('test', async () => { ${input} })` + +export const javascript = dedent