Skip to content

Commit b0fb7cd

Browse files
An631An631
authoredApr 20, 2024··
fix(prefer-web-first-assertions): Support variable reassignment (#287)
·
v2.2.0v1.6.1
1 parent 1fe2c5e commit b0fb7cd

File tree

2 files changed

+327
-9
lines changed

2 files changed

+327
-9
lines changed
 

‎src/rules/prefer-web-first-assertions.test.ts

+267
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,90 @@ runRuleTester('prefer-web-first-assertions', rule, {
3333
],
3434
output: test('await expect(page.locator(".tweet")).toBeHidden()'),
3535
},
36+
{
37+
code: test(`
38+
const unrelatedAssignment = 'unrelated'
39+
const isTweetVisible = await page.locator(".tweet").isVisible()
40+
expect(isTweetVisible).toBe(true)
41+
`),
42+
output: test(`
43+
const unrelatedAssignment = 'unrelated'
44+
const isTweetVisible = page.locator(".tweet")
45+
await expect(isTweetVisible).toBeVisible()
46+
`),
47+
errors: [
48+
{
49+
column: 9,
50+
data: { matcher: 'toBeVisible', method: 'isVisible' },
51+
endColumn: 31,
52+
line: 4,
53+
messageId: 'useWebFirstAssertion',
54+
},
55+
],
56+
},
57+
{
58+
code: test(`
59+
const unrelatedAssignment = 'unrelated'
60+
const isTweetVisible = await page.locator(".tweet").isVisible()
61+
expect(isTweetVisible).toBe(false)
62+
`),
63+
output: test(`
64+
const unrelatedAssignment = 'unrelated'
65+
const isTweetVisible = page.locator(".tweet")
66+
await expect(isTweetVisible).toBeHidden()
67+
`),
68+
errors: [
69+
{
70+
column: 9,
71+
data: { matcher: 'toBeHidden', method: 'isVisible' },
72+
endColumn: 31,
73+
line: 4,
74+
messageId: 'useWebFirstAssertion',
75+
},
76+
],
77+
},
78+
{
79+
code: test(`
80+
const locatorFoo = page.locator(".foo")
81+
const isBarVisible = await locatorFoo.locator(".bar").isVisible()
82+
expect(isBarVisible).toBe(false)
83+
`),
84+
output: test(`
85+
const locatorFoo = page.locator(".foo")
86+
const isBarVisible = locatorFoo.locator(".bar")
87+
await expect(isBarVisible).toBeHidden()
88+
`),
89+
errors: [
90+
{
91+
column: 9,
92+
data: { matcher: 'toBeHidden', method: 'isVisible' },
93+
endColumn: 29,
94+
line: 4,
95+
messageId: 'useWebFirstAssertion',
96+
},
97+
],
98+
},
99+
{
100+
code: test(`
101+
const locatorFoo = page.locator(".foo")
102+
const isBarVisible = await locatorFoo.locator(".bar").isVisible()
103+
expect(isBarVisible).toBe(true)
104+
`),
105+
output: test(`
106+
const locatorFoo = page.locator(".foo")
107+
const isBarVisible = locatorFoo.locator(".bar")
108+
await expect(isBarVisible).toBeVisible()
109+
`),
110+
errors: [
111+
{
112+
column: 9,
113+
data: { matcher: 'toBeVisible', method: 'isVisible' },
114+
endColumn: 29,
115+
line: 4,
116+
messageId: 'useWebFirstAssertion',
117+
},
118+
],
119+
},
36120
{
37121
code: test(
38122
'expect(await page.locator(".tweet").isVisible()).toEqual(true)',
@@ -301,6 +385,150 @@ runRuleTester('prefer-web-first-assertions', rule, {
301385
],
302386
output: test('await expect(foo).not.toHaveText("bar")'),
303387
},
388+
{
389+
code: test(`
390+
const fooLocator = page.locator('.fooClass');
391+
const fooLocatorText = await fooLocator.textContent();
392+
expect(fooLocatorText).toEqual('foo');
393+
`),
394+
output: test(`
395+
const fooLocator = page.locator('.fooClass');
396+
const fooLocatorText = fooLocator;
397+
await expect(fooLocatorText).toHaveText('foo');
398+
`),
399+
errors: [
400+
{
401+
column: 9,
402+
data: { matcher: 'toHaveText', method: 'textContent' },
403+
endColumn: 31,
404+
line: 4,
405+
messageId: 'useWebFirstAssertion',
406+
},
407+
],
408+
},
409+
{
410+
code: test(`
411+
const fooLocator = page.locator('.fooClass');
412+
let fooLocatorText = await fooLocator.textContent();
413+
expect(fooLocatorText).toEqual('foo');
414+
fooLocatorText = 'foo';
415+
expect(fooLocatorText).toEqual('foo');
416+
`),
417+
output: test(`
418+
const fooLocator = page.locator('.fooClass');
419+
let fooLocatorText = fooLocator;
420+
await expect(fooLocatorText).toHaveText('foo');
421+
fooLocatorText = 'foo';
422+
expect(fooLocatorText).toEqual('foo');
423+
`),
424+
errors: [
425+
{
426+
column: 9,
427+
data: { matcher: 'toHaveText', method: 'textContent' },
428+
endColumn: 31,
429+
line: 4,
430+
messageId: 'useWebFirstAssertion',
431+
},
432+
],
433+
},
434+
{
435+
code: test(`
436+
let fooLocatorText;
437+
const fooLocator = page.locator('.fooClass');
438+
fooLocatorText = 'Unrelated';
439+
fooLocatorText = await fooLocator.textContent();
440+
expect(fooLocatorText).toEqual('foo');
441+
`),
442+
output: test(`
443+
let fooLocatorText;
444+
const fooLocator = page.locator('.fooClass');
445+
fooLocatorText = 'Unrelated';
446+
fooLocatorText = fooLocator;
447+
await expect(fooLocatorText).toHaveText('foo');
448+
`),
449+
errors: [
450+
{
451+
column: 9,
452+
data: { matcher: 'toHaveText', method: 'textContent' },
453+
endColumn: 31,
454+
line: 6,
455+
messageId: 'useWebFirstAssertion',
456+
},
457+
],
458+
},
459+
{
460+
code: test(`
461+
let fooLocatorText;
462+
let fooLocatorText2;
463+
const fooLocator = page.locator('.fooClass');
464+
fooLocatorText = await fooLocator.textContent();
465+
fooLocatorText2 = await fooLocator.textContent();
466+
expect(fooLocatorText).toEqual('foo');
467+
`),
468+
output: test(`
469+
let fooLocatorText;
470+
let fooLocatorText2;
471+
const fooLocator = page.locator('.fooClass');
472+
fooLocatorText = fooLocator;
473+
fooLocatorText2 = await fooLocator.textContent();
474+
await expect(fooLocatorText).toHaveText('foo');
475+
`),
476+
errors: [
477+
{
478+
column: 9,
479+
data: { matcher: 'toHaveText', method: 'textContent' },
480+
endColumn: 31,
481+
line: 7,
482+
messageId: 'useWebFirstAssertion',
483+
},
484+
],
485+
},
486+
{
487+
code: test(`
488+
let fooLocatorText;
489+
fooLocatorText = 'foo';
490+
expect(fooLocatorText).toEqual('foo');
491+
fooLocatorText = await page.locator('.fooClass').textContent();
492+
expect(fooLocatorText).toEqual('foo');
493+
`),
494+
output: test(`
495+
let fooLocatorText;
496+
fooLocatorText = 'foo';
497+
expect(fooLocatorText).toEqual('foo');
498+
fooLocatorText = page.locator('.fooClass');
499+
await expect(fooLocatorText).toHaveText('foo');
500+
`),
501+
errors: [
502+
{
503+
column: 9,
504+
data: { matcher: 'toHaveText', method: 'textContent' },
505+
endColumn: 31,
506+
line: 6,
507+
messageId: 'useWebFirstAssertion',
508+
},
509+
],
510+
},
511+
{
512+
code: test(`
513+
const unrelatedAssignment = "unrelated";
514+
const fooLocatorText = await page.locator('.foo').textContent();
515+
expect(fooLocatorText).toEqual('foo');
516+
`),
517+
output: test(`
518+
const unrelatedAssignment = "unrelated";
519+
const fooLocatorText = page.locator('.foo');
520+
await expect(fooLocatorText).toHaveText('foo');
521+
`),
522+
errors: [
523+
{
524+
column: 9,
525+
data: { matcher: 'toHaveText', method: 'textContent' },
526+
endColumn: 31,
527+
line: 4,
528+
messageId: 'useWebFirstAssertion',
529+
},
530+
],
531+
},
304532

305533
// isChecked
306534
{
@@ -736,5 +964,44 @@ runRuleTester('prefer-web-first-assertions', rule, {
736964
expect(myValue).toBeVisible();
737965
`),
738966
},
967+
{
968+
code: test(`
969+
let fooLocatorText;
970+
const fooLocator = page.locator('.fooClass');
971+
fooLocatorText = await fooLocator.textContent();
972+
fooLocatorText = 'foo';
973+
expect(fooLocatorText).toEqual('foo');
974+
`),
975+
},
976+
{
977+
code: test(`
978+
let fooLocatorText;
979+
let fooLocatorText2;
980+
const fooLocator = page.locator('.fooClass');
981+
fooLocatorText = 'foo';
982+
fooLocatorText2 = await fooLocator.textContent();
983+
expect(fooLocatorText).toEqual('foo');
984+
`),
985+
},
986+
{
987+
code: test(`
988+
let fooLocatorText;
989+
fooLocatorText = 'foo';
990+
expect(fooLocatorText).toEqual('foo')
991+
const fooLocator = page.locator('.fooClass');
992+
fooLocatorText = fooLocator;
993+
expect(fooLocatorText).toHaveText('foo');
994+
`),
995+
},
996+
{
997+
code: test(`
998+
const fooLocator = page.locator('.fooClass');
999+
let fooLocatorText;
1000+
fooLocatorText = fooLocator;
1001+
expect(fooLocatorText).toHaveText('foo');
1002+
fooLocatorText = 'foo';
1003+
expect(fooLocatorText).toEqual('foo')
1004+
`),
1005+
},
7391006
],
7401007
})

‎src/rules/prefer-web-first-assertions.ts

+60-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Rule } from 'eslint'
2-
import ESTree from 'estree'
2+
import ESTree, { AssignmentExpression } from 'estree'
33
import {
44
findParent,
55
getRawValue,
@@ -8,6 +8,7 @@ import {
88
} from '../utils/ast'
99
import { createRule } from '../utils/createRule'
1010
import { parseFnCall } from '../utils/parseFnCall'
11+
import { TypedNodeWithParent } from '../utils/types'
1112

1213
type MethodConfig = {
1314
inverse?: string
@@ -59,25 +60,75 @@ const supportedMatchers = new Set([
5960
'toBeFalsy',
6061
])
6162

63+
const isVariableDeclarator = (
64+
node: ESTree.Node,
65+
): node is TypedNodeWithParent<'VariableDeclarator'> =>
66+
node.type === 'VariableDeclarator'
67+
68+
const isAssignmentExpression = (
69+
node: ESTree.Node,
70+
): node is TypedNodeWithParent<'AssignmentExpression'> =>
71+
node.type === 'AssignmentExpression'
72+
73+
/**
74+
* Given a Node and an assignment expression, finds out if the assignment
75+
* expression happens before the node identifier (based on their range
76+
* properties) and if the assignment expression left side is of the same name as
77+
* the name of the given node.
78+
*
79+
* @param node The node we are comparing the assignment expression to.
80+
* @param assignment The assignment that will be verified to see if its left
81+
* operand is the same as the node.name and if it happens before it.
82+
* @returns True if the assignment left hand operator belongs to the node and
83+
* occurs before it, false otherwise. If either the node or the assignment
84+
* expression doesn't contain a range array, this will also return false
85+
* because their relative positions cannot be calculated.
86+
*/
87+
function isNodeLastAssignment(
88+
node: ESTree.Identifier,
89+
assignment: AssignmentExpression,
90+
) {
91+
if (node.range && assignment.range && node.range[0] < assignment.range[1]) {
92+
return false
93+
}
94+
95+
return (
96+
assignment.left.type === 'Identifier' && assignment.left.name === node.name
97+
)
98+
}
99+
62100
/**
63101
* If the expect call argument is a variable reference, finds the variable
64-
* initializer.
102+
* initializer or last variable assignment.
103+
*
104+
* If a variable is assigned after initialization we have to look for the last
105+
* time it was assigned because it could have been changed multiple times. We
106+
* then use its right hand assignment operator as the dereferenced node.
65107
*/
66108
function dereference(context: Rule.RuleContext, node: ESTree.Node | undefined) {
67109
if (node?.type !== 'Identifier') {
68110
return node
69111
}
70112

71113
const scope = context.sourceCode.getScope(node)
114+
const parents = scope.references
115+
.map((ref) => ref.identifier as Rule.Node)
116+
.map((ident) => ident.parent)
72117

73-
// Find the variable declaration and return the initializer
74-
for (const ref of scope.references) {
75-
const refParent = (ref.identifier as Rule.Node).parent
118+
// Look for any variable declarators in the scope references that match the
119+
// dereferenced node variable name
120+
const decl = parents
121+
.filter(isVariableDeclarator)
122+
.find((p) => p.id.type === 'Identifier' && p.id.name === node.name)
76123

77-
if (refParent.type === 'VariableDeclarator') {
78-
return refParent.init
79-
}
80-
}
124+
// Look for any variable assignments in the scope references and pick the last
125+
// one that matches the dereferenced node variable name
126+
const expr = parents
127+
.filter(isAssignmentExpression)
128+
.reverse()
129+
.find((assignment) => isNodeLastAssignment(node, assignment))
130+
131+
return expr?.right ?? decl?.init
81132
}
82133

83134
export default createRule({

0 commit comments

Comments
 (0)
Please sign in to comment.