From 78000e7b671217771cbf50279c0d4bcd25eb6761 Mon Sep 17 00:00:00 2001 From: Carl Bray Date: Thu, 12 Sep 2024 14:21:11 +1200 Subject: [PATCH 1/9] Add prefer-page-locator-fill --- README.md | 1 + docs/rules/prefer-page-locator-fill.md | 21 ++++++++++++ src/index.ts | 2 ++ src/rules/prefer-page-locator-fill.test.ts | 33 +++++++++++++++++++ src/rules/prefer-page-locator-fill.ts | 37 ++++++++++++++++++++++ 5 files changed, 94 insertions(+) create mode 100644 docs/rules/prefer-page-locator-fill.md create mode 100644 src/rules/prefer-page-locator-fill.test.ts create mode 100644 src/rules/prefer-page-locator-fill.ts diff --git a/README.md b/README.md index 73d387a..7884068 100644 --- a/README.md +++ b/README.md @@ -195,6 +195,7 @@ CLI option\ | [prefer-hooks-on-top](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | | | [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | 🔧 | | | [prefer-native-locators](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-native-locators.md) | Suggest built-in locators over `page.locator()` | | 🔧 | | +| [prefer-page-locator-fill](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-page-locator-fill.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-page-locator-fill.md b/docs/rules/prefer-page-locator-fill.md new file mode 100644 index 0000000..69919c2 --- /dev/null +++ b/docs/rules/prefer-page-locator-fill.md @@ -0,0 +1,21 @@ +# Suggest using `page.locator()` (`prefer-page-locator-fill`) + +Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill) +instead of page.fill(). + +## Rule details + +This rule triggers a warning if `page.fill()` is used. + +The following patterns are considered warnings: + +```javascript +await page.fill("input[type=\"password\"]", "password"); +``` + +The following pattern is **not** a warning: + +```javascript +await page.getByRole("password").fill("password"); +await page.locator("input[type=\"password\"]").fill("password"); +``` diff --git a/src/index.ts b/src/index.ts index c185547..0031cda 100644 --- a/src/index.ts +++ b/src/index.ts @@ -32,6 +32,7 @@ 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 preferPageLocatorFill from './rules/prefer-page-locator-fill' import preferStrictEqual from './rules/prefer-strict-equal' import preferToBe from './rules/prefer-to-be' import preferToContain from './rules/prefer-to-contain' @@ -83,6 +84,7 @@ const index = { 'prefer-hooks-on-top': preferHooksOnTop, 'prefer-lowercase-title': preferLowercaseTitle, 'prefer-native-locators': preferNativeLocators, + 'prefer-page-locator-fill': preferPageLocatorFill, 'prefer-strict-equal': preferStrictEqual, 'prefer-to-be': preferToBe, 'prefer-to-contain': preferToContain, diff --git a/src/rules/prefer-page-locator-fill.test.ts b/src/rules/prefer-page-locator-fill.test.ts new file mode 100644 index 0000000..12de917 --- /dev/null +++ b/src/rules/prefer-page-locator-fill.test.ts @@ -0,0 +1,33 @@ +import { runRuleTester } from '../utils/rule-tester' +import rule from './prefer-page-locator-fill' + +runRuleTester('prefer-page-locator-fill', rule, { + invalid: [ + { + code: ` + async function test() { + await page.fill(); + } + `, + errors: [ + { + column: 15, + endColumn: 32, + endLine: 3, + line: 3, + messageId: 'avoidAwaitPageFill' + } + ], + output: null + } + ], + valid: [ + { + code: ` + async function test() { + await page.locator(); + } + ` + } + ], +}) diff --git a/src/rules/prefer-page-locator-fill.ts b/src/rules/prefer-page-locator-fill.ts new file mode 100644 index 0000000..b953557 --- /dev/null +++ b/src/rules/prefer-page-locator-fill.ts @@ -0,0 +1,37 @@ +import { isPageMethod } from '../utils/ast' +import { createRule } from '../utils/createRule' + +export default createRule({ + create(context) { + return { + AwaitExpression(node) { + // Must be a call expression + if (node.argument.type !== 'CallExpression') return; + + // Must be a page.fill() call + const { callee } = node.argument + if (callee.type !== 'MemberExpression') return; + + if (isPageMethod(node.argument, 'fill')) { + context.report({ + messageId: "avoidAwaitPageFill", + node + }); + } + } + } + }, + meta: { + docs: { + category: 'Best Practices', + description: "Discourage using 'await page.fill()'", + recommended: false, + url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-page-locator-fill.md', + }, + messages: { + avoidAwaitPageFill: "Avoid using 'await page.fill()', Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill)", + }, + schema: [], + type: 'suggestion' + } +}) From 3b380c74d6c8a7d0c4fadf4eb49aec245bb068c8 Mon Sep 17 00:00:00 2001 From: Carl Bray Date: Sat, 14 Sep 2024 08:41:35 +1200 Subject: [PATCH 2/9] Rename rule --- README.md | 2 +- .../{prefer-page-locator-fill.md => prefer-page-locator.md} | 4 ++-- src/index.ts | 4 ++-- ...refer-page-locator-fill.test.ts => prefer-locator.test.ts} | 4 ++-- src/rules/{prefer-page-locator-fill.ts => prefer-locator.ts} | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) rename docs/rules/{prefer-page-locator-fill.md => prefer-page-locator.md} (85%) rename src/rules/{prefer-page-locator-fill.test.ts => prefer-locator.test.ts} (84%) rename src/rules/{prefer-page-locator-fill.ts => prefer-locator.ts} (89%) diff --git a/README.md b/README.md index 7884068..a8815ff 100644 --- a/README.md +++ b/README.md @@ -195,7 +195,7 @@ CLI option\ | [prefer-hooks-on-top](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | | | [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | 🔧 | | | [prefer-native-locators](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-native-locators.md) | Suggest built-in locators over `page.locator()` | | 🔧 | | -| [prefer-page-locator-fill](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-page-locator-fill.md) | Suggest built-in locators over `page.locator()` | | | | +| [prefer-locator](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-locator.md) | Suggest built-in locators over page methods | | | | | [prefer-strict-equal](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | | 💡 | | [prefer-to-be](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-be.md) | Suggest using `toBe()` | | 🔧 | | | [prefer-to-contain](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | | 🔧 | | diff --git a/docs/rules/prefer-page-locator-fill.md b/docs/rules/prefer-page-locator.md similarity index 85% rename from docs/rules/prefer-page-locator-fill.md rename to docs/rules/prefer-page-locator.md index 69919c2..a0e1a36 100644 --- a/docs/rules/prefer-page-locator-fill.md +++ b/docs/rules/prefer-page-locator.md @@ -1,7 +1,7 @@ -# Suggest using `page.locator()` (`prefer-page-locator-fill`) +# Suggest using `page.locator()` (`prefer-page-locator`) Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill) -instead of page.fill(). +instead of page methods. ## Rule details diff --git a/src/index.ts b/src/index.ts index 0031cda..4e4cdac 100644 --- a/src/index.ts +++ b/src/index.ts @@ -30,9 +30,9 @@ import preferComparisonMatcher from './rules/prefer-comparison-matcher' import preferEqualityMatcher from './rules/prefer-equality-matcher' import preferHooksInOrder from './rules/prefer-hooks-in-order' import preferHooksOnTop from './rules/prefer-hooks-on-top' +import preferPageLocator from './rules/prefer-locator' import preferLowercaseTitle from './rules/prefer-lowercase-title' import preferNativeLocators from './rules/prefer-native-locators' -import preferPageLocatorFill from './rules/prefer-page-locator-fill' import preferStrictEqual from './rules/prefer-strict-equal' import preferToBe from './rules/prefer-to-be' import preferToContain from './rules/prefer-to-contain' @@ -82,9 +82,9 @@ const index = { 'prefer-equality-matcher': preferEqualityMatcher, 'prefer-hooks-in-order': preferHooksInOrder, 'prefer-hooks-on-top': preferHooksOnTop, + 'prefer-locator': preferPageLocator, 'prefer-lowercase-title': preferLowercaseTitle, 'prefer-native-locators': preferNativeLocators, - 'prefer-page-locator-fill': preferPageLocatorFill, 'prefer-strict-equal': preferStrictEqual, 'prefer-to-be': preferToBe, 'prefer-to-contain': preferToContain, diff --git a/src/rules/prefer-page-locator-fill.test.ts b/src/rules/prefer-locator.test.ts similarity index 84% rename from src/rules/prefer-page-locator-fill.test.ts rename to src/rules/prefer-locator.test.ts index 12de917..705d2ed 100644 --- a/src/rules/prefer-page-locator-fill.test.ts +++ b/src/rules/prefer-locator.test.ts @@ -1,7 +1,7 @@ import { runRuleTester } from '../utils/rule-tester' -import rule from './prefer-page-locator-fill' +import rule from './prefer-locator' -runRuleTester('prefer-page-locator-fill', rule, { +runRuleTester('prefer-locator', rule, { invalid: [ { code: ` diff --git a/src/rules/prefer-page-locator-fill.ts b/src/rules/prefer-locator.ts similarity index 89% rename from src/rules/prefer-page-locator-fill.ts rename to src/rules/prefer-locator.ts index b953557..b1de149 100644 --- a/src/rules/prefer-page-locator-fill.ts +++ b/src/rules/prefer-locator.ts @@ -24,9 +24,9 @@ export default createRule({ meta: { docs: { category: 'Best Practices', - description: "Discourage using 'await page.fill()'", + description: "Discourage using await page methods", recommended: false, - url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-page-locator-fill.md', + url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-locator.md', }, messages: { avoidAwaitPageFill: "Avoid using 'await page.fill()', Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill)", From 642553982101061e01670add12d222fe5cd06892 Mon Sep 17 00:00:00 2001 From: Carl Bray Date: Sat, 14 Sep 2024 09:04:04 +1200 Subject: [PATCH 3/9] Add set of methods to check against --- src/rules/prefer-locator.test.ts | 2 +- src/rules/prefer-locator.ts | 55 +++++++++++++++++++++++++------- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/rules/prefer-locator.test.ts b/src/rules/prefer-locator.test.ts index 705d2ed..6a8e2a9 100644 --- a/src/rules/prefer-locator.test.ts +++ b/src/rules/prefer-locator.test.ts @@ -15,7 +15,7 @@ runRuleTester('prefer-locator', rule, { endColumn: 32, endLine: 3, line: 3, - messageId: 'avoidAwaitPageFill' + messageId: 'avoidAwaitPageMethods' } ], output: null diff --git a/src/rules/prefer-locator.ts b/src/rules/prefer-locator.ts index b1de149..674a8d0 100644 --- a/src/rules/prefer-locator.ts +++ b/src/rules/prefer-locator.ts @@ -1,6 +1,42 @@ -import { isPageMethod } from '../utils/ast' +import ESTree from 'estree' +import { getStringValue, isPageMethod } from '../utils/ast' import { createRule } from '../utils/createRule' +const pageMethods = new Set([ + 'click', + 'dblclick', + 'dispatchEvent', + 'fill', + 'focus', + 'getAttribute', + 'hover', + 'innerHTML', + 'innerText', + 'inputValue', + 'isChecked', + 'isDisabled', + 'isEditable', + 'isEnabled', + 'isHidden', + 'isVisible', + 'press', + 'selectOption', + 'setChecked', + 'setInputFiles', + 'tap', + 'textContent', + 'uncheck' +]); + +function isSupportedMethod(node: ESTree.CallExpression) { + if (node.callee.type !== 'MemberExpression') return false + + const name = getStringValue(node.callee.property) + return ( + pageMethods.has(name) && isPageMethod(node, name) + ) +} + export default createRule({ create(context) { return { @@ -8,16 +44,13 @@ export default createRule({ // Must be a call expression if (node.argument.type !== 'CallExpression') return; - // Must be a page.fill() call - const { callee } = node.argument - if (callee.type !== 'MemberExpression') return; + // Must be a method we care about + if (!isSupportedMethod(node.argument)) return - if (isPageMethod(node.argument, 'fill')) { - context.report({ - messageId: "avoidAwaitPageFill", - node - }); - } + context.report({ + messageId: "avoidAwaitPageMethods", + node + }); } } }, @@ -29,7 +62,7 @@ export default createRule({ url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-locator.md', }, messages: { - avoidAwaitPageFill: "Avoid using 'await page.fill()', Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill)", + avoidAwaitPageMethods: "Avoid using page methods e.g. 'await page.fill()', Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill)", }, schema: [], type: 'suggestion' From a87969094f4200209a5e44215d46a3faeef487b2 Mon Sep 17 00:00:00 2001 From: Carl Bray Date: Sat, 14 Sep 2024 09:08:31 +1200 Subject: [PATCH 4/9] Update docs too --- docs/rules/{prefer-page-locator.md => prefer-locator.md} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename docs/rules/{prefer-page-locator.md => prefer-locator.md} (52%) diff --git a/docs/rules/prefer-page-locator.md b/docs/rules/prefer-locator.md similarity index 52% rename from docs/rules/prefer-page-locator.md rename to docs/rules/prefer-locator.md index a0e1a36..deed92e 100644 --- a/docs/rules/prefer-page-locator.md +++ b/docs/rules/prefer-locator.md @@ -1,11 +1,11 @@ -# Suggest using `page.locator()` (`prefer-page-locator`) +# Suggest using `page.locator()` (`prefer-locator`) -Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill) -instead of page methods. +Instead of using page methods use locator-based e.g. page.fill() use +[locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill) ## Rule details -This rule triggers a warning if `page.fill()` is used. +This rule triggers a warning if page methods are used, instead of locators. The following patterns are considered warnings: From 3a5ca3f3fb4940fcdcc524d4c5b85a44d60fcbfd Mon Sep 17 00:00:00 2001 From: Carl Bray Date: Sat, 14 Sep 2024 10:01:57 +1200 Subject: [PATCH 5/9] Fix formatting --- README.md | 2 +- docs/rules/prefer-locator.md | 6 +++--- src/rules/prefer-locator.test.ts | 22 +++++++++++----------- src/rules/prefer-locator.ts | 27 +++++++++++++-------------- 4 files changed, 28 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index a8815ff..cf6642b 100644 --- a/README.md +++ b/README.md @@ -195,7 +195,7 @@ CLI option\ | [prefer-hooks-on-top](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | | | [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | 🔧 | | | [prefer-native-locators](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-native-locators.md) | Suggest built-in locators over `page.locator()` | | 🔧 | | -| [prefer-locator](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-locator.md) | Suggest built-in locators over page methods | | | | +| [prefer-locator](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-locator.md) | Suggest built-in locators over page methods | | | | | [prefer-strict-equal](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | | 💡 | | [prefer-to-be](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-be.md) | Suggest using `toBe()` | | 🔧 | | | [prefer-to-contain](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | | 🔧 | | diff --git a/docs/rules/prefer-locator.md b/docs/rules/prefer-locator.md index deed92e..0695c63 100644 --- a/docs/rules/prefer-locator.md +++ b/docs/rules/prefer-locator.md @@ -10,12 +10,12 @@ This rule triggers a warning if page methods are used, instead of locators. The following patterns are considered warnings: ```javascript -await page.fill("input[type=\"password\"]", "password"); +await page.fill('input[type="password"]', 'password') ``` The following pattern is **not** a warning: ```javascript -await page.getByRole("password").fill("password"); -await page.locator("input[type=\"password\"]").fill("password"); +await page.getByRole('password').fill('password') +await page.locator('input[type="password"]').fill('password') ``` diff --git a/src/rules/prefer-locator.test.ts b/src/rules/prefer-locator.test.ts index 6a8e2a9..9e38beb 100644 --- a/src/rules/prefer-locator.test.ts +++ b/src/rules/prefer-locator.test.ts @@ -10,16 +10,16 @@ runRuleTester('prefer-locator', rule, { } `, errors: [ - { - column: 15, - endColumn: 32, - endLine: 3, - line: 3, - messageId: 'avoidAwaitPageMethods' - } + { + column: 15, + endColumn: 32, + endLine: 3, + line: 3, + messageId: 'avoidAwaitPageMethods', + }, ], - output: null - } + output: null, + }, ], valid: [ { @@ -27,7 +27,7 @@ runRuleTester('prefer-locator', rule, { async function test() { await page.locator(); } - ` - } + `, + }, ], }) diff --git a/src/rules/prefer-locator.ts b/src/rules/prefer-locator.ts index 674a8d0..bdac874 100644 --- a/src/rules/prefer-locator.ts +++ b/src/rules/prefer-locator.ts @@ -25,16 +25,14 @@ const pageMethods = new Set([ 'setInputFiles', 'tap', 'textContent', - 'uncheck' -]); + 'uncheck', +]) function isSupportedMethod(node: ESTree.CallExpression) { if (node.callee.type !== 'MemberExpression') return false const name = getStringValue(node.callee.property) - return ( - pageMethods.has(name) && isPageMethod(node, name) - ) + return pageMethods.has(name) && isPageMethod(node, name) } export default createRule({ @@ -42,29 +40,30 @@ export default createRule({ return { AwaitExpression(node) { // Must be a call expression - if (node.argument.type !== 'CallExpression') return; + if (node.argument.type !== 'CallExpression') return // Must be a method we care about if (!isSupportedMethod(node.argument)) return context.report({ - messageId: "avoidAwaitPageMethods", - node - }); - } + messageId: 'avoidAwaitPageMethods', + node, + }) + }, } }, meta: { docs: { category: 'Best Practices', - description: "Discourage using await page methods", + description: 'Discourage using await page methods', recommended: false, url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-locator.md', }, messages: { - avoidAwaitPageMethods: "Avoid using page methods e.g. 'await page.fill()', Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill)", + avoidAwaitPageMethods: + "Avoid using page methods e.g. 'await page.fill()', Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill)", }, schema: [], - type: 'suggestion' - } + type: 'suggestion', + }, }) From b66f87d21c9c8a18e1fd8dde6316ef738a99b684 Mon Sep 17 00:00:00 2001 From: Carl Bray Date: Thu, 19 Sep 2024 16:39:20 +1200 Subject: [PATCH 6/9] Change descriptons, add more tests and examples --- README.md | 2 +- docs/rules/prefer-locator.md | 16 ++++++-- src/rules/prefer-locator.test.ts | 69 ++++++++++++++++++++++++++++++-- src/rules/prefer-locator.ts | 7 ++-- 4 files changed, 82 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index cf6642b..b86d0a3 100644 --- a/README.md +++ b/README.md @@ -195,7 +195,7 @@ CLI option\ | [prefer-hooks-on-top](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | | | [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | 🔧 | | | [prefer-native-locators](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-native-locators.md) | Suggest built-in locators over `page.locator()` | | 🔧 | | -| [prefer-locator](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-locator.md) | Suggest built-in locators over page methods | | | | +| [prefer-locator](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-locator.md) | Suggest locators over page methods | | | | | [prefer-strict-equal](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | | 💡 | | [prefer-to-be](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-be.md) | Suggest using `toBe()` | | 🔧 | | | [prefer-to-contain](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | | 🔧 | | diff --git a/docs/rules/prefer-locator.md b/docs/rules/prefer-locator.md index 0695c63..3acb71c 100644 --- a/docs/rules/prefer-locator.md +++ b/docs/rules/prefer-locator.md @@ -1,7 +1,7 @@ # Suggest using `page.locator()` (`prefer-locator`) -Instead of using page methods use locator-based e.g. page.fill() use -[locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill) +Suggest using locators and their associated methods instead of page methods for +performing actions. ## Rule details @@ -10,12 +10,22 @@ This rule triggers a warning if page methods are used, instead of locators. The following patterns are considered warnings: ```javascript +await page.click('css=button') +await page.dblclick('xpath=//button') await page.fill('input[type="password"]', 'password') + +await page.frame('frame-name').click('css=button') ``` -The following pattern is **not** a warning: +The following pattern are **not** warnings: ```javascript await page.getByRole('password').fill('password') +await page.getByLabel('User Name').fill('John') +await page.getByRole('button', { name: 'Sign in' }).click() await page.locator('input[type="password"]').fill('password') +await page.locator('css=button').click() +await page.locator('xpath=//button').dblclick() + +await page.frameLocator('#my-iframe').getByText('Submit').click() ``` diff --git a/src/rules/prefer-locator.test.ts b/src/rules/prefer-locator.test.ts index 9e38beb..1be4f06 100644 --- a/src/rules/prefer-locator.test.ts +++ b/src/rules/prefer-locator.test.ts @@ -6,28 +6,89 @@ runRuleTester('prefer-locator', rule, { { code: ` async function test() { - await page.fill(); + await page.fill('input[type="password"]', 'password'); } `, errors: [ { column: 15, - endColumn: 32, + endColumn: 68, endLine: 3, line: 3, - messageId: 'avoidAwaitPageMethods', + messageId: 'preferLocator', + }, + ], + output: null, + }, + { + code: ` + async function test() { + await page.dblclick('xpath=//button'); + } + `, + errors: [ + { + column: 15, + endColumn: 52, + endLine: 3, + line: 3, + messageId: 'preferLocator', + }, + ], + output: null, + }, + { + code: ` + async function test() { + await page.frame('frame-name').click('css=button'); + } + `, + errors: [ + { + column: 15, + endColumn: 65, + endLine: 3, + line: 3, + messageId: 'preferLocator', }, ], output: null, }, ], valid: [ + { + code: `const locator = page.locator('input[type="password"]')`, + }, { code: ` async function test() { - await page.locator(); + await page.locator('input[type="password"]').fill('password'); } `, }, + { + code: ` + async function test() { + await page.locator('xpath=//button').dblclick(); + } + `, + }, + { + code: ` + async function test() { + await page.frameLocator('#my-iframe').locator('css=button').click(); + } + `, + }, + { + code: ` + async function test() { + await page.evaluate('1 + 2'); + } + `, + }, + { + code: `page.frame('frame-name')`, + }, ], }) diff --git a/src/rules/prefer-locator.ts b/src/rules/prefer-locator.ts index bdac874..b46dbd2 100644 --- a/src/rules/prefer-locator.ts +++ b/src/rules/prefer-locator.ts @@ -46,7 +46,7 @@ export default createRule({ if (!isSupportedMethod(node.argument)) return context.report({ - messageId: 'avoidAwaitPageMethods', + messageId: 'preferLocator', node, }) }, @@ -55,13 +55,12 @@ export default createRule({ meta: { docs: { category: 'Best Practices', - description: 'Discourage using await page methods', + description: 'Suggest locators over page methods', recommended: false, url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-locator.md', }, messages: { - avoidAwaitPageMethods: - "Avoid using page methods e.g. 'await page.fill()', Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill)", + preferLocator: 'Prefer locator methods instead of page methods', }, schema: [], type: 'suggestion', From ef612d607ab3be6af50fed312349ad76a351068a Mon Sep 17 00:00:00 2001 From: Carl Bray Date: Thu, 19 Sep 2024 16:55:53 +1200 Subject: [PATCH 7/9] Switch to CallExpression --- src/index.ts | 4 ++-- src/rules/prefer-locator.test.ts | 35 +++++++++++++++++++++++++++++--- src/rules/prefer-locator.ts | 7 ++----- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/index.ts b/src/index.ts index 4e4cdac..e7312d8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -30,7 +30,7 @@ import preferComparisonMatcher from './rules/prefer-comparison-matcher' import preferEqualityMatcher from './rules/prefer-equality-matcher' import preferHooksInOrder from './rules/prefer-hooks-in-order' import preferHooksOnTop from './rules/prefer-hooks-on-top' -import preferPageLocator from './rules/prefer-locator' +import preferLocator from './rules/prefer-locator' import preferLowercaseTitle from './rules/prefer-lowercase-title' import preferNativeLocators from './rules/prefer-native-locators' import preferStrictEqual from './rules/prefer-strict-equal' @@ -82,7 +82,7 @@ const index = { 'prefer-equality-matcher': preferEqualityMatcher, 'prefer-hooks-in-order': preferHooksInOrder, 'prefer-hooks-on-top': preferHooksOnTop, - 'prefer-locator': preferPageLocator, + 'prefer-locator': preferLocator, 'prefer-lowercase-title': preferLowercaseTitle, 'prefer-native-locators': preferNativeLocators, 'prefer-strict-equal': preferStrictEqual, diff --git a/src/rules/prefer-locator.test.ts b/src/rules/prefer-locator.test.ts index 1be4f06..9c1b5f1 100644 --- a/src/rules/prefer-locator.test.ts +++ b/src/rules/prefer-locator.test.ts @@ -11,7 +11,7 @@ runRuleTester('prefer-locator', rule, { `, errors: [ { - column: 15, + column: 21, endColumn: 68, endLine: 3, line: 3, @@ -28,7 +28,7 @@ runRuleTester('prefer-locator', rule, { `, errors: [ { - column: 15, + column: 21, endColumn: 52, endLine: 3, line: 3, @@ -37,6 +37,19 @@ runRuleTester('prefer-locator', rule, { ], output: null, }, + { + code: `page.click('xpath=//button');`, + errors: [ + { + column: 1, + endColumn: 29, + endLine: 1, + line: 1, + messageId: 'preferLocator', + }, + ], + output: null, + }, { code: ` async function test() { @@ -45,7 +58,7 @@ runRuleTester('prefer-locator', rule, { `, errors: [ { - column: 15, + column: 21, endColumn: 65, endLine: 3, line: 3, @@ -54,6 +67,19 @@ runRuleTester('prefer-locator', rule, { ], output: null, }, + { + code: `page.frame('frame-name').click('css=button')`, + errors: [ + { + column: 1, + endColumn: 45, + endLine: 1, + line: 1, + messageId: 'preferLocator', + }, + ], + output: null, + }, ], valid: [ { @@ -73,6 +99,9 @@ runRuleTester('prefer-locator', rule, { } `, }, + { + code: `page.locator('xpath=//button').click();`, + }, { code: ` async function test() { diff --git a/src/rules/prefer-locator.ts b/src/rules/prefer-locator.ts index b46dbd2..e2ccab1 100644 --- a/src/rules/prefer-locator.ts +++ b/src/rules/prefer-locator.ts @@ -38,12 +38,9 @@ function isSupportedMethod(node: ESTree.CallExpression) { export default createRule({ create(context) { return { - AwaitExpression(node) { - // Must be a call expression - if (node.argument.type !== 'CallExpression') return - + CallExpression(node) { // Must be a method we care about - if (!isSupportedMethod(node.argument)) return + if (!isSupportedMethod(node)) return context.report({ messageId: 'preferLocator', From 2ff86373b69618ccb5112b30195be233cdcb5949 Mon Sep 17 00:00:00 2001 From: Carl Bray Date: Thu, 19 Sep 2024 16:57:39 +1200 Subject: [PATCH 8/9] Add examples without await --- docs/rules/prefer-locator.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/rules/prefer-locator.md b/docs/rules/prefer-locator.md index 3acb71c..14efbc0 100644 --- a/docs/rules/prefer-locator.md +++ b/docs/rules/prefer-locator.md @@ -10,6 +10,7 @@ This rule triggers a warning if page methods are used, instead of locators. The following patterns are considered warnings: ```javascript +page.click('css=button') await page.click('css=button') await page.dblclick('xpath=//button') await page.fill('input[type="password"]', 'password') @@ -20,6 +21,7 @@ await page.frame('frame-name').click('css=button') The following pattern are **not** warnings: ```javascript +const locator = page.locator('css=button') await page.getByRole('password').fill('password') await page.getByLabel('User Name').fill('John') await page.getByRole('button', { name: 'Sign in' }).click() From 99bc2aac365dd0d56b3026a683ce00583736ee38 Mon Sep 17 00:00:00 2001 From: Carl Bray Date: Fri, 20 Sep 2024 11:42:49 +1200 Subject: [PATCH 9/9] Use test from rule-tester --- src/rules/prefer-locator.test.ts | 76 +++++++++++--------------------- 1 file changed, 26 insertions(+), 50 deletions(-) diff --git a/src/rules/prefer-locator.test.ts b/src/rules/prefer-locator.test.ts index 9c1b5f1..13135d4 100644 --- a/src/rules/prefer-locator.test.ts +++ b/src/rules/prefer-locator.test.ts @@ -1,44 +1,36 @@ -import { runRuleTester } from '../utils/rule-tester' +import { runRuleTester, test } from '../utils/rule-tester' import rule from './prefer-locator' runRuleTester('prefer-locator', rule, { invalid: [ { - code: ` - async function test() { - await page.fill('input[type="password"]', 'password'); - } - `, + code: test(`await page.fill('input[type="password"]', 'password')`), errors: [ { - column: 21, - endColumn: 68, - endLine: 3, - line: 3, + column: 34, + endColumn: 81, + endLine: 1, + line: 1, messageId: 'preferLocator', }, ], output: null, }, { - code: ` - async function test() { - await page.dblclick('xpath=//button'); - } - `, + code: test(`await page.dblclick('xpath=//button')`), errors: [ { - column: 21, - endColumn: 52, - endLine: 3, - line: 3, + column: 34, + endColumn: 65, + endLine: 1, + line: 1, messageId: 'preferLocator', }, ], output: null, }, { - code: `page.click('xpath=//button');`, + code: `page.click('xpath=//button')`, errors: [ { column: 1, @@ -51,17 +43,13 @@ runRuleTester('prefer-locator', rule, { output: null, }, { - code: ` - async function test() { - await page.frame('frame-name').click('css=button'); - } - `, + code: test(`await page.frame('frame-name').click('css=button')`), errors: [ { - column: 21, - endColumn: 65, - endLine: 3, - line: 3, + column: 34, + endColumn: 78, + endLine: 1, + line: 1, messageId: 'preferLocator', }, ], @@ -86,35 +74,23 @@ runRuleTester('prefer-locator', rule, { code: `const locator = page.locator('input[type="password"]')`, }, { - code: ` - async function test() { - await page.locator('input[type="password"]').fill('password'); - } - `, + code: test( + `await page.locator('input[type="password"]').fill('password')`, + ), }, { - code: ` - async function test() { - await page.locator('xpath=//button').dblclick(); - } - `, + code: test(`await page.locator('xpath=//button').dblclick()`), }, { - code: `page.locator('xpath=//button').click();`, + code: `page.locator('xpath=//button').click()`, }, { - code: ` - async function test() { - await page.frameLocator('#my-iframe').locator('css=button').click(); - } - `, + code: test( + `await page.frameLocator('#my-iframe').locator('css=button').click()`, + ), }, { - code: ` - async function test() { - await page.evaluate('1 + 2'); - } - `, + code: test(`await page.evaluate('1 + 2')`), }, { code: `page.frame('frame-name')`,