feat: Add no-standalone-expect rule (#223)

Author: mskelton

README.md

@@ -138,6 +138,7 @@ command line option.\
 | ✔  | 🔧  |     | [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              |
 |     |     |     | [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-standalone-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-standalone-expect.md)               | Disallow using expect outside of `test` blocks                    |
 | ✔  | 🔧  |     | [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-selector](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-selector.md)               | Disallow usage of `page.waitForSelector()`                        |
 | ✔  |     | 💡  | [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()`                         |

docs/rules/no-standalone-expect.md

@@ -0,0 +1,68 @@
+# Disallow using `expect` outside of `test` blocks (`no-standalone-expect`)
+
+Prevents `expect` statements outside of a `test` block. An `expect` within a
+helper function (but outside of a `test` block) will not trigger this rule.
+
+## Rule details
+
+This rule aims to eliminate `expect` statements that will not be executed. An
+`expect` inside of a `describe` block but outside of a `test` block or outside a
+`test.describe` will not execute and therefore will trigger this rule. It is
+viable, however, to have an `expect` in a helper function that is called from
+within a `test` block so `expect` statements in a function will not trigger this
+rule.
+
+Statements like `expect.hasAssertions()` will NOT trigger this rule since these
+calls will execute if they are not in a test block.
+
+Examples of **incorrect** code for this rule:
+
+```js
+// in describe
+test.describe('a test', () => {
+  expect(1).toBe(1);
+});
+
+// below other tests
+test.describe('a test', () => {
+  test('an it', () => {
+    expect(1).toBe(1);
+  });
+
+  expect(1).toBe(1);
+});
+```
+
+Examples of **correct** code for this rule:
+
+```js
+// in it block
+test.describe('a test', () => {
+  test('an it', () => {
+    expect(1).toBe(1);
+  });
+});
+
+// in helper function
+test.describe('a test', () => {
+  const helper = () => {
+    expect(1).toBe(1);
+  };
+
+  test('an it', () => {
+    helper();
+  });
+});
+
+test.describe('a test', () => {
+  expect.hasAssertions(1);
+});
+```
+
+_Note that this rule will not trigger if the helper function is never used even
+though the `expect` will not execute. Rely on a rule like no-unused-vars for
+this case._
+
+## When Not To Use It
+
+Don't use this rule on non-playwright test files.

src/index.ts

@@ -16,6 +16,7 @@ import noPagePause from './rules/no-page-pause';
 import noRawLocators from './rules/no-raw-locators';
 import noRestrictedMatchers from './rules/no-restricted-matchers';
 import noSkippedTest from './rules/no-skipped-test';
+import noStandaloneExpect from './rules/no-standalone-expect';
 import noUnsafeReferences from './rules/no-unsafe-references';
 import noUselessAwait from './rules/no-useless-await';
 import noUselessNot from './rules/no-useless-not';
@@ -53,6 +54,7 @@ const index = {
     'no-raw-locators': noRawLocators,
     'no-restricted-matchers': noRestrictedMatchers,
     'no-skipped-test': noSkippedTest,
+    'no-standalone-expect': noStandaloneExpect,
     'no-unsafe-references': noUnsafeReferences,
     'no-useless-await': noUselessAwait,
     'no-useless-not': noUselessNot,
@@ -87,6 +89,7 @@ const sharedConfig = {
     'playwright/no-networkidle': 'error',
     'playwright/no-page-pause': 'warn',
     'playwright/no-skipped-test': 'warn',
+    'playwright/no-standalone-expect': 'error',
     'playwright/no-unsafe-references': 'error',
     'playwright/no-useless-await': 'warn',
     'playwright/no-useless-not': 'warn',

src/rules/no-standalone-expect.ts

@@ -0,0 +1,129 @@
+import { Rule } from 'eslint';
+import * as ESTree from 'estree';
+import {
+  findParent,
+  getExpectType,
+  getParent,
+  getStringValue,
+  isDescribeCall,
+  isFunction,
+  isIdentifier,
+  isTestCall,
+} from '../utils/ast';
+
+const getBlockType = (
+  statement: ESTree.BlockStatement,
+): 'function' | 'describe' | null => {
+  const func = getParent(statement);
+
+  if (!func) {
+    throw new Error(
+      `Unexpected BlockStatement. No parent defined. - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
+    );
+  }
+
+  // functionDeclaration: function func() {}
+  if (func.type === 'FunctionDeclaration') {
+    return 'function';
+  }
+
+  if (isFunction(func) && func.parent) {
+    const expr = func.parent;
+
+    // arrow function or function expr
+    if (expr.type === 'VariableDeclarator') {
+      return 'function';
+    }
+
+    // if it's not a variable, it will be callExpr, we only care about describe
+    if (expr.type === 'CallExpression' && isDescribeCall(expr)) {
+      return 'describe';
+    }
+  }
+
+  return null;
+};
+
+type BlockType = 'arrow' | 'describe' | 'function' | 'template' | 'test';
+
+export default {
+  create(context: Rule.RuleContext) {
+    const callStack: BlockType[] = [];
+
+    return {
+      ArrowFunctionExpression(node) {
+        if (node.parent?.type !== 'CallExpression') {
+          callStack.push('arrow');
+        }
+      },
+      'ArrowFunctionExpression:exit'() {
+        if (callStack[callStack.length - 1] === 'arrow') {
+          callStack.pop();
+        }
+      },
+
+      BlockStatement(statement) {
+        const blockType = getBlockType(statement);
+
+        if (blockType) {
+          callStack.push(blockType);
+        }
+      },
+      'BlockStatement:exit'(statement: ESTree.BlockStatement) {
+        if (callStack[callStack.length - 1] === getBlockType(statement)) {
+          callStack.pop();
+        }
+      },
+
+      CallExpression(node) {
+        if (getExpectType(context, node)) {
+          const parent = callStack.at(-1);
+
+          if (!parent || parent === 'describe') {
+            const root = findParent(node, 'CallExpression');
+            context.report({
+              messageId: 'unexpectedExpect',
+              node: root ?? node,
+            });
+          }
+
+          return;
+        }
+
+        if (isTestCall(context, node)) {
+          callStack.push('test');
+        }
+
+        if (node.callee.type === 'TaggedTemplateExpression') {
+          callStack.push('template');
+        }
+      },
+      'CallExpression:exit'(node: ESTree.CallExpression) {
+        const top = callStack[callStack.length - 1];
+
+        if (
+          (top === 'test' &&
+            isTestCall(context, node) &&
+            node.callee.type !== 'MemberExpression') ||
+          (top === 'template' &&
+            node.callee.type === 'TaggedTemplateExpression')
+        ) {
+          callStack.pop();
+        }
+      },
+    };
+  },
+  meta: {
+    docs: {
+      category: 'Best Practices',
+      description: 'Disallow using `expect` outside of `test` blocks',
+      recommended: false,
+      url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-contain.md',
+    },
+    fixable: 'code',
+    messages: {
+      unexpectedExpect: 'Expect must be inside of a test block',
+    },
+    type: 'suggestion',
+  },
+} as Rule.RuleModule;

src/utils/ast.ts

@@ -132,9 +132,7 @@ export function isTestCall(
       !modifiers ||
       modifiers?.includes(getStringValue(node.callee.property))) &&
     node.arguments.length === 2 &&
-    ['ArrowFunctionExpression', 'FunctionExpression'].includes(
-      node.arguments[1].type,
-    )
+    isFunction(node.arguments[1])
   );
 }
 
@@ -218,10 +216,14 @@ export function isPageMethod(node: ESTree.CallExpression, name: string) {
   );
 }
 
+export type FunctionExpression = (
+  | ESTree.ArrowFunctionExpression
+  | ESTree.FunctionExpression
+) &
+  Rule.NodeParentExtension;
+
 /** Returns a boolean to indicate if the node is a function or arrow function */
-export function isFunction(
-  node: ESTree.Node,
-): node is ESTree.FunctionExpression | ESTree.ArrowFunctionExpression {
+export function isFunction(node: ESTree.Node): node is FunctionExpression {
   return (
     node.type === 'ArrowFunctionExpression' ||
     node.type === 'FunctionExpression'

test/spec/no-standalone-expect.spec.ts

@@ -0,0 +1,91 @@
+import dedent from 'dedent';
+import rule from '../../src/rules/no-standalone-expect';
+import { runRuleTester } from '../utils/rule-tester';
+
+const messageId = 'unexpectedExpect';
+
+runRuleTester('no-standalone-expect', rule, {
+  invalid: [
+    {
+      code: "(() => {})('testing', () => expect(true).toBe(false))",
+      errors: [{ column: 29, endColumn: 53, messageId }],
+    },
+    {
+      code: dedent`
+        test.describe('scenario', () => {
+          const t = Math.random() ? test.only : test;
+          t('testing', () => expect(true).toBe(false));
+        });
+      `,
+      errors: [{ column: 22, endColumn: 46, messageId }],
+    },
+    {
+      code: dedent`
+        it.describe('scenario', () => {
+          it('testing', () => expect(true).toBe(false));
+        });
+      `,
+      errors: [{ column: 23, endColumn: 47, messageId }],
+    },
+    {
+      code: 'test.describe("a test", () => { expect(1).toBe(1); });',
+      errors: [{ column: 33, endColumn: 50, messageId }],
+    },
+    {
+      code: 'test.describe("a test", () => expect(6).toBe(1));',
+      errors: [{ column: 31, endColumn: 48, messageId }],
+    },
+    {
+      code: 'test.describe("a test", () => { const func = () => { expect(6).toBe(1); }; expect(1).toBe(1); });',
+      errors: [{ column: 76, endColumn: 93, messageId }],
+    },
+    {
+      code: 'test.describe("a test", () => {  test("foo", () => { expect(1).toBe(1); }); expect(1).toBe(1); });',
+      errors: [{ column: 77, endColumn: 94, messageId }],
+    },
+    {
+      code: 'expect(1).toBe(1);',
+      errors: [{ column: 1, endColumn: 18, messageId }],
+    },
+    {
+      code: '{expect(1).toBe(1)}',
+      errors: [{ column: 2, endColumn: 19, messageId }],
+    },
+    {
+      code: dedent`
+        import { expect as pleaseExpect } from '@playwright/test';
+        test.describe("a test", () => { pleaseExpect(1).toBe(1); });
+      `,
+      errors: [{ column: 33, endColumn: 56, messageId }],
+      parserOptions: { sourceType: 'module' },
+    },
+  ],
+  valid: [
+    'expect.any(String)',
+    'expect.extend({})',
+    'test.describe("a test", () => { test("an it", () => {expect(1).toBe(1); }); });',
+    'test.describe("a test", () => { test("an it", () => { const func = () => { expect(1).toBe(1); }; }); });',
+    'test.describe("a test", () => { const func = () => { expect(1).toBe(1); }; });',
+    'test.describe("a test", () => { function func() { expect(1).toBe(1); }; });',
+    'test.describe("a test", () => { const func = function(){ expect(1).toBe(1); }; });',
+    'test("an it", () => expect(1).toBe(1))',
+    'const func = function(){ expect(1).toBe(1); };',
+    'const func = () => expect(1).toBe(1);',
+    '{}',
+    'test.only("an only", value => { expect(value).toBe(true); });',
+    'test.concurrent("an concurrent", value => { expect(value).toBe(true); });',
+    // Global aliases
+    {
+      code: dedent`
+        it.describe('scenario', () => {
+          it('testing', () => expect(true));
+        });
+      `,
+      settings: {
+        playwright: {
+          globalAliases: { test: ['it'] },
+        },
+      },
+    },
+  ],
+});