Merge pull request #138 from hikimochi/no-nested-step

Add `no-nested-step` rule

Author: mskelton

README.md

@@ -64,6 +64,7 @@ command line option.\
 |  ✔  |     |     | [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-nested-step](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-nested-step.md)                           | Disallow nested `test.step()` methods                             |
 |  ✔  |     |     | [no-networkidle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-networkidle.md)                           | Disallow usage of the `networkidle` 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-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              |

docs/rules/no-nested-step.md

@@ -0,0 +1,30 @@
+# Disallow nested `test.step()` methods (`no-nested-step`)
+
+Nesting `test.step()` methods can make your tests difficult to read.
+
+## Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```javascript
+test('foo', async () => {
+  await test.step('step1', async () => {
+    await test.step('nest step', async () => {
+      await expect(true).toBe(true);
+    });
+  });
+});
+```
+
+Examples of **correct** code for this rule:
+
+```javascript
+test('foo', async () => {
+  await test.step('step1', async () => {
+    await expect(true).toBe(true);
+  });
+  await test.step('step2', async () => {
+    await expect(true).toBe(true);
+  });
+});
+```

src/index.ts

@@ -5,6 +5,7 @@ import noElementHandle from './rules/no-element-handle';
 import noEval from './rules/no-eval';
 import noFocusedTest from './rules/no-focused-test';
 import noForceOption from './rules/no-force-option';
+import noNestedStep from './rules/no-nested-step';
 import noNetworkidle from './rules/no-networkidle';
 import noPagePause from './rules/no-page-pause';
 import noRestrictedMatchers from './rules/no-restricted-matchers';
@@ -35,6 +36,7 @@ const recommended = {
     'playwright/no-eval': 'warn',
     'playwright/no-focused-test': 'error',
     'playwright/no-force-option': 'warn',
+    'playwright/no-nested-step': 'warn',
     'playwright/no-networkidle': 'error',
     'playwright/no-page-pause': 'warn',
     'playwright/no-skipped-test': 'warn',
@@ -91,6 +93,7 @@ export = {
     'no-eval': noEval,
     'no-focused-test': noFocusedTest,
     'no-force-option': noForceOption,
+    'no-nested-step': noNestedStep,
     'no-networkidle': noNetworkidle,
     'no-page-pause': noPagePause,
     'no-restricted-matchers': noRestrictedMatchers,

src/rules/no-nested-step.ts

@@ -0,0 +1,62 @@
+import { Rule } from 'eslint';
+import * as ESTree from 'estree';
+import { isPropertyAccessor } from '../utils/ast';
+
+function isStepCall(node: ESTree.Node): boolean {
+  const inner = node.type === 'CallExpression' ? node.callee : node;
+
+  if (inner.type !== 'MemberExpression') {
+    return false;
+  }
+
+  return isPropertyAccessor(inner, 'step');
+}
+
+export default {
+  create(context) {
+    const stack: number[] = [];
+
+    function pushStepCallback(node: Rule.Node) {
+      if (node.parent.type !== 'CallExpression' || !isStepCall(node.parent)) {
+        return;
+      }
+
+      stack.push(0);
+
+      if (stack.length > 1) {
+        context.report({
+          messageId: 'noNestedStep',
+          node: node.parent.callee,
+        });
+      }
+    }
+
+    function popStepCallback(node: Rule.Node) {
+      const { parent } = node;
+
+      if (parent.type === 'CallExpression' && isStepCall(parent)) {
+        stack.pop();
+      }
+    }
+
+    return {
+      ArrowFunctionExpression: pushStepCallback,
+      'ArrowFunctionExpression:exit': popStepCallback,
+      FunctionExpression: pushStepCallback,
+      'FunctionExpression:exit': popStepCallback,
+    };
+  },
+  meta: {
+    docs: {
+      category: 'Best Practices',
+      description: 'Disallow nested `test.step()` methods',
+      recommended: true,
+      url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-nested-step.md',
+    },
+    messages: {
+      noNestedStep: 'Do not nest `test.step()` methods.',
+    },
+    schema: [],
+    type: 'problem',
+  },
+} as Rule.RuleModule;

test/spec/no-nested-step.spec.ts

@@ -0,0 +1,72 @@
+import rule from '../../src/rules/no-nested-step';
+import { runRuleTester } from '../utils/rule-tester';
+import dedent = require('dedent');
+
+const messageId = 'noNestedStep';
+
+runRuleTester('max-nested-step', rule, {
+  invalid: [
+    {
+      code: dedent`
+      test('foo', async () => {
+        await test.step("step1", async () => {
+          await test.step("nested step1", async () => {
+            await expect(true).toBe(true);
+          });
+        });
+      });
+      `,
+      errors: [{ column: 11, endColumn: 20, endLine: 3, line: 3, messageId }],
+    },
+    {
+      code: dedent`
+      test('foo', async () => {
+        await test.step("step1", async () => {
+          await test.step("nested step1", async () => {
+            await expect(true).toBe(true);
+          });
+          await test.step("nested step1", async () => {
+            await expect(true).toBe(true);
+          });
+        });
+      });
+      `,
+      errors: [
+        { column: 11, endColumn: 20, endLine: 3, line: 3, messageId },
+        { column: 11, endColumn: 20, endLine: 6, line: 6, messageId },
+      ],
+    },
+  ],
+  valid: [
+    'await test.step("step1", () => {});',
+    'await test.step("step1", async () => {});',
+    {
+      code: dedent`
+        test('foo', async () => {
+          await expect(true).toBe(true);
+        });
+      `,
+    },
+    {
+      code: dedent`
+        test('foo', async () => {
+          await test.step("step1", async () => {
+            await expect(true).toBe(true);
+          });
+        });
+      `,
+    },
+    {
+      code: dedent`
+        test('foo', async () => {
+          await test.step("step1", async () => {
+            await expect(true).toBe(true);
+          });
+          await test.step("step2", async () => {
+            await expect(true).toBe(true);
+          });
+        });
+      `,
+    },
+  ],
+});