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 13 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
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ messages will use the default message defined by the plugin.
## Rules

✅ Set in the `recommended` configuration\
🔧 Automatically fixable by the [`--fix`](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix)
🔧 Automatically fixable by the
camchenry marked this conversation as resolved.
Show resolved Hide resolved
[`--fix`](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix)
CLI option\
💡 Manually fixable by
[editor suggestions](https://eslint.org/docs/latest/developer-guide/working-with-rules#providing-suggestions)
Expand Down Expand Up @@ -194,6 +195,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
34 changes: 34 additions & 0 deletions docs/rules/prefer-native-locators.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# 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')
```
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
98 changes: 98 additions & 0 deletions src/rules/prefer-native-locators.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
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('[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('[alt="Playwright logo"]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedAltTextQuery' }],
output: 'page.getByAltText("Playwright logo")',
},
{
code: `page.locator('[title="Additional context"]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedTitleQuery' }],
output: 'page.getByTitle("Additional context")',
},
{
code: `page.locator('[data-testid="password-input"]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedTestIdQuery' }],
output: 'page.getByTestId("password-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")',
},
// 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()',
},
// Works when it is not page.locator
{
code: `this.locator('[aria-label="View more"]')`,
errors: [{ column: 1, line: 1, messageId: 'unexpectedLabelQuery' }],
output: 'this.getByLabel("View more")',
},
],
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('[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"]')`,
},
],
})
139 changes: 139 additions & 0 deletions src/rules/prefer-native-locators.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
import { getStringValue, isPageMethod } from '../utils/ast'
import { createRule } from '../utils/createRule'

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

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

// If it's something like `page.locator`, just replace the `.locator` part
const start =
node.callee.type === 'MemberExpression'
? node.callee.property.range![0]
: node.range![0]
const end = node.range![1]
const rangeToReplace: [number, number] = [start, end]
camchenry marked this conversation as resolved.
Show resolved Hide resolved

const ariaLabelPattern = /^\[aria-label=['"](.+?)['"]\]$/
camchenry marked this conversation as resolved.
Show resolved Hide resolved
const labelMatch = query.match(ariaLabelPattern)
if (labelMatch) {
context.report({
fix(fixer) {
const newText = `getByLabel("${labelMatch[1]}")`
return fixer.replaceTextRange(rangeToReplace, newText)
},
messageId: 'unexpectedLabelQuery',
node,
})
}

const rolePattern = /^\[role=['"](.+?)['"]\]$/
const roleMatch = query.match(rolePattern)
if (roleMatch) {
context.report({
fix(fixer) {
const newText = `getByRole("${roleMatch[1]}")`
return fixer.replaceTextRange(rangeToReplace, newText)
},
messageId: 'unexpectedRoleQuery',
node,
})
}

const placeholderPattern = /^\[placeholder=['"](.+?)['"]\]$/
const placeholderMatch = query.match(placeholderPattern)
if (placeholderMatch) {
context.report({
fix(fixer) {
const newText = `getByPlaceholder("${placeholderMatch[1]}")`
return fixer.replaceTextRange(rangeToReplace, newText)
},
messageId: 'unexpectedPlaceholderQuery',
node,
})
}

const altTextPattern = /^\[alt=['"](.+?)['"]\]$/
const altTextMatch = query.match(altTextPattern)
if (altTextMatch) {
context.report({
fix(fixer) {
const newText = `getByAltText("${altTextMatch[1]}")`
return fixer.replaceTextRange(rangeToReplace, newText)
},
messageId: 'unexpectedAltTextQuery',
node,
})
}

const titlePattern = /^\[title=['"](.+?)['"]\]$/
const titleMatch = query.match(titlePattern)
if (titleMatch) {
context.report({
fix(fixer) {
const newText = `getByTitle("${titleMatch[1]}")`
return fixer.replaceTextRange(rangeToReplace, newText)
},
messageId: 'unexpectedTitleQuery',
node,
})
}

const testIdPattern = new RegExp(
`^\\[${testIdAttribute}=['"](.+?)['"]\\]`,
)
const testIdMatch = query.match(testIdPattern)
if (testIdMatch) {
context.report({
fix(fixer) {
const newText = `getByTestId("${testIdMatch[1]}")`
return fixer.replaceTextRange(rangeToReplace, newText)
},
messageId: 'unexpectedTestIdQuery',
node,
})
}
},
}
},
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',
camchenry marked this conversation as resolved.
Show resolved Hide resolved
},
schema: [
{
additionalProperties: false,
properties: {
testIdAttribute: {
default: 'data-testid',
type: 'string',
},
},
type: 'object',
},
],
type: 'suggestion',
},
})