Skip to content

Commit c0937d7

Browse files
feat: Add no-unused-locators rule (#396)
* Initial implementation of rule no-unused-locators * Add code for user action is awaited * Add test for inside an expect * Move comments * Add other locators * Move locators to array clean method * Cleanup --------- Co-authored-by: Mark Skelton <[email protected]>
1 parent 6b85256 commit c0937d7

File tree

6 files changed

+107
-1
lines changed

6 files changed

+107
-1
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ CLI option\
142142
| [no-slowed-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-slowed-test.md) | Disallow usage of the `.slow` annotation | | | 💡 |
143143
| [no-standalone-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-standalone-expect.md) | Disallow using expect outside of `test` blocks || | |
144144
| [no-unsafe-references](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-unsafe-references.md) | Prevent unsafe variable references in `page.evaluate()` || 🔧 | |
145+
| [no-unused-locators](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-unused-locators.md) | Disallow usage of page locators that are not used || | |
145146
| [no-useless-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-await.md) | Disallow unnecessary `await`s for Playwright methods || 🔧 | |
146147
| [no-useless-not](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-not.md) | Disallow usage of `not` matchers when a specific matcher exists || 🔧 | |
147148
| [no-wait-for-navigation](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-navigation.md) | Disallow usage of `page.waitForNavigation()` || | 💡 |

docs/rules/no-unused-locators.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# Disallow usage of page locators that are not used (`no-unused-locators`)
2+
3+
Using locators without performing any actions or assertions on them can lead to
4+
unexpected behavior/flakiness in tests. This rule helps ensure that locators are
5+
used in some way by requiring that they are either acted upon or asserted
6+
against.
7+
8+
## Rule Details
9+
10+
Examples of **incorrect** code for this rule:
11+
12+
```javascript
13+
page.getByRole('button', { name: 'Sign in' })
14+
```
15+
16+
Examples of **correct** code for this rule:
17+
18+
```javascript
19+
const btn = page.getByRole('button', { name: 'Sign in' })
20+
21+
await page.getByRole('button', { name: 'Sign in' }).click()
22+
23+
await expect(page.getByRole('button', { name: 'Sign in' })).toBeVisible()
24+
```

src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import noSkippedTest from './rules/no-skipped-test.js'
2323
import noSlowedTest from './rules/no-slowed-test.js'
2424
import noStandaloneExpect from './rules/no-standalone-expect.js'
2525
import noUnsafeReferences from './rules/no-unsafe-references.js'
26+
import noUnusedLocators from './rules/no-unused-locators.js'
2627
import noUselessAwait from './rules/no-useless-await.js'
2728
import noUselessNot from './rules/no-useless-not.js'
2829
import noWaitForNavigation from './rules/no-wait-for-navigation.js'
@@ -78,6 +79,7 @@ const index = {
7879
'no-slowed-test': noSlowedTest,
7980
'no-standalone-expect': noStandaloneExpect,
8081
'no-unsafe-references': noUnsafeReferences,
82+
'no-unused-locators': noUnusedLocators,
8183
'no-useless-await': noUselessAwait,
8284
'no-useless-not': noUselessNot,
8385
'no-wait-for-navigation': noWaitForNavigation,
@@ -126,6 +128,7 @@ const sharedConfig = {
126128
'playwright/no-skipped-test': 'warn',
127129
'playwright/no-standalone-expect': 'error',
128130
'playwright/no-unsafe-references': 'error',
131+
'playwright/no-unused-locators': 'error',
129132
'playwright/no-useless-await': 'warn',
130133
'playwright/no-useless-not': 'warn',
131134
'playwright/no-wait-for-navigation': 'error',
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { runRuleTester } from '../utils/rule-tester.js'
2+
import rule from './no-unused-locators.js'
3+
4+
runRuleTester('no-unused-locators', rule, {
5+
invalid: [
6+
{
7+
code: "page.getByRole('button', { name: 'Sign in' })",
8+
errors: [
9+
{
10+
column: 1,
11+
endColumn: 46,
12+
endLine: 1,
13+
line: 1,
14+
messageId: 'noUnusedLocator',
15+
},
16+
],
17+
},
18+
{
19+
code: "await page.getByTestId('my-test-id')",
20+
errors: [
21+
{
22+
column: 7,
23+
endColumn: 37,
24+
endLine: 1,
25+
line: 1,
26+
messageId: 'noUnusedLocator',
27+
},
28+
],
29+
},
30+
],
31+
valid: [
32+
`await page.getByRole('button', { name: 'Sign in' }).all()`,
33+
"const btn = page.getByLabel('Sign in')",
34+
"const btn = page.getByPlaceholder('User Name').first()",
35+
"await page.getByRole('button', { name: 'Sign in' }).click()",
36+
"expect(page.getByTestId('User Name')).toBeVisible()",
37+
"expect(page.getByRole('User Name').first()).toBeVisible()",
38+
],
39+
})

src/rules/no-unused-locators.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { isPageMethod } from '../utils/ast.js'
2+
import { createRule } from '../utils/createRule.js'
3+
4+
const LOCATOR_REGEX =
5+
/locator|getBy(Role|Text|Label|Placeholder|AltText|Title|TestId)/
6+
7+
export default createRule({
8+
create(context) {
9+
return {
10+
CallExpression(node) {
11+
if (!isPageMethod(node, LOCATOR_REGEX)) {
12+
return
13+
}
14+
15+
if (
16+
node.parent.type === 'ExpressionStatement' ||
17+
node.parent.type === 'AwaitExpression'
18+
) {
19+
context.report({ messageId: 'noUnusedLocator', node })
20+
}
21+
},
22+
}
23+
},
24+
meta: {
25+
docs: {
26+
category: 'Possible Errors',
27+
description: `Disallow usage of page locators that are not used`,
28+
recommended: true,
29+
url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-unused-locators.md',
30+
},
31+
messages: {
32+
noUnusedLocator: 'Unused locator',
33+
},
34+
type: 'problem',
35+
},
36+
})

src/utils/ast.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,10 @@ export function dig(node: ESTree.Node, identifier: string | RegExp): boolean {
116116
: false
117117
}
118118

119-
export function isPageMethod(node: ESTree.CallExpression, name: string) {
119+
export function isPageMethod(
120+
node: ESTree.CallExpression,
121+
name: string | RegExp,
122+
) {
120123
return (
121124
node.callee.type === 'MemberExpression' &&
122125
dig(node.callee.object, /(^(page|frame)|(Page|Frame)$)/) &&

0 commit comments

Comments
 (0)