From e6f92b0280b65cfead49499175c4a40921ef51f3 Mon Sep 17 00:00:00 2001 From: Cam McHenry Date: Fri, 6 Sep 2024 12:02:11 -0400 Subject: [PATCH] feat(prefer-native-locators): Add rule to suggest using built-in locators (#308) * Add label text query * Add role query * Add placeholder query * Add alt text query * Add test ID query * Add title query * Simplify text matching and replacement * Put new text in its own variable * Add support for custom test ID attribute * Add more tests * Add docs * Add prefer-native-locators to README * Export prefer-native-locators rule * Move patterns to array * Drop periods from messages * Remove non-page locator method check * Add tests for empty string + no args * Use AST.Range instead of tuple * Add more tests * Allow replacing for selectors without quotes * Add docs on testIdAttribute * Undo line-break * Use same RegExp for each attribute * Move range into fixer * Remove unnecessary identifier * Add more test cases --- README.md | 1 + docs/rules/prefer-native-locators.md | 70 ++++++++ src/index.ts | 2 + src/rules/prefer-native-locators.test.ts | 194 +++++++++++++++++++++++ src/rules/prefer-native-locators.ts | 123 ++++++++++++++ 5 files changed, 390 insertions(+) create mode 100644 docs/rules/prefer-native-locators.md create mode 100644 src/rules/prefer-native-locators.test.ts create mode 100644 src/rules/prefer-native-locators.ts diff --git a/README.md b/README.md index b4c0e02..73d387a 100644 --- a/README.md +++ b/README.md @@ -194,6 +194,7 @@ CLI option\ | [prefer-hooks-in-order](https://github.com/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://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-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()` | | 🔧 | | diff --git a/docs/rules/prefer-native-locators.md b/docs/rules/prefer-native-locators.md new file mode 100644 index 0000000..22aadfa --- /dev/null +++ b/docs/rules/prefer-native-locators.md @@ -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') +``` diff --git a/src/index.ts b/src/index.ts index e37a74c..c185547 100644 --- a/src/index.ts +++ b/src/index.ts @@ -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' @@ -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, diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts new file mode 100644 index 0000000..d795fd3 --- /dev/null +++ b/src/rules/prefer-native-locators.test.ts @@ -0,0 +1,194 @@ +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()', + }, + // Works as part of a declaration or other usage + { + code: `const dialog = page.locator('[role="dialog"]')`, + errors: [{ column: 16, line: 1, messageId: 'unexpectedRoleQuery' }], + output: 'const dialog = page.getByRole("dialog")', + }, + { + code: `this.closeModalLocator = this.page.locator('[data-test=close-modal]');`, + errors: [{ column: 26, line: 1, messageId: 'unexpectedTestIdQuery' }], + options: [{ testIdAttribute: 'data-test' }], + output: 'this.closeModalLocator = this.page.getByTestId("close-modal");', + }, + { + code: `export class TestClass { + container = () => this.page.locator('[data-testid="container"]'); + }`, + errors: [{ column: 27, line: 2, messageId: 'unexpectedTestIdQuery' }], + output: `export class TestClass { + container = () => this.page.getByTestId("container"); + }`, + }, + { + code: `export class TestClass { + get alert() { + return this.page.locator("[role='alert']"); + } + }`, + errors: [{ column: 18, line: 3, messageId: 'unexpectedRoleQuery' }], + output: `export class TestClass { + get alert() { + return this.page.getByRole("alert"); + } + }`, + }, + ], + valid: [ + { code: 'page.getByLabel("View more")' }, + { code: 'page.getByRole("button")' }, + { code: 'page.getByRole("button", {name: "Open"})' }, + { code: 'page.getByPlaceholder("Enter some text...")' }, + { code: 'page.getByAltText("Playwright logo")' }, + { code: 'page.getByTestId("password-input")' }, + { code: 'page.getByTitle("Additional context")' }, + { code: 'this.page.getByLabel("View more")' }, + { code: 'this.page.getByRole("button")' }, + { code: 'this.page.getByPlaceholder("Enter some text...")' }, + { code: 'this.page.getByAltText("Playwright logo")' }, + { code: 'this.page.getByTestId("password-input")' }, + { code: 'this.page.getByTitle("Additional context")' }, + { code: 'page.locator(".class")' }, + { code: 'page.locator("#id")' }, + { code: 'this.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"]')`, + }, + { + code: `this.page.locator('[complex-query] > [title="Additional context"]')`, + }, + // Works for empty string and no arguments + { code: `page.locator('')` }, + { code: `page.locator()` }, + // Works for classes and declarations + { code: `const dialog = page.getByRole("dialog")` }, + { + code: `export class TestClass { + get alert() { + return this.page.getByRole("alert"); + } + }`, + }, + { + code: `export class TestClass { + container = () => this.page.getByTestId("container"); + }`, + }, + { code: `this.closeModalLocator = this.page.getByTestId("close-modal");` }, + ], +}) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts new file mode 100644 index 0000000..25a38e5 --- /dev/null +++ b/src/rules/prefer-native-locators.ts @@ -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) ?? {}), + } + + const patterns = compilePatterns({ testIdAttribute }) + + return { + CallExpression(node) { + if (node.callee.type !== 'MemberExpression') return + const query = getStringValue(node.arguments[0]) + if (!isPageMethod(node, 'locator')) return + + for (const pattern of patterns) { + const match = query.match(pattern.pattern) + if (match) { + context.report({ + 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://github.com/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', + }, +})