Valid expect

README.md

@@ -49,15 +49,17 @@ 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-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-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`           |
+|  ✔  | 🔧  | 💡  | 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-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`                         |
+|  ✔  |     |     | [valid-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md)                         | Enforce valid `expect()` usage                                  |

docs/rules/valid-expect.md

@@ -0,0 +1,39 @@
+# Enforce valid `expect()` usage (`valid-expect`)
+
+Ensure `expect()` is called with a matcher.
+
+## Rule details
+
+Examples of **incorrect** code for this rule:
+
+```javascript
+expect();
+expect('something');
+expect(true).toBeDefined;
+```
+
+Example of **correct** code for this rule:
+
+```javascript
+expect(locator).toHaveText('howdy');
+expect('something').toBe('something');
+expect(true).toBeDefined();
+```
+
+## Options
+
+```json
+{
+  "minArgs": 1,
+  "maxArgs": 2
+}
+```
+
+### `minArgs` & `maxArgs`
+
+Enforces the minimum and maximum number of arguments that `expect` can take, and
+is required to take.
+
+`minArgs` defaults to 1 while `maxArgs` deafults to `2` to support custom expect
+messages. If you want to enforce `expect` always or never has a custom message,
+you can adjust these two option values to your preference.

src/index.ts

@@ -9,6 +9,7 @@ import noForceOption from './rules/no-force-option';
 import maxNestedDescribe from './rules/max-nested-describe';
 import noConditionalInTest from './rules/no-conditional-in-test';
 import noUselessNot from './rules/no-useless-not';
+import validExpect from './rules/valid-expect';
 
 export = {
   configs: {
@@ -30,6 +31,7 @@ export = {
         'playwright/max-nested-describe': 'warn',
         'playwright/no-conditional-in-test': 'warn',
         'playwright/no-useless-not': 'warn',
+        'playwright/valid-expect': 'error',
       },
     },
     'jest-playwright': {
@@ -77,5 +79,6 @@ export = {
     'max-nested-describe': maxNestedDescribe,
     'no-conditional-in-test': noConditionalInTest,
     'no-useless-not': noUselessNot,
+    'valid-expect': validExpect,
   },
 };

src/rules/valid-expect.ts

@@ -0,0 +1,103 @@
+import { Rule } from 'eslint';
+import { isExpectCall, isIdentifier } from '../utils/ast';
+import { NodeWithParent } from '../utils/types';
+
+function isMatcherFound(node: NodeWithParent): boolean {
+  if (node.parent.type !== 'MemberExpression') {
+    return false;
+  }
+
+  return !(
+    isIdentifier(node.parent.property, 'not') &&
+    node.parent.parent.type !== 'MemberExpression'
+  );
+}
+
+function isMatcherCalled(node: NodeWithParent): boolean {
+  return node.parent.type === 'MemberExpression'
+    ? // If the parent is a member expression, we continue traversing upward to
+      // handle matcher chains of unknown length. e.g. expect().not.something.
+      isMatcherCalled(node.parent)
+    : // Just asserting that the parent is a call expression is not enough as
+      // the node could be an argument of a call expression which doesn't
+      // determine if it is called. To determine if it is called, we verify
+      // that the parent call expression callee is the same as the node.
+      node.parent.type === 'CallExpression' && node.parent.callee === node;
+}
+
+const getAmountData = (amount: number) => ({
+  amount: amount.toString(),
+  s: amount === 1 ? '' : 's',
+});
+
+export default {
+  create(context) {
+    const options = {
+      minArgs: 1,
+      maxArgs: 2,
+      ...((context.options?.[0] as {}) ?? {}),
+    };
+
+    const minArgs = Math.min(options.minArgs, options.maxArgs);
+    const maxArgs = Math.max(options.minArgs, options.maxArgs);
+
+    return {
+      CallExpression(node) {
+        if (!isExpectCall(node)) return;
+
+        if (!isMatcherFound(node)) {
+          context.report({ node, messageId: 'matcherNotFound' });
+        } else if (!isMatcherCalled(node)) {
+          context.report({ node, messageId: 'matcherNotCalled' });
+        }
+
+        if (node.arguments.length < minArgs) {
+          context.report({
+            messageId: 'notEnoughArgs',
+            data: getAmountData(minArgs),
+            node,
+          });
+        }
+
+        if (node.arguments.length > maxArgs) {
+          context.report({
+            messageId: 'tooManyArgs',
+            data: getAmountData(maxArgs),
+            node,
+          });
+        }
+      },
+    };
+  },
+  meta: {
+    docs: {
+      category: 'Possible Errors',
+      description: 'Enforce valid `expect()` usage',
+      recommended: true,
+      url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md',
+    },
+    messages: {
+      tooManyArgs: 'Expect takes at most {{amount}} argument{{s}}.',
+      notEnoughArgs: 'Expect requires at least {{amount}} argument{{s}}.',
+      matcherNotFound: 'Expect must have a corresponding matcher call.',
+      matcherNotCalled: 'Matchers must be called to assert.',
+    },
+    type: 'problem',
+    schema: [
+      {
+        type: 'object',
+        properties: {
+          minArgs: {
+            type: 'number',
+            minimum: 1,
+          },
+          maxArgs: {
+            type: 'number',
+            minimum: 1,
+          },
+        },
+        additionalProperties: false,
+      },
+    ],
+  },
+} as Rule.RuleModule;

src/utils/ast.ts

@@ -120,3 +120,14 @@ export function isTest(node: ESTree.CallExpression) {
     )
   );
 }
+
+const expectSubCommands = new Set(['soft', 'poll']);
+export function isExpectCall(node: ESTree.CallExpression) {
+  return (
+    isIdentifier(node.callee, 'expect') ||
+    (node.callee.type === 'MemberExpression' &&
+      isIdentifier(node.callee.object, 'expect') &&
+      node.callee.property.type === 'Identifier' &&
+      expectSubCommands.has(node.callee.property.name))
+  );
+}

src/utils/types.ts

@@ -0,0 +1,4 @@
+import { Rule } from 'eslint';
+import * as ESTree from 'estree';
+
+export type NodeWithParent = ESTree.Node & Rule.NodeParentExtension;

test/spec/valid-expect.spec.ts

@@ -0,0 +1,93 @@
+import { runRuleTester } from '../utils/rule-tester';
+import rule from '../../src/rules/valid-expect';
+
+const invalid = (code: string, messageId: string) => ({
+  code,
+  errors: [{ messageId }],
+});
+
+runRuleTester('valid-expect', rule, {
+  valid: [
+    'expect("something").toBe("else")',
+    'expect.soft("something").toBe("else")',
+    'expect.poll(() => "something").toBe("else")',
+    'expect(true).toBeDefined()',
+    'expect(undefined).not.toBeDefined()',
+    'expect([1, 2, 3]).toEqual([1, 2, 3])',
+    'expect(1, "1 !== 2").toBe(2)',
+    'expect.soft(1, "1 !== 2").toBe(2)',
+    'expect.poll(() => 1, { message: "1 !== 2" }).toBe(2)',
+    // minArgs
+    {
+      code: 'expect(1, "1 !== 2").toBe(2)',
+      options: [{ minArgs: 2 }],
+    },
+    {
+      code: 'expect(1, 2, 3).toBe(4)',
+      options: [{ minArgs: 3 }],
+    },
+    // maxArgs
+    {
+      code: 'expect(1).toBe(2)',
+      options: [{ maxArgs: 1 }],
+    },
+    {
+      code: 'expect(1, 2, 3).toBe(4)',
+      options: [{ maxArgs: 3 }],
+    },
+  ],
+  invalid: [
+    // Matcher not found
+    invalid('expect(foo)', 'matcherNotFound'),
+    invalid('expect(foo).not', 'matcherNotFound'),
+    invalid('expect.soft(foo)', 'matcherNotFound'),
+    invalid('expect.soft(foo).not', 'matcherNotFound'),
+    invalid('expect.poll(foo)', 'matcherNotFound'),
+    invalid('expect.poll(foo).not', 'matcherNotFound'),
+    // Matcher not called
+    invalid('expect(foo).toBe', 'matcherNotCalled'),
+    invalid('expect(foo).not.toBe', 'matcherNotCalled'),
+    invalid('something(expect(foo).not.toBe)', 'matcherNotCalled'),
+    invalid('expect.soft(foo).toBe', 'matcherNotCalled'),
+    invalid('expect.soft(foo).not.toBe', 'matcherNotCalled'),
+    invalid('something(expect.soft(foo).not.toBe)', 'matcherNotCalled'),
+    invalid('expect.poll(() => foo).toBe', 'matcherNotCalled'),
+    invalid('expect.poll(() => foo).not.toBe', 'matcherNotCalled'),
+    invalid('something(expect.poll(() => foo).not.toBe)', 'matcherNotCalled'),
+    // minArgs
+    {
+      code: 'expect().toBe(true)',
+      errors: [{ messageId: 'notEnoughArgs', data: { amount: 1, s: '' } }],
+    },
+    {
+      code: 'expect(foo).toBe(true)',
+      options: [{ minArgs: 2 }],
+      errors: [{ messageId: 'notEnoughArgs', data: { amount: 2, s: 's' } }],
+    },
+    // maxArgs
+    {
+      code: 'expect(foo, "bar", "baz").toBe(true)',
+      errors: [{ messageId: 'tooManyArgs', data: { amount: 2, s: 's' } }],
+    },
+    {
+      code: 'expect(foo, "bar").toBe(true)',
+      options: [{ maxArgs: 1 }],
+      errors: [{ messageId: 'tooManyArgs', data: { amount: 1, s: '' } }],
+    },
+    // Multiple errors
+    {
+      code: 'expect()',
+      errors: [
+        { messageId: 'matcherNotFound' },
+        { messageId: 'notEnoughArgs', data: { amount: 1, s: '' } },
+      ],
+    },
+    {
+      code: 'expect().toHaveText',
+      errors: [
+        { messageId: 'matcherNotCalled' },
+        { messageId: 'notEnoughArgs', data: { amount: 1, s: '' } },
+      ],
+    },
+  ],
+});