Skip to content

Commit 54544aa

Browse files
mskeltonmskelton
authored
No useless await (#135)
* Docs * Shell * Tests * Finish rule * Detect frame as well for page methods * More test cases
1 parent eefdb0e commit 54544aa

File tree

7 files changed

+320
-4
lines changed

7 files changed

+320
-4
lines changed

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ command line option.\
6363
| ✔ | | | [no-eval](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-eval.md) | Disallow usage of `page.$eval` and `page.$$eval` |
6464
| ✔ | | 💡 | [no-focused-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-focused-test.md) | Disallow usage of `.only` annotation |
6565
| ✔ | | | [no-force-option](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-force-option.md) | Disallow usage of the `{ force: true }` option |
66+
| ✔ | | | [no-networkidle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-networkidle.md) | Disallow usage of the `networkidle` option |
6667
| ✔ | | | [no-page-pause](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-page-pause.md) | Disallow using `page.pause` |
68+
| ✔ | 🔧 | | [no-useless-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-await.md) | Disallow unnecessary `await`s for Playwright methods |
6769
| | | | [no-restricted-matchers](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers |
6870
| ✔ | | 💡 | [no-skipped-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-skipped-test.md) | Disallow usage of the `.skip` annotation |
6971
| ✔ | 🔧 | | [no-useless-not](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-not.md) | Disallow usage of `not` matchers when a specific matcher exists |

‎docs/rules/no-useless-await.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# Disallow unnecessary `await`s for Playwright methods (`no-useless-await`)
2+
3+
Some Playwright methods are frequently, yet incorrectly, awaited when the await
4+
expression has no effect.
5+
6+
## Rule Details
7+
8+
Examples of **incorrect** code for this rule:
9+
10+
```javascript
11+
await page.locator('.my-element');
12+
await page.getByRole('.my-element');
13+
```
14+
15+
Examples of **correct** code for this rule:
16+
17+
```javascript
18+
page.locator('.my-element');
19+
page.getByRole('.my-element');
20+
21+
await page.$('.my-element');
22+
await page.goto('.my-element');
23+
```

‎examples/package-lock.json‎

Lines changed: 11 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import noNetworkidle from './rules/no-networkidle';
99
import noPagePause from './rules/no-page-pause';
1010
import noRestrictedMatchers from './rules/no-restricted-matchers';
1111
import noSkippedTest from './rules/no-skipped-test';
12+
import noUselessAwait from './rules/no-useless-await';
1213
import noUselessNot from './rules/no-useless-not';
1314
import noWaitForTimeout from './rules/no-wait-for-timeout';
1415
import preferLowercaseTitle from './rules/prefer-lowercase-title';
@@ -37,6 +38,7 @@ const recommended = {
3738
'playwright/no-networkidle': 'error',
3839
'playwright/no-page-pause': 'warn',
3940
'playwright/no-skipped-test': 'warn',
41+
'playwright/no-useless-await': 'warn',
4042
'playwright/no-useless-not': 'warn',
4143
'playwright/no-wait-for-timeout': 'warn',
4244
'playwright/prefer-web-first-assertions': 'error',
@@ -93,6 +95,7 @@ export = {
9395
'no-page-pause': noPagePause,
9496
'no-restricted-matchers': noRestrictedMatchers,
9597
'no-skipped-test': noSkippedTest,
98+
'no-useless-await': noUselessAwait,
9699
'no-useless-not': noUselessNot,
97100
'no-wait-for-timeout': noWaitForTimeout,
98101
'prefer-lowercase-title': preferLowercaseTitle,

‎src/rules/no-useless-await.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import { Rule } from 'eslint';
2+
import { getStringValue } from '../utils/ast';
3+
4+
const methods = new Set([
5+
'and',
6+
'childFrames',
7+
'first',
8+
'frame',
9+
'frameLocator',
10+
'frames',
11+
'getByAltText',
12+
'getByLabel',
13+
'getByPlaceholder',
14+
'getByRole',
15+
'getByTestId',
16+
'getByText',
17+
'getByTitle',
18+
'isClosed',
19+
'isDetached',
20+
'last',
21+
'locator',
22+
'mainFrame',
23+
'name',
24+
'nth',
25+
'on',
26+
'or',
27+
'page',
28+
'parentFrame',
29+
'setDefaultNavigationTimeout',
30+
'setDefaultTimeout',
31+
'url',
32+
'video',
33+
'viewportSize',
34+
'workers',
35+
]);
36+
37+
export default {
38+
create(context) {
39+
return {
40+
AwaitExpression(node) {
41+
// Must be a call expression
42+
if (node.argument.type !== 'CallExpression') return;
43+
44+
// Must be a foo.bar() call, bare calls are ignored
45+
const { callee } = node.argument;
46+
if (callee.type !== 'MemberExpression') return;
47+
48+
// Must be a method we care about
49+
const { property } = callee;
50+
if (!methods.has(getStringValue(property))) return;
51+
52+
const start = node.loc!.start;
53+
const range = node.range!;
54+
55+
context.report({
56+
fix: (fixer) => fixer.removeRange([range[0], range[0] + 6]),
57+
loc: {
58+
end: {
59+
column: start.column + 5,
60+
line: start.line,
61+
},
62+
start,
63+
},
64+
messageId: 'noUselessAwait',
65+
});
66+
},
67+
};
68+
},
69+
meta: {
70+
docs: {
71+
category: 'Possible Errors',
72+
description: 'Disallow unnecessary awaits for Playwright methods',
73+
recommended: true,
74+
},
75+
fixable: 'code',
76+
messages: {
77+
noUselessAwait:
78+
'Unnecessary await expression. This method does not return a Promise.',
79+
},
80+
type: 'problem',
81+
},
82+
} as Rule.RuleModule;

‎src/utils/ast.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ function dig(node: ESTree.Node, identifier: string | RegExp): boolean {
168168
export function isPageMethod(node: ESTree.CallExpression, name: string) {
169169
return (
170170
node.callee.type === 'MemberExpression' &&
171-
dig(node.callee.object, /(^page|Page$)/) &&
171+
dig(node.callee.object, /(^(page|frame)|(Page|Frame)$)/) &&
172172
isPropertyAccessor(node.callee, name)
173173
);
174174
}

‎test/spec/no-useless-await.spec.ts‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
import rule from '../../src/rules/no-useless-await';
2+
import { runRuleTester } from '../utils/rule-tester';
3+
4+
const messageId = 'noUselessAwait';
5+
6+
runRuleTester('no-useless-await', rule, {
7+
invalid: [
8+
// Page, frames, and locators
9+
{
10+
code: 'await page.locator(".my-element")',
11+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
12+
output: 'page.locator(".my-element")',
13+
},
14+
{
15+
code: 'await frame.locator(".my-element")',
16+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
17+
output: 'frame.locator(".my-element")',
18+
},
19+
{
20+
code: 'await foo.locator(".my-element")',
21+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
22+
output: 'foo.locator(".my-element")',
23+
},
24+
25+
// nth methods
26+
{
27+
code: 'await foo.first()',
28+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
29+
output: 'foo.first()',
30+
},
31+
{
32+
code: 'await foo.last()',
33+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
34+
output: 'foo.last()',
35+
},
36+
{
37+
code: 'await foo.nth(3)',
38+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
39+
output: 'foo.nth(3)',
40+
},
41+
{
42+
code: 'await foo.and(page.locator(".my-element"))',
43+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
44+
output: 'foo.and(page.locator(".my-element"))',
45+
},
46+
{
47+
code: 'await foo.or(page.locator(".my-element"))',
48+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
49+
output: 'foo.or(page.locator(".my-element"))',
50+
},
51+
52+
// Testing library methods
53+
{
54+
code: 'await page.getByAltText("foo")',
55+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
56+
output: 'page.getByAltText("foo")',
57+
},
58+
{
59+
code: 'await page["getByRole"]("button")',
60+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
61+
output: 'page["getByRole"]("button")',
62+
},
63+
{
64+
code: 'await page[`getByLabel`]("foo")',
65+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
66+
output: 'page[`getByLabel`]("foo")',
67+
},
68+
{
69+
code: 'await page.getByPlaceholder("foo")',
70+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
71+
output: 'page.getByPlaceholder("foo")',
72+
},
73+
{
74+
code: 'await page.getByTestId("foo")',
75+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
76+
output: 'page.getByTestId("foo")',
77+
},
78+
{
79+
code: 'await page.getByText("foo")',
80+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
81+
output: 'page.getByText("foo")',
82+
},
83+
{
84+
code: 'await page.getByTitle("foo")',
85+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
86+
output: 'page.getByTitle("foo")',
87+
},
88+
89+
// Event handlers
90+
{
91+
code: 'await page.on("console", () => {})',
92+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
93+
output: 'page.on("console", () => {})',
94+
},
95+
{
96+
code: 'await frame.on("console", () => {})',
97+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
98+
output: 'frame.on("console", () => {})',
99+
},
100+
101+
// Misc page methods
102+
{
103+
code: 'await page.frame("foo")',
104+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
105+
output: 'page.frame("foo")',
106+
},
107+
{
108+
code: 'await page.frameLocator("#foo")',
109+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
110+
output: 'page.frameLocator("#foo")',
111+
},
112+
{
113+
code: 'await page.frames()',
114+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
115+
output: 'page.frames()',
116+
},
117+
{
118+
code: 'await page.mainFrame()',
119+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
120+
output: 'page.mainFrame()',
121+
},
122+
{
123+
code: 'await page.isClosed()',
124+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
125+
output: 'page.isClosed()',
126+
},
127+
{
128+
code: 'await page.setDefaultNavigationTimeout()',
129+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
130+
output: 'page.setDefaultNavigationTimeout()',
131+
},
132+
{
133+
code: 'await page.setDefaultTimeout()',
134+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
135+
output: 'page.setDefaultTimeout()',
136+
},
137+
{
138+
code: 'await page.url()',
139+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
140+
output: 'page.url()',
141+
},
142+
{
143+
code: 'await page.video()',
144+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
145+
output: 'page.video()',
146+
},
147+
{
148+
code: 'await page.viewportSize()',
149+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
150+
output: 'page.viewportSize()',
151+
},
152+
{
153+
code: 'await page.workers()',
154+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
155+
output: 'page.workers()',
156+
},
157+
158+
// Misc frame methods
159+
{
160+
code: 'await frame.childFrames()',
161+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
162+
output: 'frame.childFrames()',
163+
},
164+
{
165+
code: 'await frame.isDetached()',
166+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
167+
output: 'frame.isDetached()',
168+
},
169+
{
170+
code: 'await frame.name()',
171+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
172+
output: 'frame.name()',
173+
},
174+
{
175+
code: 'await frame.page()',
176+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
177+
output: 'frame.page()',
178+
},
179+
{
180+
code: 'await frame.parentFrame()',
181+
errors: [{ column: 1, endColumn: 6, line: 1, messageId }],
182+
output: 'frame.parentFrame()',
183+
},
184+
],
185+
valid: [
186+
'await foo()',
187+
'await foo(".my-element")',
188+
'await foo.bar()',
189+
'await foo.bar(".my-element")',
190+
191+
'page.getByRole(".my-element")',
192+
'page.locator(".my-element")',
193+
194+
'await page.waitForLoadState({ waitUntil: "load" })',
195+
'await page.waitForUrl(url, { waitUntil: "load" })',
196+
'await page.locator(".hello-world").waitFor()',
197+
],
198+
});

0 commit comments

Comments
 (0)