Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(prefer-locator): Add rule to suggest not using page methods #315

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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')
carlbray marked this conversation as resolved.
Show resolved Hide resolved

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)
mskelton marked this conversation as resolved.
Show resolved Hide resolved
}

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