diff --git a/internal/plugins/jest/all.go b/internal/plugins/jest/all.go index f46b95f53..075bb9b3e 100644 --- a/internal/plugins/jest/all.go +++ b/internal/plugins/jest/all.go @@ -7,6 +7,7 @@ import ( "github.com/web-infra-dev/rslint/internal/plugins/jest/rules/no_deprecated_functions" "github.com/web-infra-dev/rslint/internal/plugins/jest/rules/no_disabled_tests" "github.com/web-infra-dev/rslint/internal/plugins/jest/rules/no_done_callback" + "github.com/web-infra-dev/rslint/internal/plugins/jest/rules/no_duplicate_hooks" "github.com/web-infra-dev/rslint/internal/plugins/jest/rules/no_focused_tests" "github.com/web-infra-dev/rslint/internal/plugins/jest/rules/no_hooks" "github.com/web-infra-dev/rslint/internal/plugins/jest/rules/no_identical_title" @@ -30,9 +31,10 @@ func GetAllRules() []rule.Rule { expect_expect.ExpectExpectRule, no_alias_methods.NoAliasMethodsRule, no_commented_out_tests.NoCommentedOutTestsRule, - no_disabled_tests.NoDisabledTestsRule, no_deprecated_functions.NoDeprecatedFunctionsRule, + no_disabled_tests.NoDisabledTestsRule, no_done_callback.NoDoneCallbackRule, + no_duplicate_hooks.NoDuplicateHooksRule, no_focused_tests.NoFocusedTestsRule, no_hooks.NoHooksRule, no_identical_title.NoIdenticalTitleRule, diff --git a/internal/plugins/jest/rules/no_duplicate_hooks/no_duplicate_hooks.go b/internal/plugins/jest/rules/no_duplicate_hooks/no_duplicate_hooks.go new file mode 100644 index 000000000..3ba7cf9bf --- /dev/null +++ b/internal/plugins/jest/rules/no_duplicate_hooks/no_duplicate_hooks.go @@ -0,0 +1,58 @@ +package no_duplicate_hooks + +import ( + "github.com/microsoft/typescript-go/shim/ast" + "github.com/web-infra-dev/rslint/internal/plugins/jest/utils" + "github.com/web-infra-dev/rslint/internal/rule" +) + +// Message Builder + +func buildNoDuplicateHookMessage(hook string) rule.RuleMessage { + return rule.RuleMessage{ + Id: "noDuplicateHook", + Description: "Duplicate " + hook + " in describe block", + Data: map[string]string{ + "hook": hook, + }, + } +} + +var NoDuplicateHooksRule = rule.Rule{ + Name: "jest/no-duplicate-hooks", + Run: func(ctx rule.RuleContext, options any) rule.RuleListeners { + hookContexts := []map[string]int{{}} + + return rule.RuleListeners{ + ast.KindCallExpression: func(node *ast.Node) { + jestFnCall := utils.ParseJestFnCall(node, ctx) + if jestFnCall == nil { + return + } + + if jestFnCall.Kind == utils.JestFnTypeDescribe { + hookContexts = append(hookContexts, map[string]int{}) + return + } + + if jestFnCall.Kind != utils.JestFnTypeHook { + return + } + + currentLayer := hookContexts[len(hookContexts)-1] + currentLayer[jestFnCall.Name]++ + if currentLayer[jestFnCall.Name] > 1 { + ctx.ReportNode(node, buildNoDuplicateHookMessage(jestFnCall.Name)) + } + }, + rule.ListenerOnExit(ast.KindCallExpression): func(node *ast.Node) { + if !utils.IsTypeOfJestFnCall(node, ctx, utils.JestFnTypeDescribe) { + return + } + if len(hookContexts) > 1 { + hookContexts = hookContexts[:len(hookContexts)-1] + } + }, + } + }, +} diff --git a/internal/plugins/jest/rules/no_duplicate_hooks/no_duplicate_hooks.md b/internal/plugins/jest/rules/no_duplicate_hooks/no_duplicate_hooks.md new file mode 100644 index 000000000..616affd20 --- /dev/null +++ b/internal/plugins/jest/rules/no_duplicate_hooks/no_duplicate_hooks.md @@ -0,0 +1,80 @@ +# no-duplicate-hooks + +## Rule Details + +Disallow duplicate Jest lifecycle hooks **in the same scope**. Each `describe` block (including `describe.skip`, `describe.each`, and tagged `describe.each`) opens a new scope; registering the same hook name twice (`beforeEach`, `afterEach`, `beforeAll`, or `afterAll`) reports the **second and later** calls. + +- **Same scope**: hooks directly in a `describe` callback, or anywhere still inside that `describe` while it is active (including inside nested `test` / `it` bodies). +- **Separate scopes**: nested `describe` blocks; sibling `describe` blocks; file top level (hooks outside any `describe` share one scope). +- **Allowed**: one of each hook type in the same block; the same hook name again in a child or sibling `describe`. +- **Imports**: `@jest/globals` hooks and renamed bindings (e.g. `afterEach as somethingElse`) count toward the same hook name. + +Examples of **incorrect** code for this rule: + +```javascript +describe('foo', () => { + beforeEach(() => { + // some setup + }); + beforeEach(() => { + // some setup + }); + test('foo_test', () => { + // some test + }); +}); + +// Nested describe scenario +describe('foo', () => { + beforeEach(() => { + // some setup + }); + test('foo_test', () => { + // some test + }); + describe('bar', () => { + test('bar_test', () => { + afterAll(() => { + // some teardown + }); + afterAll(() => { + // some teardown + }); + }); + }); +}); +``` + +Examples of **correct** code for this rule: + +```javascript +describe('foo', () => { + beforeEach(() => { + // some setup + }); + test('foo_test', () => { + // some test + }); +}); + +// Nested describe scenario +describe('foo', () => { + beforeEach(() => { + // some setup + }); + test('foo_test', () => { + // some test + }); + describe('bar', () => { + test('bar_test', () => { + beforeEach(() => { + // some setup + }); + }); + }); +}); +``` + +## Original Documentation + +- [jest/no-duplicate-hooks](https://github.com/jest-community/eslint-plugin-jest/blob/main/docs/rules/no-duplicate-hooks.md) diff --git a/internal/plugins/jest/rules/no_duplicate_hooks/no_duplicate_hooks_test.go b/internal/plugins/jest/rules/no_duplicate_hooks/no_duplicate_hooks_test.go new file mode 100644 index 000000000..df19beb94 --- /dev/null +++ b/internal/plugins/jest/rules/no_duplicate_hooks/no_duplicate_hooks_test.go @@ -0,0 +1,305 @@ +package no_duplicate_hooks_test + +import ( + "testing" + + "github.com/web-infra-dev/rslint/internal/plugins/jest/fixtures" + "github.com/web-infra-dev/rslint/internal/plugins/jest/rules/no_duplicate_hooks" + "github.com/web-infra-dev/rslint/internal/rule_tester" +) + +func TestNoDuplicateHooksRule(t *testing.T) { + rule_tester.RunRuleTester( + fixtures.GetRootDir(), + "tsconfig.json", + t, + &no_duplicate_hooks.NoDuplicateHooksRule, + []rule_tester.ValidTestCase{ + {Code: `describe("foo", () => { + beforeEach(() => {}) + test("bar", () => { + someFn(); + }) +})`}, + {Code: `beforeEach(() => {}) +test("bar", () => { + someFn(); +})`}, + {Code: `describe("foo", () => { + beforeAll(() => {}), + beforeEach(() => {}) + afterEach(() => {}) + afterAll(() => {}) + + test("bar", () => { + someFn(); + }) +})`}, + {Code: `describe.skip("foo", () => { + beforeEach(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) +}) +describe("foo", () => { + beforeEach(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) +})`}, + {Code: `describe("foo", () => { + beforeEach(() => {}), + test("bar", () => { + someFn(); + }) + describe("inner_foo", () => { + beforeEach(() => {}) + test("inner bar", () => { + someFn(); + }) + }) +})`}, + {Code: `describe.each(['hello'])('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); +});`}, + {Code: `describe('something', () => { + describe.each(['hello'])('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + + describe.each(['world'])('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); +});`}, + {Code: "describe.each``('%s', () => {\n beforeEach(() => {});\n\n it('is fine', () => {});\n});"}, + {Code: "describe('something', () => {\n describe.each``('%s', () => {\n beforeEach(() => {});\n\n it('is fine', () => {});\n });\n\n describe.each``('%s', () => {\n beforeEach(() => {});\n\n it('is fine', () => {});\n });\n});"}, + }, + []rule_tester.InvalidTestCase{ + { + Code: `describe("foo", () => { + beforeEach(() => {}), + beforeEach(() => {}), + test("bar", () => { + someFn(); + }) +})`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 3, Column: 3}, + }, + }, + { + Code: `describe.skip("foo", () => { + beforeEach(() => {}), + beforeAll(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) +})`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 4, Column: 3}, + }, + }, + { + Code: `describe.skip("foo", () => { + afterEach(() => {}), + afterEach(() => {}), + test("bar", () => { + someFn(); + }) +})`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 3, Column: 3}, + }, + }, + { + Code: `import { afterEach } from '@jest/globals'; + +describe.skip("foo", () => { + afterEach(() => {}), + afterEach(() => {}), + test("bar", () => { + someFn(); + }) +})`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 5, Column: 3}, + }, + }, + { + Code: `import { afterEach, afterEach as somethingElse } from '@jest/globals'; + +describe.skip("foo", () => { + afterEach(() => {}), + somethingElse(() => {}), + test("bar", () => { + someFn(); + }) +})`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 5, Column: 3}, + }, + }, + { + Code: `describe.skip("foo", () => { + afterAll(() => {}), + afterAll(() => {}), + test("bar", () => { + someFn(); + }) +})`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 3, Column: 3}, + }, + }, + { + Code: `afterAll(() => {}), +afterAll(() => {}), +test("bar", () => { + someFn(); +})`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 2, Column: 1}, + }, + }, + { + Code: `describe("foo", () => { + beforeEach(() => {}), + beforeEach(() => {}), + beforeEach(() => {}), + test("bar", () => { + someFn(); + }) +})`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 3, Column: 3}, + {MessageId: "noDuplicateHook", Line: 4, Column: 3}, + }, + }, + { + Code: `describe.skip("foo", () => { + afterAll(() => {}), + afterAll(() => {}), + beforeAll(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) +})`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 3, Column: 3}, + {MessageId: "noDuplicateHook", Line: 5, Column: 3}, + }, + }, + { + Code: `describe.skip("foo", () => { + beforeEach(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) +}) +describe("foo", () => { + beforeEach(() => {}), + beforeEach(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) +})`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 10, Column: 3}, + }, + }, + { + Code: `describe("foo", () => { + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) + describe("inner_foo", () => { + beforeEach(() => {}) + beforeEach(() => {}) + test("inner bar", () => { + someFn(); + }) + }) +})`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 8, Column: 5}, + }, + }, + { + Code: `describe.each(['hello'])('%s', () => { + beforeEach(() => {}); + beforeEach(() => {}); + + it('is not fine', () => {}); +});`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 3, Column: 3}, + }, + }, + { + Code: `describe('something', () => { + describe.each(['hello'])('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + + describe.each(['world'])('%s', () => { + beforeEach(() => {}); + beforeEach(() => {}); + + it('is not fine', () => {}); + }); +});`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 10, Column: 5}, + }, + }, + { + Code: `describe('something', () => { + describe.each(['hello'])('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + + describe.each(['world'])('%s', () => { + describe('some more', () => { + beforeEach(() => {}); + beforeEach(() => {}); + + it('is not fine', () => {}); + }); + }); +});`, + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 11, Column: 7}, + }, + }, + { + Code: "describe.each``('%s', () => {\n beforeEach(() => {});\n beforeEach(() => {});\n\n it('is fine', () => {});\n});", + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 3, Column: 3}, + }, + }, + { + Code: "describe('something', () => {\n describe.each``('%s', () => {\n beforeEach(() => {});\n\n it('is fine', () => {});\n });\n\n describe.each``('%s', () => {\n beforeEach(() => {});\n beforeEach(() => {});\n\n it('is not fine', () => {});\n });\n});", + Errors: []rule_tester.InvalidTestCaseError{ + {MessageId: "noDuplicateHook", Line: 10, Column: 5}, + }, + }, + }, + ) +} diff --git a/packages/rslint-test-tools/rstest.config.mts b/packages/rslint-test-tools/rstest.config.mts index 171915888..133a5aba7 100644 --- a/packages/rslint-test-tools/rstest.config.mts +++ b/packages/rslint-test-tools/rstest.config.mts @@ -439,7 +439,7 @@ export default defineConfig({ './tests/eslint-plugin-jest/rules/no-mocks-import.test.ts', './tests/eslint-plugin-jest/rules/no-standalone-expect.test.ts', './tests/eslint-plugin-jest/rules/no-test-prefixes.test.ts', - './tests/eslint-plugin-jest/rules/prefer-called-with.test.ts', + './tests/eslint-plugin-jest/rules/no-duplicate-hooks.test.ts', './tests/eslint-plugin-jest/rules/prefer-strict-equal.test.ts', './tests/eslint-plugin-jest/rules/prefer-to-be.test.ts', './tests/eslint-plugin-jest/rules/prefer-to-contain.test.ts', diff --git a/packages/rslint-test-tools/tests/eslint-plugin-jest/rules/no-duplicate-hooks.test.ts b/packages/rslint-test-tools/tests/eslint-plugin-jest/rules/no-duplicate-hooks.test.ts new file mode 100644 index 000000000..80dcfd9c1 --- /dev/null +++ b/packages/rslint-test-tools/tests/eslint-plugin-jest/rules/no-duplicate-hooks.test.ts @@ -0,0 +1,491 @@ +import { RuleTester } from '../rule-tester'; + +const ruleTester = new RuleTester(); + +ruleTester.run('no-duplicate-hooks', {} as never, { + valid: [ + { + code: ` + describe("foo", () => { + beforeEach(() => {}) + test("bar", () => { + someFn(); + }) + }) + `, + }, + { + code: ` + beforeEach(() => {}) + test("bar", () => { + someFn(); + }) + `, + }, + { + code: ` + describe("foo", () => { + beforeAll(() => {}), + beforeEach(() => {}) + afterEach(() => {}) + afterAll(() => {}) + + test("bar", () => { + someFn(); + }) + }) + `, + }, + // multiple describe blocks + { + code: ` + describe.skip("foo", () => { + beforeEach(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) + }) + describe("foo", () => { + beforeEach(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) + }) + `, + }, + // nested describe blocks + { + code: ` + describe("foo", () => { + beforeEach(() => {}), + test("bar", () => { + someFn(); + }) + describe("inner_foo", () => { + beforeEach(() => {}) + test("inner bar", () => { + someFn(); + }) + }) + }) + `, + }, + // describe.each blocks + { + code: ` + describe.each(['hello'])('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + `, + }, + { + code: ` + describe('something', () => { + describe.each(['hello'])('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + + describe.each(['world'])('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + }); + `, + }, + { + code: ` + describe.each\`\`('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + `, + }, + { + code: ` + describe('something', () => { + describe.each\`\`('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + + describe.each\`\`('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + }); + `, + }, + ], + invalid: [ + { + code: ` + describe("foo", () => { + beforeEach(() => {}), + beforeEach(() => {}), + test("bar", () => { + someFn(); + }) + }) + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeEach' }, + column: 3, + line: 3, + }, + ], + }, + { + code: ` + describe.skip("foo", () => { + beforeEach(() => {}), + beforeAll(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) + }) + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeAll' }, + column: 3, + line: 4, + }, + ], + }, + { + code: ` + describe.skip("foo", () => { + afterEach(() => {}), + afterEach(() => {}), + test("bar", () => { + someFn(); + }) + }) + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'afterEach' }, + column: 3, + line: 3, + }, + ], + }, + { + code: ` + import { afterEach } from '@jest/globals'; + + describe.skip("foo", () => { + afterEach(() => {}), + afterEach(() => {}), + test("bar", () => { + someFn(); + }) + }) + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'afterEach' }, + column: 3, + line: 5, + }, + ], + }, + { + code: ` + import { afterEach, afterEach as somethingElse } from '@jest/globals'; + + describe.skip("foo", () => { + afterEach(() => {}), + somethingElse(() => {}), + test("bar", () => { + someFn(); + }) + }) + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'afterEach' }, + column: 3, + line: 5, + }, + ], + }, + { + code: ` + describe.skip("foo", () => { + afterAll(() => {}), + afterAll(() => {}), + test("bar", () => { + someFn(); + }) + }) + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'afterAll' }, + column: 3, + line: 3, + }, + ], + }, + { + code: ` + afterAll(() => {}), + afterAll(() => {}), + test("bar", () => { + someFn(); + }) + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'afterAll' }, + column: 1, + line: 2, + }, + ], + }, + { + code: ` + describe("foo", () => { + beforeEach(() => {}), + beforeEach(() => {}), + beforeEach(() => {}), + test("bar", () => { + someFn(); + }) + }) + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeEach' }, + column: 3, + line: 3, + }, + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeEach' }, + column: 3, + line: 4, + }, + ], + }, + { + code: ` + describe.skip("foo", () => { + afterAll(() => {}), + afterAll(() => {}), + beforeAll(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) + }) + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'afterAll' }, + column: 3, + line: 3, + }, + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeAll' }, + column: 3, + line: 5, + }, + ], + }, + // multiple describe blocks + { + code: ` + describe.skip("foo", () => { + beforeEach(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) + }) + describe("foo", () => { + beforeEach(() => {}), + beforeEach(() => {}), + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) + }) + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeEach' }, + column: 3, + line: 10, + }, + ], + }, + // nested describe blocks + { + code: ` + describe("foo", () => { + beforeAll(() => {}), + test("bar", () => { + someFn(); + }) + describe("inner_foo", () => { + beforeEach(() => {}) + beforeEach(() => {}) + test("inner bar", () => { + someFn(); + }) + }) + }) + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeEach' }, + column: 5, + line: 8, + }, + ], + }, + // describe.each blocks + { + code: ` + describe.each(['hello'])('%s', () => { + beforeEach(() => {}); + beforeEach(() => {}); + + it('is not fine', () => {}); + }); + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeEach' }, + column: 3, + line: 3, + }, + ], + }, + { + code: ` + describe('something', () => { + describe.each(['hello'])('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + + describe.each(['world'])('%s', () => { + beforeEach(() => {}); + beforeEach(() => {}); + + it('is not fine', () => {}); + }); + }); + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeEach' }, + column: 5, + line: 10, + }, + ], + }, + { + code: ` + describe('something', () => { + describe.each(['hello'])('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + + describe.each(['world'])('%s', () => { + describe('some more', () => { + beforeEach(() => {}); + beforeEach(() => {}); + + it('is not fine', () => {}); + }); + }); + }); + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeEach' }, + column: 7, + line: 11, + }, + ], + }, + { + code: ` + describe.each\`\`('%s', () => { + beforeEach(() => {}); + beforeEach(() => {}); + + it('is fine', () => {}); + }); + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeEach' }, + column: 3, + line: 3, + }, + ], + }, + { + code: ` + describe('something', () => { + describe.each\`\`('%s', () => { + beforeEach(() => {}); + + it('is fine', () => {}); + }); + + describe.each\`\`('%s', () => { + beforeEach(() => {}); + beforeEach(() => {}); + + it('is not fine', () => {}); + }); + }); + `, + errors: [ + { + messageId: 'noDuplicateHook', + data: { hook: 'beforeEach' }, + column: 5, + line: 10, + }, + ], + }, + ], +});