From e4c32babbcd99b723c0cef473a2d32ea17df2535 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Mon, 19 Aug 2024 22:42:56 -0400 Subject: [PATCH 1/5] Check if page methods are awaited --- src/rules/missing-playwright-await.test.ts | 18 ++++++++++++++++++ src/rules/missing-playwright-await.ts | 20 +++++++++++++++++++- src/utils/ast.ts | 9 ++++++++- 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/rules/missing-playwright-await.test.ts b/src/rules/missing-playwright-await.test.ts index af3cf9c..80d2ca6 100644 --- a/src/rules/missing-playwright-await.test.ts +++ b/src/rules/missing-playwright-await.test.ts @@ -247,6 +247,21 @@ runRuleTester('missing-playwright-await', rule, { }, }, }, + // Page methods + { + code: `page.goto('https://example.com')`, + errors: [ + { column: 1, endColumn: 10, endLine: 1, line: 1, messageId: 'page' }, + ], + output: `await page.goto('https://example.com')`, + }, + { + code: test(`page.goto('https://example.com')`), + errors: [ + { column: 28, endColumn: 37, endLine: 1, line: 1, messageId: 'page' }, + ], + output: test(`await page.goto('https://example.com')`), + }, ], valid: [ // Basic @@ -368,5 +383,8 @@ runRuleTester('missing-playwright-await', rule, { }, }, }, + // Page methods + { code: `await page.goto('https://example.com')` }, + { code: test(`await page.goto('https://example.com')`) }, ], }) diff --git a/src/rules/missing-playwright-await.ts b/src/rules/missing-playwright-await.ts index 23f8ca1..17a93cd 100644 --- a/src/rules/missing-playwright-await.ts +++ b/src/rules/missing-playwright-await.ts @@ -1,6 +1,6 @@ import { Rule } from 'eslint' import ESTree from 'estree' -import { getParent, getStringValue, isIdentifier } from '../utils/ast' +import { getParent, getStringValue, isIdentifier, isPage } from '../utils/ast' import { createRule } from '../utils/createRule' import { ParsedFnCall, parseFnCall } from '../utils/parseFnCall' @@ -57,6 +57,8 @@ const playwrightTestMatchers = [ 'toBeInViewport', ] +const pageMethods = new Set(['goto']) + function getReportNode(node: ESTree.Node) { const parent = getParent(node) return parent?.type === 'MemberExpression' ? parent : node @@ -139,6 +141,21 @@ export default createRule({ return { CallExpression(node) { + // Checking validity of calls to methods on the page object + if (isPage(node) && node.callee.type === 'MemberExpression') { + const method = getStringValue(node.callee.property) + const isValid = checkValidity(node) + + if (!isValid && pageMethods.has(method)) { + context.report({ + data: { method }, + fix: (fixer) => fixer.insertTextBefore(node, 'await '), + messageId: 'page', + node: node.callee, + }) + } + } + const call = parseFnCall(context, node) if (call?.type !== 'step' && call?.type !== 'expect') return @@ -167,6 +184,7 @@ export default createRule({ messages: { expect: "'{{matcherName}}' must be awaited or returned.", expectPoll: "'expect.poll' matchers must be awaited or returned.", + page: "'{{method}}' must be awaited or returned.", testStep: "'test.step' must be awaited or returned.", }, schema: [ diff --git a/src/utils/ast.ts b/src/utils/ast.ts index ffa0b65..66adec5 100644 --- a/src/utils/ast.ts +++ b/src/utils/ast.ts @@ -116,10 +116,17 @@ export function dig(node: ESTree.Node, identifier: string | RegExp): boolean { : false } +export function isPage(node: ESTree.CallExpression) { + return ( + node.callee.type === 'MemberExpression' && + dig(node.callee.object, /(^(page|frame)|(Page|Frame)$)/) + ) +} + export function isPageMethod(node: ESTree.CallExpression, name: string) { return ( node.callee.type === 'MemberExpression' && - dig(node.callee.object, /(^(page|frame)|(Page|Frame)$)/) && + isPage(node) && isPropertyAccessor(node.callee, name) ) } From ef66ae15c3a799a7bab69b96099e43274ed8e2d1 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Mon, 19 Aug 2024 23:12:54 -0400 Subject: [PATCH 2/5] Add more methods --- src/rules/missing-playwright-await.test.ts | 8 ++- src/rules/missing-playwright-await.ts | 70 +++++++++++++++++++++- 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/src/rules/missing-playwright-await.test.ts b/src/rules/missing-playwright-await.test.ts index 80d2ca6..22690fa 100644 --- a/src/rules/missing-playwright-await.test.ts +++ b/src/rules/missing-playwright-await.test.ts @@ -385,6 +385,12 @@ runRuleTester('missing-playwright-await', rule, { }, // Page methods { code: `await page.goto('https://example.com')` }, - { code: test(`await page.goto('https://example.com')`) }, + { code: `await page.title()` }, + // Other page methods are ignored + { code: `page.frames()` }, + // Other methods with the same name are ignored + { code: `randomObject.title()` }, + // Does not need to be awaited when returned + { code: `() => { return page.content() }` }, ], }) diff --git a/src/rules/missing-playwright-await.ts b/src/rules/missing-playwright-await.ts index 17a93cd..c639bd2 100644 --- a/src/rules/missing-playwright-await.ts +++ b/src/rules/missing-playwright-await.ts @@ -57,7 +57,75 @@ const playwrightTestMatchers = [ 'toBeInViewport', ] -const pageMethods = new Set(['goto']) +const pageMethods = new Set([ + '$', // deprecated + '$$', // deprecated + '$eval', // deprecated + '$$eval', // deprecated + 'addInitScript', + 'addLocatorHandler', + 'addScriptTag', + 'addStyleTag', + 'bringToFront', + 'check', // deprecated + 'click', // deprecated + 'close', + 'content', + 'dblclick', // deprecated + 'dispatchEvent', // deprecated + 'dragAndDrop', + 'emulateMedia', + 'evaluate', + 'evaluateHandle', + 'exposeBinding', + 'exposeFunction', + 'fill', // deprecated + 'focus', // deprecated + 'getAttribute', // deprecated + 'goBack', + 'goForward', + 'goto', + 'hover', // deprecated + 'innerHTML', // deprecated + 'innerText', // deprecated + 'inputValue', // deprecated + 'isChecked', // deprecated + 'isDisabled', // deprecated + 'isEditable', // deprecated + 'isEnabled', // deprecated + 'isHidden', // deprecated + 'isVisible', // deprecated + 'opener', + 'pause', + 'pdf', + 'press', // deprecated + 'reload', + 'removeLocatorHandler', + 'route', + 'routeFromHAR', + 'screenshot', + 'selectOption', // deprecated + 'setChecked', // deprecated + 'setContent', + 'setExtraHTTPHeaders', + 'setInputFiles', // deprecated + 'setViewportSize', + 'tap', // deprecated + 'textContent', // deprecated + 'title', + 'type', // deprecated + 'unroute', + 'unrouteAll', + 'waitForEvent', + 'waitForFunction', + 'waitForLoadState', + 'waitForNavigation', // deprecated + 'waitForRequest', + 'waitForResponse', + 'waitForSelector', // deprecated + 'waitForTimeout', // deprecated + 'waitForURL', +]) function getReportNode(node: ESTree.Node) { const parent = getParent(node) From af9ea25fd7cd4aaeb5fd3a5a83e004b08a6567e6 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Mon, 19 Aug 2024 23:17:09 -0400 Subject: [PATCH 3/5] Update docs --- docs/rules/missing-playwright-await.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/rules/missing-playwright-await.md b/docs/rules/missing-playwright-await.md index dc159e5..b23c1bd 100644 --- a/docs/rules/missing-playwright-await.md +++ b/docs/rules/missing-playwright-await.md @@ -9,6 +9,7 @@ Example of **incorrect** code for this rule: ```javascript expect(page).toMatchText('text') expect.poll(() => foo).toBe(true) +page.goto('https://example.com') test.step('clicks the button', async () => { await page.click('button') @@ -20,6 +21,7 @@ Example of **correct** code for this rule: ```javascript await expect(page).toMatchText('text') await expect.poll(() => foo).toBe(true) +await page.goto('https://example.com') await test.step('clicks the button', async () => { await page.click('button') From 4b8b2a7e7cfa491d58bc39b83810280dd3e6d983 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Mon, 19 Aug 2024 23:23:56 -0400 Subject: [PATCH 4/5] Remove waitForEvent --- src/rules/missing-playwright-await.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rules/missing-playwright-await.ts b/src/rules/missing-playwright-await.ts index c639bd2..ddd6013 100644 --- a/src/rules/missing-playwright-await.ts +++ b/src/rules/missing-playwright-await.ts @@ -116,7 +116,6 @@ const pageMethods = new Set([ 'type', // deprecated 'unroute', 'unrouteAll', - 'waitForEvent', 'waitForFunction', 'waitForLoadState', 'waitForNavigation', // deprecated From cf053ac6516d58affc9a98496e7379cb2f4decf0 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Mon, 19 Aug 2024 23:29:20 -0400 Subject: [PATCH 5/5] Remove wait for request/response --- src/rules/missing-playwright-await.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/rules/missing-playwright-await.ts b/src/rules/missing-playwright-await.ts index ddd6013..89b81f5 100644 --- a/src/rules/missing-playwright-await.ts +++ b/src/rules/missing-playwright-await.ts @@ -119,8 +119,6 @@ const pageMethods = new Set([ 'waitForFunction', 'waitForLoadState', 'waitForNavigation', // deprecated - 'waitForRequest', - 'waitForResponse', 'waitForSelector', // deprecated 'waitForTimeout', // deprecated 'waitForURL',