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

Add rule to suggest using built-in locators (prefer-native-locators) #308

Merged
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
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 @@ -194,6 +194,7 @@ CLI option\
| [prefer-hooks-in-order](https:/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-in-order.md) | Prefer having hooks in a consistent order | | | |
| [prefer-hooks-on-top](https:/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:/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | 🔧 | |
| [prefer-native-locators](https:/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-native-locators.md) | Suggest built-in locators over `page.locator()` | | 🔧 | |
| [prefer-strict-equal](https:/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | | 💡 |
| [prefer-to-be](https:/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-be.md) | Suggest using `toBe()` | | 🔧 | |
| [prefer-to-contain](https:/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | | 🔧 | |
Expand Down
70 changes: 70 additions & 0 deletions docs/rules/prefer-native-locators.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# Suggest using native Playwright locators (`prefer-native-locators`)

Playwright has built-in locators for common query selectors such as finding
elements by placeholder text, ARIA role, accessible name, and more. This rule
suggests using these native locators instead of using `page.locator()` with an
equivalent selector.

In some cases this can be more robust too, such as finding elements by ARIA role
or accessible name, because some elements have implicit roles, and there are
multiple ways to specify accessible names.

## Rule details

Examples of **incorrect** code for this rule:

```javascript
page.locator('[aria-label="View more"]')
page.locator('[role="button"]')
page.locator('[placeholder="Enter some text..."]')
page.locator('[alt="Playwright logo"]')
page.locator('[title="Additional context"]')
page.locator('[data-testid="password-input"]')
```

Examples of **correct** code for this rule:

```javascript
page.getByLabel('View more')
page.getByRole('Button')
page.getByPlaceholder('Enter some text...')
page.getByAltText('Playwright logo')
page.getByTestId('password-input')
page.getByTitle('Additional context')
```

## Options

```json
{
"playwright/prefer-native-locators": [
"error",
{
"testIdAttribute": "data-testid"
}
]
}
```

### `testIdAttribute`

Default: `data-testid`

This string option specifies the test ID attribute to look for and replace with
`page.getByTestId()` calls. If you are using
[`page.setTestIdAttribute()`](https://playwright.dev/docs/api/class-selectors#selectors-set-test-id-attribute),
this should be set to the same value as what you pass in to that method.

Examples of **incorrect** code when using
`{ "testIdAttribute": "data-custom-testid" }` option:

```js
page.locator('[data-custom-testid="password-input"]')
```

Examples of **correct** code when using
`{ "testIdAttribute": "data-custom-testid" }` option:

```js
page.getByTestId('password-input')
```
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import preferEqualityMatcher from './rules/prefer-equality-matcher'
import preferHooksInOrder from './rules/prefer-hooks-in-order'
import preferHooksOnTop from './rules/prefer-hooks-on-top'
import preferLowercaseTitle from './rules/prefer-lowercase-title'
import preferNativeLocators from './rules/prefer-native-locators'
import preferStrictEqual from './rules/prefer-strict-equal'
import preferToBe from './rules/prefer-to-be'
import preferToContain from './rules/prefer-to-contain'
Expand Down Expand Up @@ -81,6 +82,7 @@ const index = {
'prefer-hooks-in-order': preferHooksInOrder,
'prefer-hooks-on-top': preferHooksOnTop,
'prefer-lowercase-title': preferLowercaseTitle,
'prefer-native-locators': preferNativeLocators,
'prefer-strict-equal': preferStrictEqual,
'prefer-to-be': preferToBe,
'prefer-to-contain': preferToContain,
Expand Down
134 changes: 134 additions & 0 deletions src/rules/prefer-native-locators.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import { runRuleTester } from '../utils/rule-tester'
import rule from './prefer-native-locators'

runRuleTester('prefer-native-locators', rule, {
invalid: [
{
code: `page.locator('[aria-label="View more"]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedLabelQuery' }],
output: 'page.getByLabel("View more")',
},
{
code: `page.locator('[aria-label=Edit]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedLabelQuery' }],
output: 'page.getByLabel("Edit")',
},
{
code: `page.locator('[role="button"]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedRoleQuery' }],
output: 'page.getByRole("button")',
},
{
code: `page.locator('[role=button]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedRoleQuery' }],
output: 'page.getByRole("button")',
},
{
code: `page.locator('[placeholder="Enter some text..."]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedPlaceholderQuery' }],
output: 'page.getByPlaceholder("Enter some text...")',
},
{
code: `page.locator('[placeholder=Name]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedPlaceholderQuery' }],
output: 'page.getByPlaceholder("Name")',
},
{
code: `page.locator('[alt="Playwright logo"]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedAltTextQuery' }],
output: 'page.getByAltText("Playwright logo")',
},
{
code: `page.locator('[alt=Logo]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedAltTextQuery' }],
output: 'page.getByAltText("Logo")',
},
{
code: `page.locator('[title="Additional context"]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedTitleQuery' }],
output: 'page.getByTitle("Additional context")',
},
{
code: `page.locator('[title=Context]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedTitleQuery' }],
output: 'page.getByTitle("Context")',
},
{
code: `page.locator('[data-testid="password-input"]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedTestIdQuery' }],
output: 'page.getByTestId("password-input")',
},
{
code: `page.locator('[data-testid=input]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedTestIdQuery' }],
output: 'page.getByTestId("input")',
},
{
code: `page.locator('[data-custom-testid="password-input"]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedTestIdQuery' }],
options: [{ testIdAttribute: 'data-custom-testid' }],
output: 'page.getByTestId("password-input")',
},
{
code: `page.locator('[data-custom-testid=input]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedTestIdQuery' }],
options: [{ testIdAttribute: 'data-custom-testid' }],
output: 'page.getByTestId("input")',
},
// Works when locators are chained
{
code: `this.page.locator('[role="heading"]').first()`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedRoleQuery' }],
output: 'this.page.getByRole("heading").first()',
},
// Works when used inside an assertion
{
code: `await expect(page.locator('[role="alert"]')).toBeVisible()`,
errors: [{ column: 14, line: 1, messageId: 'unexpectedRoleQuery' }],
output: 'await expect(page.getByRole("alert")).toBeVisible()',
},
{
code: `await expect(page.locator('[data-testid="top"]')).toContainText(firstRule)`,
errors: [{ column: 14, line: 1, messageId: 'unexpectedTestIdQuery' }],
output: 'await expect(page.getByTestId("top")).toContainText(firstRule)',
},
// Works when used as part of an action
{
code: `await page.locator('[placeholder="New password"]').click()`,
errors: [{ column: 7, line: 1, messageId: 'unexpectedPlaceholderQuery' }],
output: 'await page.getByPlaceholder("New password").click()',
},
],
valid: [
{ code: 'page.getByLabel("View more")' },
camchenry marked this conversation as resolved.
Show resolved Hide resolved
{ code: 'page.getByRole("button")' },
{ code: 'page.getByPlaceholder("Enter some text...")' },
{ code: 'page.getByAltText("Playwright logo")' },
{ code: 'page.getByTestId("password-input")' },
{ code: 'page.getByTitle("Additional context")' },
{ code: 'page.locator(".class")' },
{ code: 'page.locator("#id")' },
// Does not match on more complex queries
{
code: `page.locator('[complex-query] > [aria-label="View more"]')`,
},
{
code: `page.locator('[complex-query] > [role="button"]')`,
},
{
code: `page.locator('[complex-query] > [placeholder="Enter some text..."]')`,
},
{
code: `page.locator('[complex-query] > [alt="Playwright logo"]')`,
},
{
code: `page.locator('[complex-query] > [data-testid="password-input"]')`,
},
{
code: `page.locator('[complex-query] > [title="Additional context"]')`,
},
// Works for empty string and no arguments
{ code: `page.locator('')` },
{ code: `page.locator()` },
],
})
123 changes: 123 additions & 0 deletions src/rules/prefer-native-locators.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import { AST } from 'eslint'
import { getStringValue, isPageMethod } from '../utils/ast'
import { createRule } from '../utils/createRule'

type Pattern = {
messageId: string
pattern: RegExp
replacement: string
}

const compilePatterns = ({
testIdAttribute,
}: {
testIdAttribute: string
}): Pattern[] => {
const patterns = [
{
attribute: 'aria-label',
messageId: 'unexpectedLabelQuery',
replacement: 'getByLabel',
},
{
attribute: 'role',
messageId: 'unexpectedRoleQuery',
replacement: 'getByRole',
},
{
attribute: 'placeholder',
messageId: 'unexpectedPlaceholderQuery',
replacement: 'getByPlaceholder',
},
{
attribute: 'alt',
messageId: 'unexpectedAltTextQuery',
replacement: 'getByAltText',
},
{
attribute: 'title',
messageId: 'unexpectedTitleQuery',
replacement: 'getByTitle',
},
{
attribute: testIdAttribute,
messageId: 'unexpectedTestIdQuery',
replacement: 'getByTestId',
},
]
return patterns.map(({ attribute, ...pattern }) => ({
...pattern,
pattern: new RegExp(`^\\[${attribute}=['"]?(.+?)['"]?\\]$`),
}))
}

export default createRule({
create(context) {
const { testIdAttribute } = {
testIdAttribute: 'data-testid',
...((context.options?.[0] as Record<string, unknown>) ?? {}),
}

const patterns = compilePatterns({ testIdAttribute })

return {
CallExpression(node) {
if (node.callee.type !== 'MemberExpression') return
const query = getStringValue(node.arguments[0])
camchenry marked this conversation as resolved.
Show resolved Hide resolved
if (!isPageMethod(node, 'locator')) return

for (const pattern of patterns) {
const match = query.match(pattern.pattern)
if (match) {
context.report({
camchenry marked this conversation as resolved.
Show resolved Hide resolved
fix(fixer) {
const start =
node.callee.type === 'MemberExpression'
? node.callee.property.range![0]
: node.range![0]
const end = node.range![1]
const rangeToReplace: AST.Range = [start, end]

const newText = `${pattern.replacement}("${match[1]}")`
return fixer.replaceTextRange(rangeToReplace, newText)
},
messageId: pattern.messageId,
node,
})
return
}
}
},
}
},
meta: {
docs: {
category: 'Best Practices',
description: 'Prefer native locator functions',
recommended: false,
url: 'https:/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-native-locators.md',
},
fixable: 'code',
messages: {
unexpectedAltTextQuery: 'Use getByAltText() instead',
unexpectedLabelQuery: 'Use getByLabel() instead',
unexpectedPlaceholderQuery: 'Use getByPlaceholder() instead',
unexpectedRoleQuery: 'Use getByRole() instead',
unexpectedTestIdQuery: 'Use getByTestId() instead',
unexpectedTitleQuery: 'Use getByTitle() instead',
},
schema: [
{
additionalProperties: false,
properties: {
testIdAttribute: {
default: 'data-testid',
type: 'string',
},
},
type: 'object',
},
],
type: 'suggestion',
},
})