fix(prefer-web-first-assertions): Support variable reassignment (#287)

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')
+      `),
+    },
   ],
 })

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,25 +60,75 @@ 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') {
     return node
   }
 
   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({