From 56eea9c26be3db5b4d1e8fef6b26ca71d2c05b8a Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 21:26:36 -0400 Subject: [PATCH 01/26] Add label text query --- src/rules/prefer-native-locators.test.ts | 18 ++++++++ src/rules/prefer-native-locators.ts | 55 ++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 src/rules/prefer-native-locators.test.ts create mode 100644 src/rules/prefer-native-locators.ts diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts new file mode 100644 index 0000000..fbc33fe --- /dev/null +++ b/src/rules/prefer-native-locators.test.ts @@ -0,0 +1,18 @@ +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")', + }, + ], + valid: [ + { code: 'page.getByLabel("View more")' }, + { + code: `page.locator('[something-more-complex][aria-label="View more"]')`, + }, + ], +}) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts new file mode 100644 index 0000000..2d8c234 --- /dev/null +++ b/src/rules/prefer-native-locators.ts @@ -0,0 +1,55 @@ +import { getStringValue, isPageMethod } from '../utils/ast' +import { createRule } from '../utils/createRule' + +export default createRule({ + create(context) { + return { + CallExpression(node) { + if (node.callee.type !== 'MemberExpression') return + const method = getStringValue(node.callee.property) + const query = getStringValue(node.arguments[0]) + const isLocator = isPageMethod(node, 'locator') || method === 'locator' + if (!isLocator) return + + const ariaLabelPattern = /^\[aria-label=['"](.+?)['"]\]$/ + if (query.match(ariaLabelPattern)) { + context.report({ + fix(fixer) { + const [, label] = query.match(ariaLabelPattern) ?? [] + // Replace .locator(...) with .getByLabel(...) + const start = + node.callee.type === 'MemberExpression' + ? node.callee.property.range![0] + : node.range![0] + const end = node.range![1] + return fixer.replaceTextRange( + [start, end], + `getByLabel("${label}")`, + ) + }, + messageId: 'unexpectedLabelQuery', + node, + }) + } + }, + } + }, + 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', + }, + schema: [], + type: 'suggestion', + }, +}) From 3cce6b9f7fca5ff75013de22bb7a1bae1afcd3af Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 21:32:09 -0400 Subject: [PATCH 02/26] Add role query --- src/rules/prefer-native-locators.test.ts | 11 ++++++++++- src/rules/prefer-native-locators.ts | 21 ++++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index fbc33fe..84660f9 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -8,11 +8,20 @@ runRuleTester('prefer-native-locators', rule, { 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")', + }, ], valid: [ { code: 'page.getByLabel("View more")' }, + { code: 'page.getByRole("Button")' }, + { + code: `page.locator('[complex-query] > [aria-label="View more"]')`, + }, { - code: `page.locator('[something-more-complex][aria-label="View more"]')`, + code: `page.locator('[complex-query] > [role="button"]')`, }, ], }) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index 2d8c234..75b370e 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -16,7 +16,6 @@ export default createRule({ context.report({ fix(fixer) { const [, label] = query.match(ariaLabelPattern) ?? [] - // Replace .locator(...) with .getByLabel(...) const start = node.callee.type === 'MemberExpression' ? node.callee.property.range![0] @@ -31,6 +30,26 @@ export default createRule({ node, }) } + + const rolePattern = /^\[role=['"](.+?)['"]\]$/ + if (query.match(rolePattern)) { + context.report({ + fix(fixer) { + const [, role] = query.match(rolePattern) ?? [] + const start = + node.callee.type === 'MemberExpression' + ? node.callee.property.range![0] + : node.range![0] + const end = node.range![1] + return fixer.replaceTextRange( + [start, end], + `getByRole("${role}")`, + ) + }, + messageId: 'unexpectedRoleQuery', + node, + }) + } }, } }, From f30eeaa86dffba3f0629b2899bee030aebce3cc7 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 21:37:13 -0400 Subject: [PATCH 03/26] Add placeholder query --- src/rules/prefer-native-locators.test.ts | 9 +++++++++ src/rules/prefer-native-locators.ts | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index 84660f9..a3a3840 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -13,15 +13,24 @@ runRuleTester('prefer-native-locators', rule, { 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...")', + }, ], valid: [ { code: 'page.getByLabel("View more")' }, { code: 'page.getByRole("Button")' }, + { code: 'page.getByPlaceholder("Enter some text...")' }, { 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..."]')`, + }, ], }) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index 75b370e..95aade9 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -50,6 +50,26 @@ export default createRule({ node, }) } + + const placeholderPattern = /^\[placeholder=['"](.+?)['"]\]$/ + if (query.match(placeholderPattern)) { + context.report({ + fix(fixer) { + const [, placeholder] = query.match(placeholderPattern) ?? [] + const start = + node.callee.type === 'MemberExpression' + ? node.callee.property.range![0] + : node.range![0] + const end = node.range![1] + return fixer.replaceTextRange( + [start, end], + `getByPlaceholder("${placeholder}")`, + ) + }, + messageId: 'unexpectedPlaceholderQuery', + node, + }) + } }, } }, From d19ef20c3c68ccb670702235d2d1978eab98b3b3 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 21:40:38 -0400 Subject: [PATCH 04/26] Add alt text query --- src/rules/prefer-native-locators.test.ts | 9 +++++++++ src/rules/prefer-native-locators.ts | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index a3a3840..0db874d 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -18,11 +18,17 @@ runRuleTester('prefer-native-locators', rule, { 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")', + }, ], valid: [ { code: 'page.getByLabel("View more")' }, { code: 'page.getByRole("Button")' }, { code: 'page.getByPlaceholder("Enter some text...")' }, + { code: 'page.getByAltText("Playwright logo")' }, { code: `page.locator('[complex-query] > [aria-label="View more"]')`, }, @@ -32,5 +38,8 @@ runRuleTester('prefer-native-locators', rule, { { code: `page.locator('[complex-query] > [placeholder="Enter some text..."]')`, }, + { + code: `page.locator('[complex-query] > [alt="Playwright logo"]')`, + }, ], }) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index 95aade9..b5260ba 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -70,6 +70,26 @@ export default createRule({ node, }) } + + const altTextPattern = /^\[alt=['"](.+?)['"]\]$/ + if (query.match(altTextPattern)) { + context.report({ + fix(fixer) { + const [, alt] = query.match(altTextPattern) ?? [] + const start = + node.callee.type === 'MemberExpression' + ? node.callee.property.range![0] + : node.range![0] + const end = node.range![1] + return fixer.replaceTextRange( + [start, end], + `getByAltText("${alt}")`, + ) + }, + messageId: 'unexpectedAltTextQuery', + node, + }) + } }, } }, From 01e14d36cb5fb594219e22a740e209bf81a3b12d Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 21:45:05 -0400 Subject: [PATCH 05/26] Add test ID query --- src/rules/prefer-native-locators.test.ts | 9 +++++++++ src/rules/prefer-native-locators.ts | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index 0db874d..7bc5aac 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -23,12 +23,18 @@ runRuleTester('prefer-native-locators', rule, { errors: [{ column: 1, line: 1, messageId: 'unexpectedAltTextQuery' }], output: 'page.getByAltText("Playwright logo")', }, + { + code: `page.locator('[data-testid="password-input"]')`, + errors: [{ column: 1, line: 1, messageId: 'unexpectedTestIdQuery' }], + output: 'page.getByTestId("password-input")', + }, ], valid: [ { code: 'page.getByLabel("View more")' }, { code: 'page.getByRole("Button")' }, { code: 'page.getByPlaceholder("Enter some text...")' }, { code: 'page.getByAltText("Playwright logo")' }, + { code: 'page.getByTestId("password-input")' }, { code: `page.locator('[complex-query] > [aria-label="View more"]')`, }, @@ -41,5 +47,8 @@ runRuleTester('prefer-native-locators', rule, { { code: `page.locator('[complex-query] > [alt="Playwright logo"]')`, }, + { + code: `page.locator('[complex-query] > [data-testid="password-input"]')`, + }, ], }) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index b5260ba..0fb3a57 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -90,6 +90,30 @@ export default createRule({ node, }) } + + // TODO: Add support for custom test ID attribute + const testIdAttributeName = 'data-testid' + const testIdPattern = new RegExp( + `^\\[${testIdAttributeName}=['"](.+?)['"]\\]`, + ) + if (query.match(testIdPattern)) { + context.report({ + fix(fixer) { + const [, testId] = query.match(testIdPattern) ?? [] + const start = + node.callee.type === 'MemberExpression' + ? node.callee.property.range![0] + : node.range![0] + const end = node.range![1] + return fixer.replaceTextRange( + [start, end], + `getByTestId("${testId}")`, + ) + }, + messageId: 'unexpectedTestIdQuery', + node, + }) + } }, } }, From 98795fa9384165b20f124a09d31711937ffe5042 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 21:48:03 -0400 Subject: [PATCH 06/26] Add title query --- src/rules/prefer-native-locators.test.ts | 9 +++++++++ src/rules/prefer-native-locators.ts | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index 7bc5aac..5bd15a5 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -23,6 +23,11 @@ runRuleTester('prefer-native-locators', rule, { 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' }], @@ -35,6 +40,7 @@ runRuleTester('prefer-native-locators', rule, { { 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"]')`, }, @@ -50,5 +56,8 @@ runRuleTester('prefer-native-locators', rule, { { code: `page.locator('[complex-query] > [data-testid="password-input"]')`, }, + { + code: `page.locator('[complex-query] > [title="Additional context"]')`, + }, ], }) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index 0fb3a57..81760cc 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -91,6 +91,26 @@ export default createRule({ }) } + const titlePattern = /^\[title=['"](.+?)['"]\]$/ + if (query.match(titlePattern)) { + context.report({ + fix(fixer) { + const [, title] = query.match(titlePattern) ?? [] + const start = + node.callee.type === 'MemberExpression' + ? node.callee.property.range![0] + : node.range![0] + const end = node.range![1] + return fixer.replaceTextRange( + [start, end], + `getByTitle("${title}")`, + ) + }, + messageId: 'unexpectedTitleQuery', + node, + }) + } + // TODO: Add support for custom test ID attribute const testIdAttributeName = 'data-testid' const testIdPattern = new RegExp( @@ -131,6 +151,7 @@ export default createRule({ unexpectedPlaceholderQuery: 'Use .getByPlaceholder() instead', unexpectedRoleQuery: 'Use .getByRole() instead', unexpectedTestIdQuery: 'Use .getByTestId() instead', + unexpectedTitleQuery: 'Use .getByTitle() instead', }, schema: [], type: 'suggestion', From dacf2c2780d5dbeb76b0d2bfbd20616e087662fc Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 22:08:41 -0400 Subject: [PATCH 07/26] Simplify text matching and replacement --- src/rules/prefer-native-locators.ts | 86 +++++++++++------------------ 1 file changed, 32 insertions(+), 54 deletions(-) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index 81760cc..a327817 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -11,19 +11,22 @@ export default createRule({ const isLocator = isPageMethod(node, 'locator') || method === 'locator' 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] + const ariaLabelPattern = /^\[aria-label=['"](.+?)['"]\]$/ - if (query.match(ariaLabelPattern)) { + const labelMatch = query.match(ariaLabelPattern) + if (labelMatch) { context.report({ fix(fixer) { - const [, label] = query.match(ariaLabelPattern) ?? [] - const start = - node.callee.type === 'MemberExpression' - ? node.callee.property.range![0] - : node.range![0] - const end = node.range![1] return fixer.replaceTextRange( - [start, end], - `getByLabel("${label}")`, + rangeToReplace, + `getByLabel("${labelMatch[1]}")`, ) }, messageId: 'unexpectedLabelQuery', @@ -32,18 +35,13 @@ export default createRule({ } const rolePattern = /^\[role=['"](.+?)['"]\]$/ - if (query.match(rolePattern)) { + const roleMatch = query.match(rolePattern) + if (roleMatch) { context.report({ fix(fixer) { - const [, role] = query.match(rolePattern) ?? [] - const start = - node.callee.type === 'MemberExpression' - ? node.callee.property.range![0] - : node.range![0] - const end = node.range![1] return fixer.replaceTextRange( - [start, end], - `getByRole("${role}")`, + rangeToReplace, + `getByRole("${roleMatch[1]}")`, ) }, messageId: 'unexpectedRoleQuery', @@ -52,18 +50,13 @@ export default createRule({ } const placeholderPattern = /^\[placeholder=['"](.+?)['"]\]$/ - if (query.match(placeholderPattern)) { + const placeholderMatch = query.match(placeholderPattern) + if (placeholderMatch) { context.report({ fix(fixer) { - const [, placeholder] = query.match(placeholderPattern) ?? [] - const start = - node.callee.type === 'MemberExpression' - ? node.callee.property.range![0] - : node.range![0] - const end = node.range![1] return fixer.replaceTextRange( - [start, end], - `getByPlaceholder("${placeholder}")`, + rangeToReplace, + `getByPlaceholder("${placeholderMatch[1]}")`, ) }, messageId: 'unexpectedPlaceholderQuery', @@ -72,18 +65,13 @@ export default createRule({ } const altTextPattern = /^\[alt=['"](.+?)['"]\]$/ - if (query.match(altTextPattern)) { + const altTextMatch = query.match(altTextPattern) + if (altTextMatch) { context.report({ fix(fixer) { - const [, alt] = query.match(altTextPattern) ?? [] - const start = - node.callee.type === 'MemberExpression' - ? node.callee.property.range![0] - : node.range![0] - const end = node.range![1] return fixer.replaceTextRange( - [start, end], - `getByAltText("${alt}")`, + rangeToReplace, + `getByAltText("${altTextMatch[1]}")`, ) }, messageId: 'unexpectedAltTextQuery', @@ -92,18 +80,13 @@ export default createRule({ } const titlePattern = /^\[title=['"](.+?)['"]\]$/ - if (query.match(titlePattern)) { + const titleMatch = query.match(titlePattern) + if (titleMatch) { context.report({ fix(fixer) { - const [, title] = query.match(titlePattern) ?? [] - const start = - node.callee.type === 'MemberExpression' - ? node.callee.property.range![0] - : node.range![0] - const end = node.range![1] return fixer.replaceTextRange( - [start, end], - `getByTitle("${title}")`, + rangeToReplace, + `getByTitle("${titleMatch[1]}")`, ) }, messageId: 'unexpectedTitleQuery', @@ -116,18 +99,13 @@ export default createRule({ const testIdPattern = new RegExp( `^\\[${testIdAttributeName}=['"](.+?)['"]\\]`, ) - if (query.match(testIdPattern)) { + const testIdMatch = query.match(testIdPattern) + if (testIdMatch) { context.report({ fix(fixer) { - const [, testId] = query.match(testIdPattern) ?? [] - const start = - node.callee.type === 'MemberExpression' - ? node.callee.property.range![0] - : node.range![0] - const end = node.range![1] return fixer.replaceTextRange( - [start, end], - `getByTestId("${testId}")`, + rangeToReplace, + `getByTestId("${testIdMatch[1]}")`, ) }, messageId: 'unexpectedTestIdQuery', From 558f03d99052ebd328c5bfac73d4a272bf041440 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 22:10:34 -0400 Subject: [PATCH 08/26] Put new text in its own variable --- src/rules/prefer-native-locators.ts | 36 ++++++++++------------------- 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index a327817..d9cf244 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -24,10 +24,8 @@ export default createRule({ if (labelMatch) { context.report({ fix(fixer) { - return fixer.replaceTextRange( - rangeToReplace, - `getByLabel("${labelMatch[1]}")`, - ) + const newText = `getByLabel("${labelMatch[1]}")` + return fixer.replaceTextRange(rangeToReplace, newText) }, messageId: 'unexpectedLabelQuery', node, @@ -39,10 +37,8 @@ export default createRule({ if (roleMatch) { context.report({ fix(fixer) { - return fixer.replaceTextRange( - rangeToReplace, - `getByRole("${roleMatch[1]}")`, - ) + const newText = `getByRole("${roleMatch[1]}")` + return fixer.replaceTextRange(rangeToReplace, newText) }, messageId: 'unexpectedRoleQuery', node, @@ -54,10 +50,8 @@ export default createRule({ if (placeholderMatch) { context.report({ fix(fixer) { - return fixer.replaceTextRange( - rangeToReplace, - `getByPlaceholder("${placeholderMatch[1]}")`, - ) + const newText = `getByPlaceholder("${placeholderMatch[1]}")` + return fixer.replaceTextRange(rangeToReplace, newText) }, messageId: 'unexpectedPlaceholderQuery', node, @@ -69,10 +63,8 @@ export default createRule({ if (altTextMatch) { context.report({ fix(fixer) { - return fixer.replaceTextRange( - rangeToReplace, - `getByAltText("${altTextMatch[1]}")`, - ) + const newText = `getByAltText("${altTextMatch[1]}")` + return fixer.replaceTextRange(rangeToReplace, newText) }, messageId: 'unexpectedAltTextQuery', node, @@ -84,10 +76,8 @@ export default createRule({ if (titleMatch) { context.report({ fix(fixer) { - return fixer.replaceTextRange( - rangeToReplace, - `getByTitle("${titleMatch[1]}")`, - ) + const newText = `getByTitle("${titleMatch[1]}")` + return fixer.replaceTextRange(rangeToReplace, newText) }, messageId: 'unexpectedTitleQuery', node, @@ -103,10 +93,8 @@ export default createRule({ if (testIdMatch) { context.report({ fix(fixer) { - return fixer.replaceTextRange( - rangeToReplace, - `getByTestId("${testIdMatch[1]}")`, - ) + const newText = `getByTestId("${testIdMatch[1]}")` + return fixer.replaceTextRange(rangeToReplace, newText) }, messageId: 'unexpectedTestIdQuery', node, From 19c486ee4b1447e432e3ee0785ac21b8c24375ca Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 22:23:16 -0400 Subject: [PATCH 09/26] Add support for custom test ID attribute --- src/rules/prefer-native-locators.test.ts | 6 ++++++ src/rules/prefer-native-locators.ts | 22 ++++++++++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index 5bd15a5..3641fdc 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -33,6 +33,12 @@ runRuleTester('prefer-native-locators', rule, { 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")', + }, ], valid: [ { code: 'page.getByLabel("View more")' }, diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index d9cf244..5d0a3fb 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -5,6 +5,11 @@ export default createRule({ create(context) { return { CallExpression(node) { + const { testIdAttribute } = { + testIdAttribute: 'data-testid', + ...((context.options?.[0] as Record) ?? {}), + } + if (node.callee.type !== 'MemberExpression') return const method = getStringValue(node.callee.property) const query = getStringValue(node.arguments[0]) @@ -84,10 +89,8 @@ export default createRule({ }) } - // TODO: Add support for custom test ID attribute - const testIdAttributeName = 'data-testid' const testIdPattern = new RegExp( - `^\\[${testIdAttributeName}=['"](.+?)['"]\\]`, + `^\\[${testIdAttribute}=['"](.+?)['"]\\]`, ) const testIdMatch = query.match(testIdPattern) if (testIdMatch) { @@ -119,7 +122,18 @@ export default createRule({ unexpectedTestIdQuery: 'Use .getByTestId() instead', unexpectedTitleQuery: 'Use .getByTitle() instead', }, - schema: [], + schema: [ + { + additionalProperties: false, + properties: { + testIdAttribute: { + default: 'data-testid', + type: 'string', + }, + }, + type: 'object', + }, + ], type: 'suggestion', }, }) From 6a5d92ac6206a73441e2753affa0056c64c9b6f0 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 22:42:42 -0400 Subject: [PATCH 10/26] Add more tests --- src/rules/prefer-native-locators.test.ts | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index 3641fdc..54df905 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -39,6 +39,35 @@ runRuleTester('prefer-native-locators', rule, { 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")' }, From 8e80331683c280fc1c5823d2029d47b662c5a18a Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 22:51:41 -0400 Subject: [PATCH 11/26] Add docs --- docs/rules/prefer-native-locators.md | 34 ++++++++++++++++++++++++ src/rules/prefer-native-locators.test.ts | 2 +- 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 docs/rules/prefer-native-locators.md diff --git a/docs/rules/prefer-native-locators.md b/docs/rules/prefer-native-locators.md new file mode 100644 index 0000000..13a31b0 --- /dev/null +++ b/docs/rules/prefer-native-locators.md @@ -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') +``` diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index 54df905..8d5946a 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -71,7 +71,7 @@ runRuleTester('prefer-native-locators', rule, { ], valid: [ { code: 'page.getByLabel("View more")' }, - { code: 'page.getByRole("Button")' }, + { code: 'page.getByRole("button")' }, { code: 'page.getByPlaceholder("Enter some text...")' }, { code: 'page.getByAltText("Playwright logo")' }, { code: 'page.getByTestId("password-input")' }, From 7524dc69f48e5d01f73a7d3b39f664d7e05008f0 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 22:57:10 -0400 Subject: [PATCH 12/26] Add prefer-native-locators to README --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b4c0e02..3a3fe9d 100644 --- a/README.md +++ b/README.md @@ -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 +[`--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) @@ -194,6 +195,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()` | | 🔧 | | From 7eaf78741ed7db42d74b72b9a761dad67c598be7 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sun, 18 Aug 2024 22:58:24 -0400 Subject: [PATCH 13/26] Export prefer-native-locators rule --- src/index.ts | 2 ++ 1 file changed, 2 insertions(+) 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, From 61fbebb476281c163e1c1a81b2a258259bdc3f9c Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Thu, 29 Aug 2024 18:48:06 -0400 Subject: [PATCH 14/26] Move patterns to array --- src/rules/prefer-native-locators.ts | 139 +++++++++++----------------- 1 file changed, 56 insertions(+), 83 deletions(-) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index 5d0a3fb..d743522 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -1,15 +1,54 @@ import { getStringValue, isPageMethod } from '../utils/ast' import { createRule } from '../utils/createRule' +type Pattern = { + messageId: string + pattern: RegExp + replacement: string +} + export default createRule({ create(context) { + const { testIdAttribute } = { + testIdAttribute: 'data-testid', + ...((context.options?.[0] as Record) ?? {}), + } + + const patterns: Array = [ + { + messageId: 'unexpectedLabelQuery', + pattern: /^\[aria-label=['"](.+?)['"]\]$/, + replacement: 'getByLabel', + }, + { + messageId: 'unexpectedRoleQuery', + pattern: /^\[role=['"](.+?)['"]\]$/, + replacement: 'getByRole', + }, + { + messageId: 'unexpectedPlaceholderQuery', + pattern: /^\[placeholder=['"](.+?)['"]\]$/, + replacement: 'getByPlaceholder', + }, + { + messageId: 'unexpectedAltTextQuery', + pattern: /^\[alt=['"](.+?)['"]\]$/, + replacement: 'getByAltText', + }, + { + messageId: 'unexpectedTitleQuery', + pattern: /^\[title=['"](.+?)['"]\]$/, + replacement: 'getByTitle', + }, + { + messageId: 'unexpectedTestIdQuery', + pattern: new RegExp(`^\\[${testIdAttribute}=['"](.+?)['"]\\]`), + replacement: 'getByTestId', + }, + ] + return { CallExpression(node) { - const { testIdAttribute } = { - testIdAttribute: 'data-testid', - ...((context.options?.[0] as Record) ?? {}), - } - if (node.callee.type !== 'MemberExpression') return const method = getStringValue(node.callee.property) const query = getStringValue(node.arguments[0]) @@ -24,84 +63,18 @@ export default createRule({ const end = node.range![1] const rangeToReplace: [number, number] = [start, end] - const ariaLabelPattern = /^\[aria-label=['"](.+?)['"]\]$/ - 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, - }) + for (const pattern of patterns) { + const match = query.match(pattern.pattern) + if (match) { + context.report({ + fix(fixer) { + const newText = `${pattern.replacement}("${match[1]}")` + return fixer.replaceTextRange(rangeToReplace, newText) + }, + messageId: pattern.messageId, + node, + }) + } } }, } From c3b834db2654ddc594545ce9a28085ee0d44f344 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Thu, 29 Aug 2024 18:48:42 -0400 Subject: [PATCH 15/26] Drop periods from messages --- src/rules/prefer-native-locators.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index d743522..7e54ab7 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -88,12 +88,12 @@ export default createRule({ }, 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', + 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: [ { From a93d0ebde3b65056ff4da0eb5aa85c7336707d84 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Thu, 29 Aug 2024 18:52:24 -0400 Subject: [PATCH 16/26] Remove non-page locator method check --- src/rules/prefer-native-locators.test.ts | 6 ------ src/rules/prefer-native-locators.ts | 3 +-- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index 8d5946a..d9e2541 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -62,12 +62,6 @@ runRuleTester('prefer-native-locators', rule, { 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")' }, diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index 7e54ab7..fb88df0 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -50,9 +50,8 @@ export default createRule({ return { CallExpression(node) { if (node.callee.type !== 'MemberExpression') return - const method = getStringValue(node.callee.property) const query = getStringValue(node.arguments[0]) - const isLocator = isPageMethod(node, 'locator') || method === 'locator' + const isLocator = isPageMethod(node, 'locator') if (!isLocator) return // If it's something like `page.locator`, just replace the `.locator` part From 4c7ecaf10fd06cd92d3d0d25bb29c179648c7fc4 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Thu, 29 Aug 2024 18:54:57 -0400 Subject: [PATCH 17/26] Add tests for empty string + no args --- src/rules/prefer-native-locators.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index d9e2541..b292462 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -70,6 +70,7 @@ runRuleTester('prefer-native-locators', rule, { { code: 'page.getByAltText("Playwright logo")' }, { code: 'page.getByTestId("password-input")' }, { code: 'page.getByTitle("Additional context")' }, + // Does not match on more complex queries { code: `page.locator('[complex-query] > [aria-label="View more"]')`, }, @@ -88,5 +89,8 @@ runRuleTester('prefer-native-locators', rule, { { code: `page.locator('[complex-query] > [title="Additional context"]')`, }, + // Works for empty string and no arguments + { code: `page.locator('')` }, + { code: `page.locator()` }, ], }) From 12798a14212a70b7c0aefe9ebb53370c81d6804c Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Thu, 29 Aug 2024 18:57:38 -0400 Subject: [PATCH 18/26] Use AST.Range instead of tuple --- src/rules/prefer-native-locators.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index fb88df0..5a24c36 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -1,3 +1,4 @@ +import { AST } from 'eslint' import { getStringValue, isPageMethod } from '../utils/ast' import { createRule } from '../utils/createRule' @@ -60,7 +61,7 @@ export default createRule({ ? node.callee.property.range![0] : node.range![0] const end = node.range![1] - const rangeToReplace: [number, number] = [start, end] + const rangeToReplace: AST.Range = [start, end] for (const pattern of patterns) { const match = query.match(pattern.pattern) From a77f3a3a6007b2469523970a9fcaa922a76998dc Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Thu, 29 Aug 2024 19:00:05 -0400 Subject: [PATCH 19/26] Add more tests --- src/rules/prefer-native-locators.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index b292462..73d3677 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -70,6 +70,8 @@ runRuleTester('prefer-native-locators', rule, { { 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"]')`, From 62f9a89c6054e1af48571c1216456b671a422abb Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Thu, 29 Aug 2024 19:10:42 -0400 Subject: [PATCH 20/26] Allow replacing for selectors without quotes --- src/rules/prefer-native-locators.test.ts | 36 ++++++++++++++++++++++++ src/rules/prefer-native-locators.ts | 12 ++++---- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index 73d3677..ab9c7c0 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -8,37 +8,73 @@ runRuleTester('prefer-native-locators', rule, { 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()`, diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index 5a24c36..7c69e5f 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -18,32 +18,32 @@ export default createRule({ const patterns: Array = [ { messageId: 'unexpectedLabelQuery', - pattern: /^\[aria-label=['"](.+?)['"]\]$/, + pattern: /^\[aria-label=['"]?(.+?)['"]?\]$/, replacement: 'getByLabel', }, { messageId: 'unexpectedRoleQuery', - pattern: /^\[role=['"](.+?)['"]\]$/, + pattern: /^\[role=['"]?(.+?)['"]?\]$/, replacement: 'getByRole', }, { messageId: 'unexpectedPlaceholderQuery', - pattern: /^\[placeholder=['"](.+?)['"]\]$/, + pattern: /^\[placeholder=['"]?(.+?)['"]?\]$/, replacement: 'getByPlaceholder', }, { messageId: 'unexpectedAltTextQuery', - pattern: /^\[alt=['"](.+?)['"]\]$/, + pattern: /^\[alt=['"]?(.+?)['"]?\]$/, replacement: 'getByAltText', }, { messageId: 'unexpectedTitleQuery', - pattern: /^\[title=['"](.+?)['"]\]$/, + pattern: /^\[title=['"]?(.+?)['"]?\]$/, replacement: 'getByTitle', }, { messageId: 'unexpectedTestIdQuery', - pattern: new RegExp(`^\\[${testIdAttribute}=['"](.+?)['"]\\]`), + pattern: new RegExp(`^\\[${testIdAttribute}=['"]?(.+?)['"]?\\]`), replacement: 'getByTestId', }, ] From a65e8e41d8f64e18dd5484571f2fb5cc16c4f055 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Thu, 29 Aug 2024 19:25:40 -0400 Subject: [PATCH 21/26] Add docs on testIdAttribute --- docs/rules/prefer-native-locators.md | 36 ++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/docs/rules/prefer-native-locators.md b/docs/rules/prefer-native-locators.md index 13a31b0..22aadfa 100644 --- a/docs/rules/prefer-native-locators.md +++ b/docs/rules/prefer-native-locators.md @@ -32,3 +32,39 @@ 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') +``` From 7ee29d827be2f0724b6eb6e1eb00f590a82a5a9a Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Tue, 3 Sep 2024 00:57:54 -0400 Subject: [PATCH 22/26] Undo line-break --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 3a3fe9d..73d387a 100644 --- a/README.md +++ b/README.md @@ -155,8 +155,7 @@ 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 [`--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) From 0626abeaec95a863c6c3734d7374b9fafb058fac Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Tue, 3 Sep 2024 01:06:45 -0400 Subject: [PATCH 23/26] Use same RegExp for each attribute --- src/rules/prefer-native-locators.ts | 77 +++++++++++++++++------------ 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index 7c69e5f..4023f17 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -8,6 +8,49 @@ type Pattern = { 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 } = { @@ -15,38 +58,7 @@ export default createRule({ ...((context.options?.[0] as Record) ?? {}), } - const patterns: Array = [ - { - messageId: 'unexpectedLabelQuery', - pattern: /^\[aria-label=['"]?(.+?)['"]?\]$/, - replacement: 'getByLabel', - }, - { - messageId: 'unexpectedRoleQuery', - pattern: /^\[role=['"]?(.+?)['"]?\]$/, - replacement: 'getByRole', - }, - { - messageId: 'unexpectedPlaceholderQuery', - pattern: /^\[placeholder=['"]?(.+?)['"]?\]$/, - replacement: 'getByPlaceholder', - }, - { - messageId: 'unexpectedAltTextQuery', - pattern: /^\[alt=['"]?(.+?)['"]?\]$/, - replacement: 'getByAltText', - }, - { - messageId: 'unexpectedTitleQuery', - pattern: /^\[title=['"]?(.+?)['"]?\]$/, - replacement: 'getByTitle', - }, - { - messageId: 'unexpectedTestIdQuery', - pattern: new RegExp(`^\\[${testIdAttribute}=['"]?(.+?)['"]?\\]`), - replacement: 'getByTestId', - }, - ] + const patterns = compilePatterns({ testIdAttribute }) return { CallExpression(node) { @@ -74,6 +86,7 @@ export default createRule({ messageId: pattern.messageId, node, }) + return } } }, From 0be9b194abd80a94b5f6d22f573f361615f706c5 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Tue, 3 Sep 2024 01:10:15 -0400 Subject: [PATCH 24/26] Move range into fixer --- src/rules/prefer-native-locators.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index 4023f17..9b2f1e5 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -67,19 +67,18 @@ export default createRule({ const isLocator = isPageMethod(node, 'locator') 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: AST.Range = [start, end] - 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) }, From 06ec0f6d83f1bef0192bfcf126dd3e2d3ec232d6 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Tue, 3 Sep 2024 01:11:08 -0400 Subject: [PATCH 25/26] Remove unnecessary identifier --- src/rules/prefer-native-locators.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/rules/prefer-native-locators.ts b/src/rules/prefer-native-locators.ts index 9b2f1e5..25a38e5 100644 --- a/src/rules/prefer-native-locators.ts +++ b/src/rules/prefer-native-locators.ts @@ -64,8 +64,7 @@ export default createRule({ CallExpression(node) { if (node.callee.type !== 'MemberExpression') return const query = getStringValue(node.arguments[0]) - const isLocator = isPageMethod(node, 'locator') - if (!isLocator) return + if (!isPageMethod(node, 'locator')) return for (const pattern of patterns) { const match = query.match(pattern.pattern) From ccd2fc5dd76cf0c3db5a882e387a535f409c9988 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Tue, 3 Sep 2024 10:19:05 -0400 Subject: [PATCH 26/26] Add more test cases --- src/rules/prefer-native-locators.test.ts | 60 ++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/rules/prefer-native-locators.test.ts b/src/rules/prefer-native-locators.test.ts index ab9c7c0..d795fd3 100644 --- a/src/rules/prefer-native-locators.test.ts +++ b/src/rules/prefer-native-locators.test.ts @@ -98,16 +98,58 @@ runRuleTester('prefer-native-locators', rule, { 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"]')`, @@ -127,8 +169,26 @@ runRuleTester('prefer-native-locators', rule, { { 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");` }, ], })