From 247cd5562404ed11a67036b6a5b80778e1edda5c Mon Sep 17 00:00:00 2001 From: Michal Sroczynski <93290723+MSroczynski3@users.noreply.github.com> Date: Wed, 13 Aug 2025 18:35:27 +0200 Subject: [PATCH 1/4] Support test and expect aliases with variables --- src/rules/expect-expect.test.ts | 14 +++ src/rules/no-commented-out-tests.test.ts | 5 + src/rules/no-commented-out-tests.ts | 4 +- src/rules/no-standalone-expect.test.ts | 16 +++ src/utils/ast.ts | 36 ++++++ src/utils/parseFnCall.test.ts | 149 +++++++++++++++++++++++ src/utils/parseFnCall.ts | 44 ++++++- 7 files changed, 264 insertions(+), 4 deletions(-) diff --git a/src/rules/expect-expect.test.ts b/src/rules/expect-expect.test.ts index 725288d..e773e99 100644 --- a/src/rules/expect-expect.test.ts +++ b/src/rules/expect-expect.test.ts @@ -7,6 +7,13 @@ runRuleTester('expect-expect', rule, { code: 'test("should fail", () => {});', errors: [{ messageId: 'noAssertions', type: 'Identifier' }], }, + { + code: javascript` + import { test as stuff } from '@playwright/test'; + stuff("should fail", () => {}); + `, + errors: [{ messageId: 'noAssertions', type: 'Identifier' }], + }, { code: 'test.skip("should fail", () => {});', errors: [{ messageId: 'noAssertions', type: 'MemberExpression' }], @@ -110,6 +117,13 @@ runRuleTester('expect-expect', rule, { name: 'Custom assert class method', options: [{ assertFunctionNames: ['assertCustomCondition'] }], }, + { + code: javascript` + import { test as stuff, expect as check } from '@playwright/test'; + stuff('works', () => { check(1).toBe(1); }); + `, + name: 'Imported aliases for test and expect', + }, { code: 'it("should pass", () => expect(true).toBeDefined())', name: 'Global alias - test', diff --git a/src/rules/no-commented-out-tests.test.ts b/src/rules/no-commented-out-tests.test.ts index 0a7b5cc..812dc60 100644 --- a/src/rules/no-commented-out-tests.test.ts +++ b/src/rules/no-commented-out-tests.test.ts @@ -143,5 +143,10 @@ runRuleTester('no-commented-out-tests', rule, { }, }, }, + // Imported alias for test used properly and not commented + javascript` + import { test as stuff, expect as check } from '@playwright/test'; + stuff('foo', () => { check(1).toBe(1); }); + `, ], }) diff --git a/src/rules/no-commented-out-tests.ts b/src/rules/no-commented-out-tests.ts index e986b93..9dd14a9 100644 --- a/src/rules/no-commented-out-tests.ts +++ b/src/rules/no-commented-out-tests.ts @@ -1,10 +1,12 @@ import { Rule } from 'eslint' import * as ESTree from 'estree' import { createRule } from '../utils/createRule.js' +import { getImportedAliases } from '../utils/ast.js' function getTestNames(context: Rule.RuleContext) { const aliases = context.settings.playwright?.globalAliases?.test ?? [] - return ['test', ...aliases] + const importAliases = getImportedAliases(context, 'test') + return ['test', ...aliases, ...importAliases] } function hasTests(context: Rule.RuleContext, node: ESTree.Comment) { diff --git a/src/rules/no-standalone-expect.test.ts b/src/rules/no-standalone-expect.test.ts index 4a226d1..8d65b07 100644 --- a/src/rules/no-standalone-expect.test.ts +++ b/src/rules/no-standalone-expect.test.ts @@ -9,6 +9,22 @@ runRuleTester('no-standalone-expect', rule, { code: 'test.describe("a test", () => { expect(1).toBe(1); });', errors: [{ column: 33, endColumn: 50, messageId }], }, + { + code: javascript` + import { expect as check } from '@playwright/test'; + check(1).toBe(1); + `, + errors: [{ messageId }], + name: 'Imported expect alias outside of test', + }, + { + code: javascript` + import { test as stuff, expect as check } from '@playwright/test'; + stuff.describe('a test', () => { check(1).toBe(1); }); + `, + errors: [{ messageId }], + name: 'Aliased expect inside aliased describe should be flagged', + }, { code: 'test.describe("a test", () => { expect.soft(1).toBe(1); });', errors: [{ column: 33, endColumn: 55, messageId }], diff --git a/src/utils/ast.ts b/src/utils/ast.ts index fa94e4a..3a4ae5a 100644 --- a/src/utils/ast.ts +++ b/src/utils/ast.ts @@ -247,3 +247,39 @@ export function dereference( return expr?.right ?? decl?.init } + +/** + * Returns local alias names imported from `@playwright/test` for a given named + * import. + * + * For example, for `import { test as foo } from '@playwright/test'`, + * `getImportedAliases(context, 'test')` will return `['foo']`. + */ +export function getImportedAliases( + context: Rule.RuleContext, + importedName: string, +): string[] { + const program = context.sourceCode.ast + const aliases = new Set() + + if (program.type !== 'Program') return [] + + for (const stmt of program.body) { + if (stmt.type !== 'ImportDeclaration') continue + if (stmt.source.value !== '@playwright/test') continue + + for (const spec of stmt.specifiers) { + if (spec.type !== 'ImportSpecifier') continue + + const imported = (spec.imported as ESTree.Identifier).name + if (imported !== importedName) continue + + const localName = spec.local.name + if (localName !== imported) { + aliases.add(localName) + } + } + } + + return [...aliases] +} diff --git a/src/utils/parseFnCall.test.ts b/src/utils/parseFnCall.test.ts index fee4e20..a0f5ea0 100644 --- a/src/utils/parseFnCall.test.ts +++ b/src/utils/parseFnCall.test.ts @@ -130,6 +130,58 @@ runRuleTester('nonexistent methods', rule, { runRuleTester('expect', rule, { invalid: [ + { + code: javascript` + import { expect as verify, expect as assertThat } from '@playwright/test'; + + verify(x).toBe(y); + assertThat(x).toBe(y); + `, + errors: [ + { + column: 1, + data: expectedParsedFnCallResultData({ + args: ['x'], + group: 'expect', + head: { + local: 'verify', + node: 'verify', + original: 'expect', + }, + matcher: 'toBe', + matcherArgs: ['y'], + matcherName: 'toBe', + members: ['toBe'], + modifiers: [], + name: 'expect', + type: 'expect', + }), + line: 3, + messageId: 'details', + }, + { + column: 1, + data: expectedParsedFnCallResultData({ + args: ['x'], + group: 'expect', + head: { + local: 'assertThat', + node: 'assertThat', + original: 'expect', + }, + matcher: 'toBe', + matcherArgs: ['y'], + matcherName: 'toBe', + members: ['toBe'], + modifiers: [], + name: 'expect', + type: 'expect', + }), + line: 4, + messageId: 'details', + }, + ], + }, { code: 'expect(x).toBe(y);', errors: [ @@ -320,6 +372,36 @@ runRuleTester('expect', rule, { }, ], }, + { + code: javascript` + import { expect as verify } from '@playwright/test'; + + verify(x).toBe(y); + `, + errors: [ + { + column: 1, + data: expectedParsedFnCallResultData({ + args: ['x'], + group: 'expect', + head: { + local: 'verify', + node: 'verify', + original: 'expect', + }, + matcher: 'toBe', + matcherArgs: ['y'], + matcherName: 'toBe', + members: ['toBe'], + modifiers: [], + name: 'expect', + type: 'expect', + }), + line: 3, + messageId: 'details', + }, + ], + }, { code: 'something(expect(x).not.toBe(y))', errors: [ @@ -418,6 +500,48 @@ runRuleTester('expect', rule, { runRuleTester('test', rule, { invalid: [ + { + code: javascript` + import { test as it, test as spec } from '@playwright/test'; + + it('a test', () => {}); + spec('another test', () => {}); + `, + errors: [ + { + column: 1, + data: expectedParsedFnCallResultData({ + group: 'test', + head: { + local: 'it', + node: 'it', + original: 'test', + }, + members: [], + name: 'test', + type: 'test', + }), + line: 3, + messageId: 'details', + }, + { + column: 1, + data: expectedParsedFnCallResultData({ + group: 'test', + head: { + local: 'spec', + node: 'spec', + original: 'test', + }, + members: [], + name: 'test', + type: 'test', + }), + line: 4, + messageId: 'details', + }, + ], + }, { code: 'test("a test", () => {});', errors: [ @@ -805,6 +929,31 @@ runRuleTester('test', rule, { }, ], }, + { + code: javascript` + import { test as it } from '@playwright/test'; + + it('a test', () => {}); + `, + errors: [ + { + column: 1, + data: expectedParsedFnCallResultData({ + group: 'test', + head: { + local: 'it', + node: 'it', + original: 'test', + }, + members: [], + name: 'test', + type: 'test', + }), + line: 3, + messageId: 'details', + }, + ], + }, ], valid: [ // Other functions diff --git a/src/utils/parseFnCall.ts b/src/utils/parseFnCall.ts index 91f78b3..26ab62d 100644 --- a/src/utils/parseFnCall.ts +++ b/src/utils/parseFnCall.ts @@ -2,6 +2,7 @@ import { Rule } from 'eslint' import * as ESTree from 'estree' import { findParent, + getImportedAliases, getParent, getStringValue, isFunction, @@ -162,6 +163,28 @@ interface ResolvedFn { original: string | null } +/** + * Resolves the head identifier of a call chain to a Playwright function. + * + * Input: + * + * - `accessor`: an identifier or string literal at the head of a call chain + * (e.g., `expect`, `test`, or a local alias). + * + * Resolution order: + * + * 1. Imported aliases from `@playwright/test` for `expect` and `test` (highest + * priority). + * 2. Direct `expect` usage, normalizing `Expect` to `expect`. + * 3. Configured global aliases from `settings.playwright.globalAliases`. + * + * Returns `{ original, local }` where: + * + * - `original` is the canonical Playwright name (`'expect'`, `'test'`, or `null` + * if unknown). + * - `local` is the identifier as written in source (or `'expect'` when + * normalizing `Expect`). + */ const resolveToPlaywrightFn = ( context: Rule.RuleContext, accessor: AccessorNode, @@ -169,10 +192,25 @@ const resolveToPlaywrightFn = ( const ident = getStringValue(accessor) const resolved = /(^expect|Expect)$/.test(ident) ? 'expect' : ident + const expectAliases = getImportedAliases(context, 'expect') + const testAliases = getImportedAliases(context, 'test') + + let original: string | null = null + let local = resolved + + if (expectAliases.includes(ident)) { + original = 'expect' + local = ident + } else if (testAliases.includes(ident)) { + original = 'test' + local = ident + } else { + original = resolvePossibleAliasedGlobal(context, resolved) + } + return { - // eslint-disable-next-line sort/object-properties - original: resolvePossibleAliasedGlobal(context, resolved), - local: resolved, + local, + original, } } From a96a0045fcc3dd20749436b41c5de2cc74a2d41c Mon Sep 17 00:00:00 2001 From: Michal Sroczynski <93290723+MSroczynski3@users.noreply.github.com> Date: Wed, 13 Aug 2025 19:50:55 +0200 Subject: [PATCH 2/4] More test cases --- src/rules/expect-expect.test.ts | 30 +++++ src/rules/no-commented-out-tests.test.ts | 14 ++ src/rules/no-commented-out-tests.ts | 3 +- src/utils/ast.ts | 7 +- src/utils/parseFnCall.test.ts | 155 +++++++++++++++++++++++ 5 files changed, 207 insertions(+), 2 deletions(-) diff --git a/src/rules/expect-expect.test.ts b/src/rules/expect-expect.test.ts index e773e99..9c96e49 100644 --- a/src/rules/expect-expect.test.ts +++ b/src/rules/expect-expect.test.ts @@ -13,11 +13,20 @@ runRuleTester('expect-expect', rule, { stuff("should fail", () => {}); `, errors: [{ messageId: 'noAssertions', type: 'Identifier' }], + name: 'Imported alias for test without assertions', }, { code: 'test.skip("should fail", () => {});', errors: [{ messageId: 'noAssertions', type: 'MemberExpression' }], }, + { + code: javascript` + import { test as stuff } from '@playwright/test'; + stuff.skip("should fail", () => {}); + `, + errors: [{ messageId: 'noAssertions', type: 'MemberExpression' }], + name: 'Imported alias for test without assertions', + }, { code: javascript` test('should fail', async ({ page }) => { @@ -124,6 +133,27 @@ runRuleTester('expect-expect', rule, { `, name: 'Imported aliases for test and expect', }, + { + code: javascript` + import { test as stuff } from '@playwright/test'; + stuff('works', () => { stuff.expect(1).toBe(1); }); + `, + name: 'Aliased test with property expect', + }, + { + code: javascript` + import { test as stuff, expect } from '@playwright/test'; + stuff('works', () => { expect(1).toBe(1); }); + `, + name: 'Aliased test with direct expect', + }, + { + code: javascript` + import { test, expect as check } from '@playwright/test'; + test('works', () => { check(1).toBe(1); }); + `, + name: 'Direct test with aliased expect', + }, { code: 'it("should pass", () => expect(true).toBeDefined())', name: 'Global alias - test', diff --git a/src/rules/no-commented-out-tests.test.ts b/src/rules/no-commented-out-tests.test.ts index 812dc60..5278e89 100644 --- a/src/rules/no-commented-out-tests.test.ts +++ b/src/rules/no-commented-out-tests.test.ts @@ -87,6 +87,20 @@ runRuleTester('no-commented-out-tests', rule, { }, }, }, + // Duplicate alias coming from both settings and import aliases should not + // create redundant entries in the regex and should still be detected. + { + code: javascript` + import { test as it } from '@playwright/test'; + // it("foo", () => {}); + `, + errors: [{ column: 1, line: 2, messageId }], + settings: { + playwright: { + globalAliases: { test: ['it'] }, + }, + }, + }, ], valid: [ '// foo("bar", function () {})', diff --git a/src/rules/no-commented-out-tests.ts b/src/rules/no-commented-out-tests.ts index 9dd14a9..8960cf7 100644 --- a/src/rules/no-commented-out-tests.ts +++ b/src/rules/no-commented-out-tests.ts @@ -6,7 +6,8 @@ import { getImportedAliases } from '../utils/ast.js' function getTestNames(context: Rule.RuleContext) { const aliases = context.settings.playwright?.globalAliases?.test ?? [] const importAliases = getImportedAliases(context, 'test') - return ['test', ...aliases, ...importAliases] + const combined = ['test', ...aliases, ...importAliases] + return Array.from(new Set(combined)) } function hasTests(context: Rule.RuleContext, node: ESTree.Comment) { diff --git a/src/utils/ast.ts b/src/utils/ast.ts index 3a4ae5a..5dde223 100644 --- a/src/utils/ast.ts +++ b/src/utils/ast.ts @@ -266,7 +266,12 @@ export function getImportedAliases( for (const stmt of program.body) { if (stmt.type !== 'ImportDeclaration') continue - if (stmt.source.value !== '@playwright/test') continue + if ( + !isStringLiteral(stmt.source) || + stmt.source.value !== '@playwright/test' + ) + continue + if ((stmt as any).importKind === 'type') continue for (const spec of stmt.specifiers) { if (spec.type !== 'ImportSpecifier') continue diff --git a/src/utils/parseFnCall.test.ts b/src/utils/parseFnCall.test.ts index a0f5ea0..4bf082a 100644 --- a/src/utils/parseFnCall.test.ts +++ b/src/utils/parseFnCall.test.ts @@ -182,6 +182,59 @@ runRuleTester('expect', rule, { }, ], }, + { + code: javascript` + import { expect as verify } from '@playwright/test'; + import { expect as assertThat } from '@playwright/test'; + + verify(x).toBe(y); + assertThat(x).toBe(y); + `, + errors: [ + { + column: 1, + data: expectedParsedFnCallResultData({ + args: ['x'], + group: 'expect', + head: { + local: 'verify', + node: 'verify', + original: 'expect', + }, + matcher: 'toBe', + matcherArgs: ['y'], + matcherName: 'toBe', + members: ['toBe'], + modifiers: [], + name: 'expect', + type: 'expect', + }), + line: 4, + messageId: 'details', + }, + { + column: 1, + data: expectedParsedFnCallResultData({ + args: ['x'], + group: 'expect', + head: { + local: 'assertThat', + node: 'assertThat', + original: 'expect', + }, + matcher: 'toBe', + matcherArgs: ['y'], + matcherName: 'toBe', + members: ['toBe'], + modifiers: [], + name: 'expect', + type: 'expect', + }), + line: 5, + messageId: 'details', + }, + ], + }, { code: 'expect(x).toBe(y);', errors: [ @@ -402,6 +455,36 @@ runRuleTester('expect', rule, { }, ], }, + { + code: javascript` + import { expect as verify } from '@playwright/test'; + + verify(x).not.toBe(y); + `, + errors: [ + { + column: 1, + data: expectedParsedFnCallResultData({ + args: ['x'], + group: 'expect', + head: { + local: 'verify', + node: 'verify', + original: 'expect', + }, + matcher: 'toBe', + matcherArgs: ['y'], + matcherName: 'toBe', + members: ['not', 'toBe'], + modifiers: ['not'], + name: 'expect', + type: 'expect', + }), + line: 3, + messageId: 'details', + }, + ], + }, { code: 'something(expect(x).not.toBe(y))', errors: [ @@ -542,6 +625,49 @@ runRuleTester('test', rule, { }, ], }, + { + code: javascript` + import { test as it } from '@playwright/test'; + import { test as check } from '@playwright/test'; + + it('a test', () => {}); + check('another test', () => {}); + `, + errors: [ + { + column: 1, + data: expectedParsedFnCallResultData({ + group: 'test', + head: { + local: 'it', + node: 'it', + original: 'test', + }, + members: [], + name: 'test', + type: 'test', + }), + line: 4, + messageId: 'details', + }, + { + column: 1, + data: expectedParsedFnCallResultData({ + group: 'test', + head: { + local: 'check', + node: 'check', + original: 'test', + }, + members: [], + name: 'test', + type: 'test', + }), + line: 5, + messageId: 'details', + }, + ], + }, { code: 'test("a test", () => {});', errors: [ @@ -1313,5 +1439,34 @@ runTSRuleTester('typescript', rule, { expect.anything(); `, }, + // Type-only imports/specifiers should NOT create runtime aliases + { + code: typescript` + import type { test as it } from '@playwright/test'; + + it('is not detected as Playwright test', () => {}); + `, + }, + { + code: typescript` + import { type test as it } from '@playwright/test'; + + it('is not detected as Playwright test', () => {}); + `, + }, + { + code: typescript` + import type { expect as verify } from '@playwright/test'; + + verify(x).toBe(y); + `, + }, + { + code: typescript` + import { type expect as verify } from '@playwright/test'; + + verify(x).toBe(y); + `, + }, ], }) From fc68409a07c44a7454f64efc9d22d76941295513 Mon Sep 17 00:00:00 2001 From: Michal Sroczynski <93290723+MSroczynski3@users.noreply.github.com> Date: Wed, 13 Aug 2025 19:51:07 +0200 Subject: [PATCH 3/4] Guard against synthetic AST --- src/utils/ast.test.ts | 39 +++++++++++++++++++++++++++++++++++++++ src/utils/ast.ts | 10 ++++++++-- 2 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 src/utils/ast.test.ts diff --git a/src/utils/ast.test.ts b/src/utils/ast.test.ts new file mode 100644 index 0000000..89f186a --- /dev/null +++ b/src/utils/ast.test.ts @@ -0,0 +1,39 @@ +import { describe, it, expect } from 'vitest' +import type { Rule } from 'eslint' +import { getImportedAliases } from './ast.js' + +describe('getImportedAliases', () => { + it('supports ImportSpecifier.imported as Literal', () => { + // Intentionally construct a nonstandard/synthetic AST: in valid JS parsed by + // standard ESTree parsers, `ImportSpecifier.imported` is an Identifier. + // Here we use a Literal ('test') to ensure `getImportedAliases` gracefully + // handles such shapes and still treats `import { test as it }` as aliasing. + const program: any = { + type: 'Program', + sourceType: 'module', + body: [ + { + type: 'ImportDeclaration', + specifiers: [ + { + type: 'ImportSpecifier', + imported: { type: 'Literal', value: 'test', raw: "'test'" }, + local: { type: 'Identifier', name: 'it' }, + }, + ], + source: { + type: 'Literal', + value: '@playwright/test', + raw: "'@playwright/test'", + }, + }, + ], + } + + const context = { + sourceCode: { ast: program }, + } as unknown as Rule.RuleContext + const aliases = getImportedAliases(context, 'test') + expect(aliases).toEqual(['it']) + }) +}) diff --git a/src/utils/ast.ts b/src/utils/ast.ts index 5dde223..0a9c7f9 100644 --- a/src/utils/ast.ts +++ b/src/utils/ast.ts @@ -275,8 +275,14 @@ export function getImportedAliases( for (const spec of stmt.specifiers) { if (spec.type !== 'ImportSpecifier') continue - - const imported = (spec.imported as ESTree.Identifier).name + if ((spec as any).importKind === 'type') continue + const importedNode = spec.imported as ESTree.Identifier | ESTree.Literal + const imported = + importedNode.type === 'Identifier' + ? importedNode.name + : typeof importedNode.value === 'string' + ? importedNode.value + : undefined if (imported !== importedName) continue const localName = spec.local.name From 0dfc1ca743c5432c992bf54923f3bc3c8f8cd4c0 Mon Sep 17 00:00:00 2001 From: Michal Sroczynski <93290723+MSroczynski3@users.noreply.github.com> Date: Wed, 13 Aug 2025 20:00:36 +0200 Subject: [PATCH 4/4] Lint --- src/rules/no-commented-out-tests.ts | 2 +- src/utils/ast.test.ts | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/rules/no-commented-out-tests.ts b/src/rules/no-commented-out-tests.ts index 8960cf7..ff69701 100644 --- a/src/rules/no-commented-out-tests.ts +++ b/src/rules/no-commented-out-tests.ts @@ -1,7 +1,7 @@ import { Rule } from 'eslint' import * as ESTree from 'estree' -import { createRule } from '../utils/createRule.js' import { getImportedAliases } from '../utils/ast.js' +import { createRule } from '../utils/createRule.js' function getTestNames(context: Rule.RuleContext) { const aliases = context.settings.playwright?.globalAliases?.test ?? [] diff --git a/src/utils/ast.test.ts b/src/utils/ast.test.ts index 89f186a..57c6814 100644 --- a/src/utils/ast.test.ts +++ b/src/utils/ast.test.ts @@ -1,5 +1,5 @@ -import { describe, it, expect } from 'vitest' import type { Rule } from 'eslint' +import { describe, expect, it } from 'vitest' import { getImportedAliases } from './ast.js' describe('getImportedAliases', () => { @@ -9,25 +9,25 @@ describe('getImportedAliases', () => { // Here we use a Literal ('test') to ensure `getImportedAliases` gracefully // handles such shapes and still treats `import { test as it }` as aliasing. const program: any = { - type: 'Program', - sourceType: 'module', body: [ { - type: 'ImportDeclaration', + source: { + raw: "'@playwright/test'", + type: 'Literal', + value: '@playwright/test', + }, specifiers: [ { + imported: { raw: "'test'", type: 'Literal', value: 'test' }, + local: { name: 'it', type: 'Identifier' }, type: 'ImportSpecifier', - imported: { type: 'Literal', value: 'test', raw: "'test'" }, - local: { type: 'Identifier', name: 'it' }, }, ], - source: { - type: 'Literal', - value: '@playwright/test', - raw: "'@playwright/test'", - }, + type: 'ImportDeclaration', }, ], + sourceType: 'module', + type: 'Program', } const context = {