From e2128a1eb2a9990b88b6a0b540075aae38653b76 Mon Sep 17 00:00:00 2001 From: David Enke Date: Mon, 25 Aug 2025 22:38:57 +0200 Subject: [PATCH 1/2] feat: add new rule for consistent spacing between test blocks, like `test()`, `test.step()` and so on --- src/index.ts | 2 + .../consistent-spacing-between-blocks.test.ts | 128 +++++++++++++ .../consistent-spacing-between-blocks.ts | 169 ++++++++++++++++++ src/utils/test-expression.ts | 83 +++++++++ 4 files changed, 382 insertions(+) create mode 100644 src/rules/consistent-spacing-between-blocks.test.ts create mode 100644 src/rules/consistent-spacing-between-blocks.ts create mode 100644 src/utils/test-expression.ts diff --git a/src/index.ts b/src/index.ts index 0101521..88a373f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,5 @@ import globals from 'globals' +import consistentSpacingBetweenBlocks from './rules/consistent-spacing-between-blocks.js' import expectExpect from './rules/expect-expect.js' import maxExpects from './rules/max-expects.js' import maxNestedDescribe from './rules/max-nested-describe.js' @@ -54,6 +55,7 @@ import validTitle from './rules/valid-title.js' const index = { configs: {}, rules: { + 'consistent-spacing-between-blocks': consistentSpacingBetweenBlocks, 'expect-expect': expectExpect, 'max-expects': maxExpects, 'max-nested-describe': maxNestedDescribe, diff --git a/src/rules/consistent-spacing-between-blocks.test.ts b/src/rules/consistent-spacing-between-blocks.test.ts new file mode 100644 index 0000000..904949f --- /dev/null +++ b/src/rules/consistent-spacing-between-blocks.test.ts @@ -0,0 +1,128 @@ +import { javascript, runRuleTester } from '../utils/rule-tester.js' +// some tests import from `../src/`, looks like a tooling issue; here, +// the template used is `prefer-lowercase-title` and its tests +import rule from './consistent-spacing-between-blocks.js' + +runRuleTester('consistent-spacing-between-blocks', rule, { + invalid: [ + { + code: javascript` + test.beforeEach('Test 1', () => {}); + test('Test 2', async () => { + await test.step('Step 1', () => {}); + // a comment + test.step('Step 2', () => {}); + test.step('Step 3', () => {}); + const foo = await test.step('Step 4', () => {}); + foo = await test.step('Step 5', () => {}); + }); + /** + * another comment + */ + test('Test 6', () => {}); + `, + errors: [ + { messageId: 'missingWhitespace' }, + { messageId: 'missingWhitespace' }, + { messageId: 'missingWhitespace' }, + { messageId: 'missingWhitespace' }, + { messageId: 'missingWhitespace' }, + { messageId: 'missingWhitespace' }, + ], + name: 'missing blank lines before test blocks', + output: javascript` + test.beforeEach('Test 1', () => {}); + + test('Test 2', async () => { + await test.step('Step 1', () => {}); + + // a comment + test.step('Step 2', () => {}); + + test.step('Step 3', () => {}); + + const foo = await test.step('Step 4', () => {}); + + foo = await test.step('Step 5', () => {}); + }); + + /** + * another comment + */ + test('Test 6', () => {}); + `, + }, + ], + valid: [ + { + code: javascript` + test('Test 1', () => {}); + + test('Test 2', () => {}); + `, + name: 'blank line between simple test blocks', + }, + { + code: javascript` + test.beforeEach(() => {}); + + test.skip('Test 2', () => {}); + `, + name: 'blank line between test modifiers', + }, + { + code: javascript` + test('Test', async () => { + await test.step('Step 1', () => {}); + + await test.step('Step 2', () => {}); + }); + `, + name: 'blank line between nested steps in async test', + }, + { + code: javascript` + test('Test', async () => { + await test.step('Step 1', () => {}); + + // some comment + await test.step('Step 2', () => {}); + }); + `, + name: 'nested steps with a line comment in between', + }, + { + code: javascript` + test('Test', async () => { + await test.step('Step 1', () => {}); + + /** + * another comment + */ + await test.step('Step 2', () => {}); + }); + `, + name: 'nested steps with a block comment in between', + }, + { + code: javascript` + test('assign', async () => { + let foo = await test.step('Step 1', () => {}); + + foo = await test.step('Step 2', () => {}); + }); + `, + name: 'assignments initialized by test.step', + }, + { + code: javascript` + test('assign', async () => { + let { foo } = await test.step('Step 1', () => {}); + + ({ foo } = await test.step('Step 2', () => {})); + }); + `, + name: 'destructuring assignments initialized by test.step', + }, + ], +}) diff --git a/src/rules/consistent-spacing-between-blocks.ts b/src/rules/consistent-spacing-between-blocks.ts new file mode 100644 index 0000000..2c3dd24 --- /dev/null +++ b/src/rules/consistent-spacing-between-blocks.ts @@ -0,0 +1,169 @@ +import type { AST } from 'eslint' +import type { Comment, Expression, Node } from 'estree' +import { createRule } from '../utils/createRule.js' +import { isTestExpression, unwrapExpression } from '../utils/test-expression.js' + +/** + * An ESLint rule that ensures consistent spacing between test blocks (e.g. + * `test`, `test.step`, `test.beforeEach`, etc.). This rule helps improve the + * readability and maintainability of test code by ensuring that test blocks are + * clearly separated from each other. + */ +export default createRule({ + create(context) { + /** + * Recursively determines the previous token (if present) and, if necessary, + * a stand-in token to check spacing against. Therefore, the current start + * token can optionally be passed through and used as the comparison token. + * + * Returns the previous token that is not a comment or a grouping expression + * (`previous`), the first token to compare (`start`), and the actual token + * being examined (`origin`). + * + * If there is no previous token for the expression, `null` is returned for + * it. Ideally, the first comparable token is the same as the actual token. + * + * | 1 | test('foo', async () => { + * previous > | 2 | await test.step(...); + * | 3 | + * start > | 4 | // Erster Kommentar + * | 5 | // weiterer Kommentar + * origin > | 6 | await test.step(...); + */ + function getPreviousToken( + node: AST.Token | Node, + start?: AST.Token | Comment | Node, + ): { + /** The token actually being checked */ + origin: AST.Token | Node + + /** + * The previous token that is neither a comment nor a grouping expression, + * if present + */ + previous: AST.Token | null + + /** + * The first token used for comparison, e.g. the start of the test + * expression + */ + start: AST.Token | Comment | Node + } { + const current = start ?? node + const previous = context.sourceCode.getTokenBefore(current, { + includeComments: true, + }) + + // no predecessor present + if ( + previous === null || + previous === undefined || + previous.value === '{' + ) { + return { + origin: node, + previous: null, + start: current, + } + } + + // Recursively traverse comments and determine a stand-in + // and unwrap parenthesized expressions + if ( + previous.type === 'Line' || // line comment + previous.type === 'Block' || // block comment + previous.value === '(' // grouping operator + ) { + return getPreviousToken(node, previous) + } + + // Return result + return { + origin: node, + previous: previous as AST.Token, + start: current, + } + } + + /** + * Checks whether the spacing before the given test block meets + * expectations. Optionally an offset token can be provided to check + * against, for example in the case of an assignment. + * + * @param node - The node to be checked. + * @param offset - Optional offset token to check spacing against. + */ + function checkSpacing(node: Expression, offset?: AST.Token | Node) { + const { previous, start } = getPreviousToken(node, offset) + + // First expression or no previous token + if (previous === null) return + + // Ignore when there is one or more blank lines between + if (previous.loc.end.line < start.loc!.start.line - 1) { + return + } + + // Since the hint in the IDE may not appear on the affected test expression + // but possibly on the preceding comment, include the test expression in the message + const source = context.sourceCode.getText(unwrapExpression(node)) + + context.report({ + data: { source }, + fix(fixer) { + return fixer.insertTextAfter(previous, '\n') + }, + loc: { + end: { + column: start.loc!.start.column, + line: start.loc!.start.line, + }, + start: { + column: 0, + line: previous.loc.end.line + 1, + }, + }, + messageId: 'missingWhitespace', + node, + }) + } + + return { + // Checks call expressions that could be test steps, + // e.g. `test(...)`, `test.step(...)`, or `await test.step(...)`, but also `foo = test(...)` + ExpressionStatement(node) { + if (isTestExpression(context, node.expression)) { + checkSpacing(node.expression) + } + }, + // Checks declarations that might be initialized from return values of test steps, + // e.g. `let result = await test(...)` or `const result = await test.step(...)` + VariableDeclaration(node) { + node.declarations.forEach((declaration) => { + if (declaration.init && isTestExpression(context, declaration.init)) { + // When declaring a variable, our examined test expression is used for initialization. + // Therefore, to check spacing we use the keyword token (let, const, var) before it: + // 1 | const foo = test('foo', () => {}); + // 2 | ^ + const offset = context.sourceCode.getTokenBefore(declaration) + checkSpacing(declaration.init, offset ?? undefined) + } + }) + }, + } + }, + meta: { + docs: { + description: + 'Enforces a blank line between Playwright test blocks (e.g., test, test.step, test.beforeEach, etc.).', + recommended: true, + }, + fixable: 'whitespace', + messages: { + missingWhitespace: + "A blank line is required before the test block '{{source}}'.", + }, + schema: [], + type: 'layout', + }, +}) diff --git a/src/utils/test-expression.ts b/src/utils/test-expression.ts new file mode 100644 index 0000000..dd7192d --- /dev/null +++ b/src/utils/test-expression.ts @@ -0,0 +1,83 @@ +import type { Rule } from 'eslint' +import type { Expression } from 'estree' +import { parseFnCall } from '../utils/parseFnCall.js' + +/** + * Unwraps a given expression to get the actual expression being called. This is + * useful when checking the final expression specifically. + * + * This handles: + * + * - Assignment expressions like `result = test(...)` + * - Async calls like `await test(...)` + * - Chained function calls like `test.step().then(...)` + * + * @param node - The expression to unwrap + * @returns The unwrapped expression + */ +export function unwrapExpression(node: Expression): Expression { + // Resolve assignments to get the actual expression + if (node.type === 'AssignmentExpression') { + return unwrapExpression(node.right) + } + + // Resolve async await expressions + if (node.type === 'AwaitExpression') { + return unwrapExpression(node.argument) + } + + // Traverse chains recursively to find the actual call + if ( + node.type === 'CallExpression' && + node.callee.type === 'MemberExpression' && + node.callee.object.type === 'CallExpression' + ) { + return unwrapExpression(node.callee.object) + } + + return node +} + +/** + * Checks if a given expression is a test-related call. A test call is a call to + * `test(...)` or one of its methods like `test.step(...)`. + * + * If the expression is chained, the calls are recursively traced back to find + * the actual call. This also handles assignments and async calls with `await`. + * + * @param context - The ESLint rule context + * @param node - The expression to check + * @param methods - Optional list of specific methods to check for + * @returns Whether it's a test block call + */ +export function isTestExpression( + context: Rule.RuleContext, + node: Expression, + methods?: string[], +): boolean { + // Unwrap the actual expression to check the call + const unwrapped = unwrapExpression(node) + + // Must be a call expression to be a test call + if (unwrapped.type !== 'CallExpression') { + return false + } + + // Use the existing parseFnCall to identify test-related calls + const call = parseFnCall(context, unwrapped) + if (!call) { + return false + } + + // If specific methods are requested, check if it's one of them + if (methods !== undefined) { + return ( + call.type === 'step' || + call.type === 'hook' || + (call.type === 'test' && methods.includes('test')) + ) + } + + // Check if it's any test-related call + return ['test', 'step', 'hook'].includes(call.type) +} From 3b6154f862ca0b0ad2b8e0861c8a144cfe3b7cee Mon Sep 17 00:00:00 2001 From: David Enke Date: Mon, 25 Aug 2025 22:49:28 +0200 Subject: [PATCH 2/2] docs: add readme for consistent spacing rule --- README.md | 1 + .../consistent-spacing-between-blocks.md | 54 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 docs/rules/consistent-spacing-between-blocks.md diff --git a/README.md b/README.md index dae8f3a..18f7f5f 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,7 @@ CLI option\ | Rule | Description | ✅ | 🔧 | 💡 | | --------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------ | :-: | :-: | :-: | +| [consistent-spacing-between-blocks](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/consistent-spacing-between-blocks.md) | Enforce consistent spacing between test blocks | ✅ | 🔧 | | | [expect-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | ✅ | | | | [max-expects](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/max-expects.md) | Enforces a maximum number assertion calls in a test body | | | | | [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 | ✅ | | | diff --git a/docs/rules/consistent-spacing-between-blocks.md b/docs/rules/consistent-spacing-between-blocks.md new file mode 100644 index 0000000..cf00160 --- /dev/null +++ b/docs/rules/consistent-spacing-between-blocks.md @@ -0,0 +1,54 @@ +# Enforce consistent spacing between test blocks (`enforce-consistent-spacing-between-blocks`) + +Ensure that there is a consistent spacing between test blocks. + +## Rule Details + +Examples of **incorrect** code for this rule: + +```javascript +test('example 1', () => { + expect(true).toBe(true) +}) +test('example 2', () => { + expect(true).toBe(true) +}) +``` + +```javascript +test.beforeEach(() => {}) +test('example 3', () => { + await test.step('first', async () => { + expect(true).toBe(true) + }) + await test.step('second', async () => { + expect(true).toBe(true) + }) +}) +``` + +Examples of **correct** code for this rule: + +```javascript +test('example 1', () => { + expect(true).toBe(true) +}) + +test('example 2', () => { + expect(true).toBe(true) +}) +``` + +```javascript +test.beforeEach(() => {}) + +test('example 3', () => { + await test.step('first', async () => { + expect(true).toBe(true) + }) + + await test.step('second', async () => { + expect(true).toBe(true) + }) +}) +```