diff --git a/src/rules/prefer-web-first-assertions.test.ts b/src/rules/prefer-web-first-assertions.test.ts index 0c238fc..871d398 100644 --- a/src/rules/prefer-web-first-assertions.test.ts +++ b/src/rules/prefer-web-first-assertions.test.ts @@ -33,6 +33,90 @@ runRuleTester('prefer-web-first-assertions', rule, { ], output: test('await expect(page.locator(".tweet")).toBeHidden()'), }, + { + code: test(` + const unrelatedAssignment = 'unrelated' + const isTweetVisible = await page.locator(".tweet").isVisible() + expect(isTweetVisible).toBe(true) + `), + output: test(` + const unrelatedAssignment = 'unrelated' + const isTweetVisible = page.locator(".tweet") + await expect(isTweetVisible).toBeVisible() + `), + errors: [ + { + column: 9, + data: { matcher: 'toBeVisible', method: 'isVisible' }, + endColumn: 31, + line: 4, + messageId: 'useWebFirstAssertion', + }, + ], + }, + { + code: test(` + const unrelatedAssignment = 'unrelated' + const isTweetVisible = await page.locator(".tweet").isVisible() + expect(isTweetVisible).toBe(false) + `), + output: test(` + const unrelatedAssignment = 'unrelated' + const isTweetVisible = page.locator(".tweet") + await expect(isTweetVisible).toBeHidden() + `), + errors: [ + { + column: 9, + data: { matcher: 'toBeHidden', method: 'isVisible' }, + endColumn: 31, + line: 4, + messageId: 'useWebFirstAssertion', + }, + ], + }, + { + code: test(` + const locatorFoo = page.locator(".foo") + const isBarVisible = await locatorFoo.locator(".bar").isVisible() + expect(isBarVisible).toBe(false) + `), + output: test(` + const locatorFoo = page.locator(".foo") + const isBarVisible = locatorFoo.locator(".bar") + await expect(isBarVisible).toBeHidden() + `), + errors: [ + { + column: 9, + data: { matcher: 'toBeHidden', method: 'isVisible' }, + endColumn: 29, + line: 4, + messageId: 'useWebFirstAssertion', + }, + ], + }, + { + code: test(` + const locatorFoo = page.locator(".foo") + const isBarVisible = await locatorFoo.locator(".bar").isVisible() + expect(isBarVisible).toBe(true) + `), + output: test(` + const locatorFoo = page.locator(".foo") + const isBarVisible = locatorFoo.locator(".bar") + await expect(isBarVisible).toBeVisible() + `), + errors: [ + { + column: 9, + data: { matcher: 'toBeVisible', method: 'isVisible' }, + endColumn: 29, + line: 4, + messageId: 'useWebFirstAssertion', + }, + ], + }, { code: test( 'expect(await page.locator(".tweet").isVisible()).toEqual(true)', @@ -301,6 +385,150 @@ runRuleTester('prefer-web-first-assertions', rule, { ], output: test('await expect(foo).not.toHaveText("bar")'), }, + { + code: test(` + const fooLocator = page.locator('.fooClass'); + const fooLocatorText = await fooLocator.textContent(); + expect(fooLocatorText).toEqual('foo'); + `), + output: test(` + const fooLocator = page.locator('.fooClass'); + const fooLocatorText = fooLocator; + await expect(fooLocatorText).toHaveText('foo'); + `), + errors: [ + { + column: 9, + data: { matcher: 'toHaveText', method: 'textContent' }, + endColumn: 31, + line: 4, + messageId: 'useWebFirstAssertion', + }, + ], + }, + { + code: test(` + const fooLocator = page.locator('.fooClass'); + let fooLocatorText = await fooLocator.textContent(); + expect(fooLocatorText).toEqual('foo'); + fooLocatorText = 'foo'; + expect(fooLocatorText).toEqual('foo'); + `), + output: test(` + const fooLocator = page.locator('.fooClass'); + let fooLocatorText = fooLocator; + await expect(fooLocatorText).toHaveText('foo'); + fooLocatorText = 'foo'; + expect(fooLocatorText).toEqual('foo'); + `), + errors: [ + { + column: 9, + data: { matcher: 'toHaveText', method: 'textContent' }, + endColumn: 31, + line: 4, + messageId: 'useWebFirstAssertion', + }, + ], + }, + { + code: test(` + let fooLocatorText; + const fooLocator = page.locator('.fooClass'); + fooLocatorText = 'Unrelated'; + fooLocatorText = await fooLocator.textContent(); + expect(fooLocatorText).toEqual('foo'); + `), + output: test(` + let fooLocatorText; + const fooLocator = page.locator('.fooClass'); + fooLocatorText = 'Unrelated'; + fooLocatorText = fooLocator; + await expect(fooLocatorText).toHaveText('foo'); + `), + errors: [ + { + column: 9, + data: { matcher: 'toHaveText', method: 'textContent' }, + endColumn: 31, + line: 6, + messageId: 'useWebFirstAssertion', + }, + ], + }, + { + code: test(` + let fooLocatorText; + let fooLocatorText2; + const fooLocator = page.locator('.fooClass'); + fooLocatorText = await fooLocator.textContent(); + fooLocatorText2 = await fooLocator.textContent(); + expect(fooLocatorText).toEqual('foo'); + `), + output: test(` + let fooLocatorText; + let fooLocatorText2; + const fooLocator = page.locator('.fooClass'); + fooLocatorText = fooLocator; + fooLocatorText2 = await fooLocator.textContent(); + await expect(fooLocatorText).toHaveText('foo'); + `), + errors: [ + { + column: 9, + data: { matcher: 'toHaveText', method: 'textContent' }, + endColumn: 31, + line: 7, + messageId: 'useWebFirstAssertion', + }, + ], + }, + { + code: test(` + let fooLocatorText; + fooLocatorText = 'foo'; + expect(fooLocatorText).toEqual('foo'); + fooLocatorText = await page.locator('.fooClass').textContent(); + expect(fooLocatorText).toEqual('foo'); + `), + output: test(` + let fooLocatorText; + fooLocatorText = 'foo'; + expect(fooLocatorText).toEqual('foo'); + fooLocatorText = page.locator('.fooClass'); + await expect(fooLocatorText).toHaveText('foo'); + `), + errors: [ + { + column: 9, + data: { matcher: 'toHaveText', method: 'textContent' }, + endColumn: 31, + line: 6, + messageId: 'useWebFirstAssertion', + }, + ], + }, + { + code: test(` + const unrelatedAssignment = "unrelated"; + const fooLocatorText = await page.locator('.foo').textContent(); + expect(fooLocatorText).toEqual('foo'); + `), + output: test(` + const unrelatedAssignment = "unrelated"; + const fooLocatorText = page.locator('.foo'); + await expect(fooLocatorText).toHaveText('foo'); + `), + errors: [ + { + column: 9, + data: { matcher: 'toHaveText', method: 'textContent' }, + endColumn: 31, + line: 4, + messageId: 'useWebFirstAssertion', + }, + ], + }, // isChecked { @@ -736,5 +964,44 @@ runRuleTester('prefer-web-first-assertions', rule, { expect(myValue).toBeVisible(); `), }, + { + code: test(` + let fooLocatorText; + const fooLocator = page.locator('.fooClass'); + fooLocatorText = await fooLocator.textContent(); + fooLocatorText = 'foo'; + expect(fooLocatorText).toEqual('foo'); + `), + }, + { + code: test(` + let fooLocatorText; + let fooLocatorText2; + const fooLocator = page.locator('.fooClass'); + fooLocatorText = 'foo'; + fooLocatorText2 = await fooLocator.textContent(); + expect(fooLocatorText).toEqual('foo'); + `), + }, + { + code: test(` + let fooLocatorText; + fooLocatorText = 'foo'; + expect(fooLocatorText).toEqual('foo') + const fooLocator = page.locator('.fooClass'); + fooLocatorText = fooLocator; + expect(fooLocatorText).toHaveText('foo'); + `), + }, + { + code: test(` + const fooLocator = page.locator('.fooClass'); + let fooLocatorText; + fooLocatorText = fooLocator; + expect(fooLocatorText).toHaveText('foo'); + fooLocatorText = 'foo'; + expect(fooLocatorText).toEqual('foo') + `), + }, ], }) diff --git a/src/rules/prefer-web-first-assertions.ts b/src/rules/prefer-web-first-assertions.ts index 537c85c..2d7adc6 100644 --- a/src/rules/prefer-web-first-assertions.ts +++ b/src/rules/prefer-web-first-assertions.ts @@ -1,5 +1,5 @@ import { Rule } from 'eslint' -import ESTree from 'estree' +import ESTree, { AssignmentExpression } from 'estree' import { findParent, getRawValue, @@ -8,6 +8,7 @@ import { } from '../utils/ast' import { createRule } from '../utils/createRule' import { parseFnCall } from '../utils/parseFnCall' +import { TypedNodeWithParent } from '../utils/types' type MethodConfig = { inverse?: string @@ -59,9 +60,50 @@ const supportedMatchers = new Set([ 'toBeFalsy', ]) +const isVariableDeclarator = ( + node: ESTree.Node, +): node is TypedNodeWithParent<'VariableDeclarator'> => + node.type === 'VariableDeclarator' + +const isAssignmentExpression = ( + node: ESTree.Node, +): node is TypedNodeWithParent<'AssignmentExpression'> => + node.type === 'AssignmentExpression' + +/** + * Given a Node and an assignment expression, finds out if the assignment + * expression happens before the node identifier (based on their range + * properties) and if the assignment expression left side is of the same name as + * the name of the given node. + * + * @param node The node we are comparing the assignment expression to. + * @param assignment The assignment that will be verified to see if its left + * operand is the same as the node.name and if it happens before it. + * @returns True if the assignment left hand operator belongs to the node and + * occurs before it, false otherwise. If either the node or the assignment + * expression doesn't contain a range array, this will also return false + * because their relative positions cannot be calculated. + */ +function isNodeLastAssignment( + node: ESTree.Identifier, + assignment: AssignmentExpression, +) { + if (node.range && assignment.range && node.range[0] < assignment.range[1]) { + return false + } + + return ( + assignment.left.type === 'Identifier' && assignment.left.name === node.name + ) +} + /** * If the expect call argument is a variable reference, finds the variable - * initializer. + * initializer or last variable assignment. + * + * If a variable is assigned after initialization we have to look for the last + * time it was assigned because it could have been changed multiple times. We + * then use its right hand assignment operator as the dereferenced node. */ function dereference(context: Rule.RuleContext, node: ESTree.Node | undefined) { if (node?.type !== 'Identifier') { @@ -69,15 +111,24 @@ function dereference(context: Rule.RuleContext, node: ESTree.Node | undefined) { } const scope = context.sourceCode.getScope(node) + const parents = scope.references + .map((ref) => ref.identifier as Rule.Node) + .map((ident) => ident.parent) - // Find the variable declaration and return the initializer - for (const ref of scope.references) { - const refParent = (ref.identifier as Rule.Node).parent + // Look for any variable declarators in the scope references that match the + // dereferenced node variable name + const decl = parents + .filter(isVariableDeclarator) + .find((p) => p.id.type === 'Identifier' && p.id.name === node.name) - if (refParent.type === 'VariableDeclarator') { - return refParent.init - } - } + // Look for any variable assignments in the scope references and pick the last + // one that matches the dereferenced node variable name + const expr = parents + .filter(isAssignmentExpression) + .reverse() + .find((assignment) => isNodeLastAssignment(node, assignment)) + + return expr?.right ?? decl?.init } export default createRule({