Skip to content

Commit

Permalink
feat(prefer-locator): Add rule to suggest not using page methods (#315)
Browse files Browse the repository at this point in the history
* Add prefer-page-locator-fill

* Rename rule

* Add set of methods to check against

* Update docs too

* Fix formatting

* Change descriptons, add more tests and examples

* Switch to CallExpression

* Add examples without await

* Use test from rule-tester

---------

Co-authored-by: Carl Bray <[email protected]>
  • Loading branch information
carlbray and Carl Bray authored Sep 19, 2024
1 parent 5c20c52 commit 731a4e1
Show file tree
Hide file tree
Showing 5 changed files with 200 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ CLI option\
| [prefer-hooks-on-top](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | |
| [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | 🔧 | |
| [prefer-native-locators](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-native-locators.md) | Suggest built-in locators over `page.locator()` | | 🔧 | |
| [prefer-locator](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-locator.md) | Suggest locators over page methods | | | |
| [prefer-strict-equal](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | | 💡 |
| [prefer-to-be](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-be.md) | Suggest using `toBe()` | | 🔧 | |
| [prefer-to-contain](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | | 🔧 | |
Expand Down
33 changes: 33 additions & 0 deletions docs/rules/prefer-locator.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Suggest using `page.locator()` (`prefer-locator`)

Suggest using locators and their associated methods instead of page methods for
performing actions.

## Rule details

This rule triggers a warning if page methods are used, instead of locators.

The following patterns are considered warnings:

```javascript
page.click('css=button')
await page.click('css=button')
await page.dblclick('xpath=//button')
await page.fill('input[type="password"]', 'password')

await page.frame('frame-name').click('css=button')
```

The following pattern are **not** warnings:

```javascript
const locator = page.locator('css=button')
await page.getByRole('password').fill('password')
await page.getByLabel('User Name').fill('John')
await page.getByRole('button', { name: 'Sign in' }).click()
await page.locator('input[type="password"]').fill('password')
await page.locator('css=button').click()
await page.locator('xpath=//button').dblclick()

await page.frameLocator('#my-iframe').getByText('Submit').click()
```
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import preferComparisonMatcher from './rules/prefer-comparison-matcher'
import preferEqualityMatcher from './rules/prefer-equality-matcher'
import preferHooksInOrder from './rules/prefer-hooks-in-order'
import preferHooksOnTop from './rules/prefer-hooks-on-top'
import preferLocator from './rules/prefer-locator'
import preferLowercaseTitle from './rules/prefer-lowercase-title'
import preferNativeLocators from './rules/prefer-native-locators'
import preferStrictEqual from './rules/prefer-strict-equal'
Expand Down Expand Up @@ -81,6 +82,7 @@ const index = {
'prefer-equality-matcher': preferEqualityMatcher,
'prefer-hooks-in-order': preferHooksInOrder,
'prefer-hooks-on-top': preferHooksOnTop,
'prefer-locator': preferLocator,
'prefer-lowercase-title': preferLowercaseTitle,
'prefer-native-locators': preferNativeLocators,
'prefer-strict-equal': preferStrictEqual,
Expand Down
99 changes: 99 additions & 0 deletions src/rules/prefer-locator.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { runRuleTester, test } from '../utils/rule-tester'
import rule from './prefer-locator'

runRuleTester('prefer-locator', rule, {
invalid: [
{
code: test(`await page.fill('input[type="password"]', 'password')`),
errors: [
{
column: 34,
endColumn: 81,
endLine: 1,
line: 1,
messageId: 'preferLocator',
},
],
output: null,
},
{
code: test(`await page.dblclick('xpath=//button')`),
errors: [
{
column: 34,
endColumn: 65,
endLine: 1,
line: 1,
messageId: 'preferLocator',
},
],
output: null,
},
{
code: `page.click('xpath=//button')`,
errors: [
{
column: 1,
endColumn: 29,
endLine: 1,
line: 1,
messageId: 'preferLocator',
},
],
output: null,
},
{
code: test(`await page.frame('frame-name').click('css=button')`),
errors: [
{
column: 34,
endColumn: 78,
endLine: 1,
line: 1,
messageId: 'preferLocator',
},
],
output: null,
},
{
code: `page.frame('frame-name').click('css=button')`,
errors: [
{
column: 1,
endColumn: 45,
endLine: 1,
line: 1,
messageId: 'preferLocator',
},
],
output: null,
},
],
valid: [
{
code: `const locator = page.locator('input[type="password"]')`,
},
{
code: test(
`await page.locator('input[type="password"]').fill('password')`,
),
},
{
code: test(`await page.locator('xpath=//button').dblclick()`),
},
{
code: `page.locator('xpath=//button').click()`,
},
{
code: test(
`await page.frameLocator('#my-iframe').locator('css=button').click()`,
),
},
{
code: test(`await page.evaluate('1 + 2')`),
},
{
code: `page.frame('frame-name')`,
},
],
})
65 changes: 65 additions & 0 deletions src/rules/prefer-locator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import ESTree from 'estree'
import { getStringValue, isPageMethod } from '../utils/ast'
import { createRule } from '../utils/createRule'

const pageMethods = new Set([
'click',
'dblclick',
'dispatchEvent',
'fill',
'focus',
'getAttribute',
'hover',
'innerHTML',
'innerText',
'inputValue',
'isChecked',
'isDisabled',
'isEditable',
'isEnabled',
'isHidden',
'isVisible',
'press',
'selectOption',
'setChecked',
'setInputFiles',
'tap',
'textContent',
'uncheck',
])

function isSupportedMethod(node: ESTree.CallExpression) {
if (node.callee.type !== 'MemberExpression') return false

const name = getStringValue(node.callee.property)
return pageMethods.has(name) && isPageMethod(node, name)
}

export default createRule({
create(context) {
return {
CallExpression(node) {
// Must be a method we care about
if (!isSupportedMethod(node)) return

context.report({
messageId: 'preferLocator',
node,
})
},
}
},
meta: {
docs: {
category: 'Best Practices',
description: 'Suggest locators over page methods',
recommended: false,
url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-locator.md',
},
messages: {
preferLocator: 'Prefer locator methods instead of page methods',
},
schema: [],
type: 'suggestion',
},
})

0 comments on commit 731a4e1

Please sign in to comment.