Always report missing await for expect.poll() regardless of matcher (…

…#98) * Update tests * Finish updates * More tests
playwright-community · Aug 31, 2022 · 0d872b8 · 0d872b8
1 parent 637dc69
commit 0d872b8
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 41 deletions.
diff --git a/docs/rules/missing-playwright-await.md b/docs/rules/missing-playwright-await.md
@@ -8,6 +8,7 @@ Example of **incorrect** code for this rule:
 
 ```javascript
 expect(page).toMatchText('text');
+expect.poll(() => foo).toBe(true);
 
 test.step('clicks the button', async () => {
   await page.click('button');
@@ -18,6 +19,7 @@ Example of **correct** code for this rule:
 
 ```javascript
 await expect(page).toMatchText('text');
+await expect.poll(() => foo).toBe(true);
 
 await test.step('clicks the button', async () => {
   await page.click('button');

diff --git a/src/rules/missing-playwright-await.ts b/src/rules/missing-playwright-await.ts
@@ -1,37 +1,19 @@
 import * as ESTree from 'estree';
 import { Rule } from 'eslint';
-import { getStringValue } from '../utils/ast';
-
-type MemberExpression = ESTree.MemberExpression & Rule.NodeParentExtension;
-
-function getMemberExpressionNode(
-  node: MemberExpression,
-  matchers: Set<string>
-) {
-  const propertyName = getStringValue(node.property);
-
-  if (getStringValue(node.object) === 'test') {
-    return propertyName === 'step' ? { node, type: 'testStep' } : undefined;
-  }
-
-  return propertyName && matchers.has(propertyName)
-    ? { node, type: 'expect' }
-    : undefined;
-}
+import {
+  getMatchers,
+  isPropertyAccessor,
+  parseExpectCall,
+  isIdentifier,
+  getStringValue,
+} from '../utils/ast';
 
 const validTypes = new Set([
   'AwaitExpression',
   'ReturnStatement',
   'ArrowFunctionExpression',
 ]);
 
-function isValid(node: MemberExpression) {
-  return (
-    validTypes.has(node.parent?.type) ||
-    validTypes.has(node.parent?.parent?.type)
-  );
-}
-
 const expectPlaywrightMatchers = [
   'toBeChecked',
   'toBeDisabled',
@@ -75,25 +57,66 @@ const playwrightTestMatchers = [
   'toHaveValue',
 ];
 
+function getCallType(
+  node: ESTree.CallExpression & Rule.NodeParentExtension,
+  awaitableMatchers: Set<string>
+) {
+  // test.step
+  if (
+    node.callee.type === 'MemberExpression' &&
+    isIdentifier(node.callee.object, 'test') &&
+    isPropertyAccessor(node.callee, 'step')
+  ) {
+    return { messageId: 'testStep' };
+  }
+
+  const expectType = parseExpectCall(node);
+  if (!expectType) return;
+
+  // expect.poll
+  if (expectType === 'poll') {
+    return { messageId: 'expectPoll' };
+  }
+
+  // expect with awitable matcher
+  const [lastMatcher] = getMatchers(node).slice(-1);
+  const matcherName = getStringValue(lastMatcher);
+
+  if (awaitableMatchers.has(matcherName)) {
+    return { messageId: 'expect', data: { matcherName } };
+  }
+}
+
+function checkValidity(node: Rule.Node): ESTree.Node | undefined {
+  return validTypes.has(node.parent.type)
+    ? undefined
+    : node.parent.type === 'MemberExpression' ||
+      (node.parent.type === 'CallExpression' && node.parent.callee === node)
+    ? checkValidity(node.parent)
+    : node;
+}
+
 export default {
   create(context) {
     const options = context.options[0] || {};
-    const matchers = new Set([
+    const awaitableMatchers = new Set([
       ...expectPlaywrightMatchers,
       ...playwrightTestMatchers,
       // Add any custom matchers to the set
       ...(options.customMatchers || []),
     ]);
 
     return {
-      MemberExpression(statement) {
-        const result = getMemberExpressionNode(statement, matchers);
+      CallExpression(node) {
+        const result = getCallType(node, awaitableMatchers);
+        const reportNode = result ? checkValidity(node) : undefined;
 
-        if (result && !isValid(result.node)) {
+        if (result && reportNode) {
           context.report({
-            fix: (fixer) => fixer.insertTextBefore(result.node, 'await '),
-            messageId: result.type,
-            node: result.node,
+            fix: (fixer) => fixer.insertTextBefore(node, 'await '),
+            messageId: result.messageId,
+            data: result.data,
+            node: reportNode,
           });
         }
       },
@@ -107,7 +130,8 @@ export default {
     },
     fixable: 'code',
     messages: {
-      expect: "'expect' matchers must be awaited or returned.",
+      expect: "'{{matcherName}}' must be awaited or returned.",
+      expectPoll: "'expect.poll' matchers 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
@@ -2,7 +2,9 @@ import { Rule } from 'eslint';
 import * as ESTree from 'estree';
 import { NodeWithParent, TypedNodeWithParent } from './types';
 
-export function getStringValue(node: ESTree.Node) {
+export function getStringValue(node: ESTree.Node | undefined) {
+  if (!node) return '';
+
   return node.type === 'Identifier'
     ? node.name
     : node.type === 'TemplateLiteral'
@@ -12,7 +14,7 @@ export function getStringValue(node: ESTree.Node) {
     : '';
 }
 
-function isIdentifier(node: ESTree.Node, name: string) {
+export function isIdentifier(node: ESTree.Node, name: string) {
   return node.type === 'Identifier' && node.name === name;
 }
 
@@ -120,13 +122,26 @@ export function isTestHook(node: ESTree.CallExpression) {
 }
 
 const expectSubCommands = new Set(['soft', 'poll']);
+export type ExpectType = 'standalone' | 'soft' | 'poll';
+
+export function parseExpectCall(
+  node: ESTree.CallExpression
+): ExpectType | undefined {
+  if (isIdentifier(node.callee, 'expect')) {
+    return 'standalone';
+  }
+
+  if (
+    node.callee.type === 'MemberExpression' &&
+    isIdentifier(node.callee.object, 'expect')
+  ) {
+    const type = getStringValue(node.callee.property);
+    return expectSubCommands.has(type) ? (type as ExpectType) : undefined;
+  }
+}
+
 export function isExpectCall(node: ESTree.CallExpression) {
-  return (
-    isIdentifier(node.callee, 'expect') ||
-    (node.callee.type === 'MemberExpression' &&
-      isIdentifier(node.callee.object, 'expect') &&
-      expectSubCommands.has(getStringValue(node.callee.property)))
-  );
+  return !!parseExpectCall(node);
 }
 
 export function getMatchers(

diff --git a/test/spec/missing-playwright-await.spec.ts b/test/spec/missing-playwright-await.spec.ts
@@ -64,6 +64,23 @@ runRuleTester('missing-playwright-await', rule, {
       'await expect[`soft`](page)[`toBeChecked`]()'
     ),
 
+    // expect.poll
+    invalid(
+      'expectPoll',
+      'expect.poll(() => foo).toBe(true)',
+      'await expect.poll(() => foo).toBe(true)'
+    ),
+    invalid(
+      'expectPoll',
+      'expect["poll"](() => foo)["toContain"]("bar")',
+      'await expect["poll"](() => foo)["toContain"]("bar")'
+    ),
+    invalid(
+      'expectPoll',
+      'expect[`poll`](() => foo)[`toBeTruthy`]()',
+      'await expect[`poll`](() => foo)[`toBeTruthy`]()'
+    ),
+
     // test.step
     invalid(
       'testStep',
@@ -99,6 +116,11 @@ runRuleTester('missing-playwright-await', rule, {
     valid('await expect.soft(page).toHaveText("text")'),
     valid('await expect.soft(page).not.toHaveText("text")'),
 
+    // expect.poll
+    valid('await expect.poll(() => foo).toBe("text")'),
+    valid('await expect["poll"](() => foo).toContain("text")'),
+    valid('await expect[`poll`](() => foo).toBeTruthy()'),
+
     // test.step
     valid("await test.step('foo', async () => {})"),
   ],