Add prefer-web-first-assertions rule

.github/workflows/test.yml

@@ -9,11 +9,11 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        node-version: [14.x, 16.x, 18.x]
+        node-version: [16.x, 18.x]
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - name: Use Node.js ${{ matrix.node-version }}
-        uses: actions/setup-node@v1
+        uses: actions/setup-node@v3
         with:
           node-version: ${{ matrix.node-version }}
       - run: npm ci

README.md

@@ -49,24 +49,25 @@ command line option.\
 💡: Some problems reported by this rule are manually fixable by editor
 [suggestions](https://eslint.org/docs/latest/developer-guide/working-with-rules#providing-suggestions).
 
-|  ✔  | 🔧  | 💡  | Rule                                                                                                                                                        | Description                                                       |
-| :-: | :-: | :-: | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- |
-|  ✔  |     |     | [max-nested-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/max-nested-describe.md)                         | Enforces a maximum depth to nested describe calls                 |
-|  ✔  | 🔧  |     | [missing-playwright-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/missing-playwright-await.md)               | Enforce Playwright APIs to be awaited                             |
-|  ✔  |     |     | [no-conditional-in-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-conditional-in-test.md)                   | Disallow conditional logic in tests                               |
-|  ✔  |     | 💡  | [no-element-handle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-element-handle.md)                             | Disallow usage of element handles                                 |
-|  ✔  |     |     | [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`                  |
-|  ✔  |     | 💡  | [no-focused-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-focused-test.md)                                 | Disallow usage of `.only` annotation                              |
-|  ✔  |     |     | [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                    |
-|  ✔  |     |     | [no-page-pause](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-page-pause.md)                                     | Disallow using `page.pause`                                       |
-|     |     |     | [no-restricted-matchers](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-restricted-matchers.md)                   | Disallow specific matchers & modifiers                            |
-|  ✔  |     | 💡  | [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                          |
-|  ✔  | 🔧  |     | [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   |
-|  ✔  |     | 💡  | [no-wait-for-timeout](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-timeout.md)                         | Disallow usage of `page.waitForTimeout`                           |
-|     |     | 💡  | [prefer-strict-equal](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-strict-equal.md)                         | Suggest using `toStrictEqual()`                                   |
-|     | 🔧  |     | [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md)                   | Enforce lowercase test names                                      |
-|     | 🔧  |     | [prefer-to-be](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-be.md)                                       | Suggest using `toBe()`                                            |
-|     | 🔧  |     | [prefer-to-have-length](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-have-length.md)                     | Suggest using `toHaveLength()`                                    |
-|     |     |     | [require-top-level-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-top-level-describe.md)           | Require test cases and hooks to be inside a `test.describe` block |
-|     | 🔧  |     | [require-soft-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-soft-assertions.md) | Require assertions to use `expect.soft()`                         |
-|  ✔  |     |     | [valid-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md)                                       | Enforce valid `expect()` usage                                    |
+|  ✔  | 🔧  | 💡  | Rule                                                                                                                                                | Description                                                       |
+| :-: | :-: | :-: | --------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- |
+|  ✔  |     |     | [max-nested-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/max-nested-describe.md)                 | Enforces a maximum depth to nested describe calls                 |
+|  ✔  | 🔧  |     | [missing-playwright-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/missing-playwright-await.md)       | Enforce Playwright APIs to be awaited                             |
+|  ✔  |     |     | [no-conditional-in-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-conditional-in-test.md)           | Disallow conditional logic in tests                               |
+|  ✔  |     | 💡  | [no-element-handle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-element-handle.md)                     | Disallow usage of element handles                                 |
+|  ✔  |     |     | [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`                  |
+|  ✔  |     | 💡  | [no-focused-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-focused-test.md)                         | Disallow usage of `.only` annotation                              |
+|  ✔  |     |     | [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                    |
+|  ✔  |     |     | [no-page-pause](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-page-pause.md)                             | Disallow using `page.pause`                                       |
+|     |     |     | [no-restricted-matchers](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-restricted-matchers.md)           | Disallow specific matchers & modifiers                            |
+|  ✔  |     | 💡  | [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                          |
+|  ✔  | 🔧  |     | [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   |
+|  ✔  |     | 💡  | [no-wait-for-timeout](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-timeout.md)                 | Disallow usage of `page.waitForTimeout`                           |
+|     |     | 💡  | [prefer-strict-equal](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-strict-equal.md)                 | Suggest using `toStrictEqual()`                                   |
+|     | 🔧  |     | [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md)           | Enforce lowercase test names                                      |
+|     | 🔧  |     | [prefer-to-be](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-be.md)                               | Suggest using `toBe()`                                            |
+|     | 🔧  |     | [prefer-to-have-length](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-have-length.md)             | Suggest using `toHaveLength()`                                    |
+|  ✔  | 🔧  |     | [prefer-web-first-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-web-first-assertions.md) | Suggest using web first assertions                                |
+|     |     |     | [require-top-level-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-top-level-describe.md)   | Require test cases and hooks to be inside a `test.describe` block |
+|     | 🔧  |     | [require-soft-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-soft-assertions.md)         | Require assertions to use `expect.soft()`                         |
+|  ✔  |     |     | [valid-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md)                               | Enforce valid `expect()` usage                                    |

docs/rules/prefer-web-first-assertions.md

@@ -0,0 +1,24 @@
+# Prefer web first assertions (`prefer-web-first-assertions`)
+
+Playwright supports many web first assertions to assert properties or conditions
+on elements. These assertions are preferred over instance methods as the web
+first assertions will automatically wait for the conditions to be fulfilled
+resulting in more resilient tests.
+
+## Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```javascript
+expect(await page.locator('.tweet').isVisible()).toBe(true);
+expect(await page.locator('.tweet').isEnabled()).toBe(true);
+expect(await page.locator('.tweet').innerText()).toBe('bar');
+```
+
+Example of **correct** code for this rule:
+
+```javascript
+await expect(page.locator('.tweet')).toBeVisible();
+await expect(page.locator('.tweet')).toBeEnabled();
+await expect(page.locator('.tweet')).toHaveText('bar');
+```

src/index.ts

@@ -17,6 +17,7 @@ import preferStrictEqual from './rules/prefer-strict-equal';
 import requireSoftAssertions from './rules/require-soft-assertions';
 import requireTopLevelDescribe from './rules/require-top-level-describe';
 import validExpect from './rules/valid-expect';
+import preferWebFirstAssertions from './rules/prefer-web-first-assertions';
 
 export = {
   configs: {
@@ -38,6 +39,7 @@ export = {
         'playwright/max-nested-describe': 'warn',
         'playwright/no-conditional-in-test': 'warn',
         'playwright/no-useless-not': 'warn',
+        'playwright/prefer-web-first-assertions': 'error',
         'playwright/valid-expect': 'error',
       },
     },
@@ -91,6 +93,7 @@ export = {
     'prefer-strict-equal': preferStrictEqual,
     'prefer-to-be': preferToBe,
     'prefer-to-have-length': preferToHaveLength,
+    'prefer-web-first-assertions': preferWebFirstAssertions,
     'require-top-level-describe': requireTopLevelDescribe,
     'require-soft-assertions': requireSoftAssertions,
     'valid-expect': validExpect,

src/rules/prefer-web-first-assertions.ts

@@ -0,0 +1,194 @@
+import { Rule } from 'eslint';
+import * as ESTree from 'estree';
+import { getRawValue, getStringValue, isBooleanLiteral } from '../utils/ast';
+import { parseExpectCall } from '../utils/parseExpectCall';
+
+type MethodConfig = {
+  type: 'string' | 'boolean';
+  matcher: string;
+  inverse?: string;
+};
+
+const methods: Record<string, MethodConfig> = {
+  innerText: { type: 'string', matcher: 'toHaveText' },
+  textContent: { type: 'string', matcher: 'toHaveText' },
+  inputValue: { type: 'string', matcher: 'toHaveValue' },
+  isEditable: { type: 'boolean', matcher: 'toBeEditable' },
+  isChecked: { type: 'boolean', matcher: 'toBeChecked' },
+  isDisabled: {
+    type: 'boolean',
+    matcher: 'toBeDisabled',
+    inverse: 'toBeEnabled',
+  },
+  isEnabled: {
+    type: 'boolean',
+    matcher: 'toBeEnabled',
+    inverse: 'toBeDisabled',
+  },
+  isHidden: {
+    type: 'boolean',
+    matcher: 'toBeHidden',
+    inverse: 'toBeVisible',
+  },
+  isVisible: {
+    type: 'boolean',
+    matcher: 'toBeVisible',
+    inverse: 'toBeHidden',
+  },
+  getAttribute: {
+    type: 'string',
+    matcher: 'toHaveAttribute',
+  },
+};
+
+const supportedMatchers = new Set([
+  'toBe',
+  'toEqual',
+  'toBeTruthy',
+  'toBeFalsy',
+]);
+
+export function getMatcherCall(node: ESTree.Node) {
+  const grandparent = (node as any).parent?.parent as ESTree.Node;
+  return grandparent.type === 'CallExpression' ? grandparent : undefined;
+}
+
+export default {
+  create(context) {
+    return {
+      CallExpression(node) {
+        const expectCall = parseExpectCall(node);
+        if (!expectCall) return;
+
+        const [arg] = node.arguments;
+        if (
+          arg.type !== 'AwaitExpression' ||
+          arg.argument.type !== 'CallExpression' ||
+          arg.argument.callee.type !== 'MemberExpression'
+        ) {
+          return;
+        }
+
+        // Matcher must be supported
+        if (!supportedMatchers.has(expectCall.matcherName)) return;
+
+        // Playwright method must be supported
+        const method = getStringValue(arg.argument.callee.property);
+        const methodConfig = methods[method];
+        if (!methodConfig) return;
+
+        const { callee } = arg.argument;
+        context.report({
+          messageId: 'useWebFirstAssertion',
+          suggest: [
+            {
+              messageId: 'suggestWebFirstAssertion',
+              fix: (fixer) => {
+                const methodArgs =
+                  arg.argument.type === 'CallExpression'
+                    ? arg.argument.arguments
+                    : [];
+
+                const methodEnd = methodArgs.length
+                  ? methodArgs[methodArgs.length - 1]!.range![1] + 1
+                  : callee.property.range![1] + 2;
+
+                const fixes = [
+                  // Add await to the expect call
+                  fixer.insertTextBefore(node, 'await '),
+                  // Remove the await keyword
+                  fixer.replaceTextRange(
+                    [arg.range![0], arg.argument.range![0]],
+                    ''
+                  ),
+                  // Remove the old Playwright method and any arguments
+                  fixer.replaceTextRange(
+                    [callee.property.range![0] - 1, methodEnd],
+                    ''
+                  ),
+                ];
+
+                // Change the matcher
+                const { args, matcher } = expectCall;
+                const notModifier = expectCall.modifiers.find(
+                  (mod) => getStringValue(mod) === 'not'
+                );
+
+                const isFalsy =
+                  methodConfig.type === 'boolean' &&
+                  ((!!args.length && isBooleanLiteral(args[0], false)) ||
+                    expectCall.matcherName === 'toBeFalsy');
+
+                const isInverse = methodConfig.inverse
+                  ? notModifier || isFalsy
+                  : notModifier && isFalsy;
+
+                // Remove not from matcher chain if no longer needed
+                if (isInverse && notModifier) {
+                  const notRange = notModifier.range!;
+                  fixes.push(fixer.removeRange([notRange[0], notRange[1] + 1]));
+                }
+
+                // Add not to the matcher chain if no inverse matcher exists
+                if (!methodConfig.inverse && !notModifier && isFalsy) {
+                  fixes.push(fixer.insertTextBefore(matcher, 'not.'));
+                }
+
+                // Replace the old matcher with the new matcher. The inverse
+                // matcher should only be used if the old statement was not a
+                // double negation.
+                const newMatcher =
+                  (+!!notModifier ^ +isFalsy && methodConfig.inverse) ||
+                  methodConfig.matcher;
+                fixes.push(fixer.replaceText(matcher, newMatcher));
+
+                // Remove boolean argument if it exists
+                const [matcherArg] = args ?? [];
+                if (matcherArg && isBooleanLiteral(matcherArg)) {
+                  fixes.push(fixer.remove(matcherArg));
+                }
+
+                // Add the new matcher arguments if needed
+                const hasOtherArgs = !!methodArgs.filter(
+                  (arg) => !isBooleanLiteral(arg)
+                ).length;
+
+                if (methodArgs) {
+                  const range = matcher.range!;
+                  const stringArgs = methodArgs
+                    .map((arg) => getRawValue(arg))
+                    .concat(hasOtherArgs ? '' : [])
+                    .join(', ');
+
+                  fixes.push(
+                    fixer.insertTextAfterRange(
+                      [range[0], range[1] + 1],
+                      stringArgs
+                    )
+                  );
+                }
+
+                return fixes;
+              },
+            },
+          ],
+          node,
+        });
+      },
+    };
+  },
+  meta: {
+    docs: {
+      category: 'Best Practices',
+      description: 'Prefer web first assertions',
+      recommended: true,
+      url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-web-first-assertions.md',
+    },
+    hasSuggestions: true,
+    messages: {
+      useWebFirstAssertion: 'Prefer web first assertions.',
+      suggestWebFirstAssertion: 'Replace {{method}} with {{matcher}}.',
+    },
+    type: 'suggestion',
+  },
+} as Rule.RuleModule;

src/utils/ast.ts

@@ -14,6 +14,10 @@ export function getStringValue(node: ESTree.Node | undefined) {
     : '';
 }
 
+export function getRawValue(node: ESTree.Node) {
+  return node.type === 'Literal' ? node.raw : undefined;
+}
+
 export function isIdentifier(node: ESTree.Node, name: string) {
   return node.type === 'Identifier' && node.name === name;
 }