From 746a0ac702bbd27c03d48a7d2837decea125c661 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Fri, 22 Mar 2024 07:43:43 +1300 Subject: [PATCH] feat: remove `no-if` rule BREAKING CHANGE --- README.md | 118 ++-- docs/rules/no-if.md | 58 -- src/__tests__/rules.test.ts | 2 +- src/rules/__tests__/no-if.test.ts | 866 ------------------------------ src/rules/no-if.ts | 121 ----- 5 files changed, 59 insertions(+), 1106 deletions(-) delete mode 100644 docs/rules/no-if.md delete mode 100644 src/rules/__tests__/no-if.test.ts delete mode 100644 src/rules/no-if.ts diff --git a/README.md b/README.md index 02d08e26b..afd0e7da6 100644 --- a/README.md +++ b/README.md @@ -318,69 +318,67 @@ set to warn in.\ 🎨 Set in the `style` [configuration](https://github.com/jest-community/eslint-plugin-jest/blob/main/README.md#shareable-configurations).\ πŸ”§ Automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/user-guide/command-line-interface#--fix).\ -πŸ’‘ Manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions).\ -❌ Deprecated. - -| NameΒ Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β  | Description | πŸ’Ό | ⚠️ | πŸ”§ | πŸ’‘ | ❌ | -| :--------------------------------------------------------------------------- | :------------------------------------------------------------------------ | :-- | :-- | :-- | :-- | :-- | -| [consistent-test-it](docs/rules/consistent-test-it.md) | Enforce `test` and `it` usage conventions | | | πŸ”§ | | | -| [expect-expect](docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | | βœ… | | | | -| [max-expects](docs/rules/max-expects.md) | Enforces a maximum number assertion calls in a test body | | | | | | -| [max-nested-describe](docs/rules/max-nested-describe.md) | Enforces a maximum depth to nested describe calls | | | | | | -| [no-alias-methods](docs/rules/no-alias-methods.md) | Disallow alias methods | βœ… | 🎨 | πŸ”§ | | | -| [no-commented-out-tests](docs/rules/no-commented-out-tests.md) | Disallow commented out tests | | βœ… | | | | -| [no-conditional-expect](docs/rules/no-conditional-expect.md) | Disallow calling `expect` conditionally | βœ… | | | | | -| [no-conditional-in-test](docs/rules/no-conditional-in-test.md) | Disallow conditional logic in tests | | | | | | -| [no-confusing-set-timeout](docs/rules/no-confusing-set-timeout.md) | Disallow confusing usages of jest.setTimeout | | | | | | -| [no-deprecated-functions](docs/rules/no-deprecated-functions.md) | Disallow use of deprecated functions | βœ… | | πŸ”§ | | | -| [no-disabled-tests](docs/rules/no-disabled-tests.md) | Disallow disabled tests | | βœ… | | | | -| [no-done-callback](docs/rules/no-done-callback.md) | Disallow using a callback in asynchronous tests and hooks | βœ… | | | πŸ’‘ | | -| [no-duplicate-hooks](docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | | | | | -| [no-export](docs/rules/no-export.md) | Disallow using `exports` in files containing tests | βœ… | | | | | -| [no-focused-tests](docs/rules/no-focused-tests.md) | Disallow focused tests | βœ… | | | πŸ’‘ | | -| [no-hooks](docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | | | | | -| [no-identical-title](docs/rules/no-identical-title.md) | Disallow identical titles | βœ… | | | | | -| [no-if](docs/rules/no-if.md) | Disallow conditional logic | | | | | ❌ | -| [no-interpolation-in-snapshots](docs/rules/no-interpolation-in-snapshots.md) | Disallow string interpolation inside snapshots | βœ… | | | | | -| [no-jasmine-globals](docs/rules/no-jasmine-globals.md) | Disallow Jasmine globals | βœ… | | πŸ”§ | | | -| [no-large-snapshots](docs/rules/no-large-snapshots.md) | Disallow large snapshots | | | | | | -| [no-mocks-import](docs/rules/no-mocks-import.md) | Disallow manually importing from `__mocks__` | βœ… | | | | | -| [no-restricted-jest-methods](docs/rules/no-restricted-jest-methods.md) | Disallow specific `jest.` methods | | | | | | -| [no-restricted-matchers](docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | | | | | -| [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | βœ… | | | | | -| [no-test-prefixes](docs/rules/no-test-prefixes.md) | Require using `.only` and `.skip` over `f` and `x` | βœ… | | πŸ”§ | | | -| [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | | | | | -| [no-untyped-mock-factory](docs/rules/no-untyped-mock-factory.md) | Disallow using `jest.mock()` factories without an explicit type parameter | | | πŸ”§ | | | -| [prefer-called-with](docs/rules/prefer-called-with.md) | Suggest using `toBeCalledWith()` or `toHaveBeenCalledWith()` | | | | | | -| [prefer-comparison-matcher](docs/rules/prefer-comparison-matcher.md) | Suggest using the built-in comparison matchers | | | πŸ”§ | | | -| [prefer-each](docs/rules/prefer-each.md) | Prefer using `.each` rather than manual loops | | | | | | -| [prefer-equality-matcher](docs/rules/prefer-equality-matcher.md) | Suggest using the built-in equality matchers | | | | πŸ’‘ | | -| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | | | πŸ’‘ | | -| [prefer-expect-resolves](docs/rules/prefer-expect-resolves.md) | Prefer `await expect(...).resolves` over `expect(await ...)` syntax | | | πŸ”§ | | | -| [prefer-hooks-in-order](docs/rules/prefer-hooks-in-order.md) | Prefer having hooks in a consistent order | | | | | | -| [prefer-hooks-on-top](docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | | | | -| [prefer-lowercase-title](docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | | πŸ”§ | | | -| [prefer-mock-promise-shorthand](docs/rules/prefer-mock-promise-shorthand.md) | Prefer mock resolved/rejected shorthands for promises | | | πŸ”§ | | | -| [prefer-snapshot-hint](docs/rules/prefer-snapshot-hint.md) | Prefer including a hint with external snapshots | | | | | | -| [prefer-spy-on](docs/rules/prefer-spy-on.md) | Suggest using `jest.spyOn()` | | | πŸ”§ | | | -| [prefer-strict-equal](docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | | | πŸ’‘ | | -| [prefer-to-be](docs/rules/prefer-to-be.md) | Suggest using `toBe()` for primitive literals | 🎨 | | πŸ”§ | | | -| [prefer-to-contain](docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | 🎨 | | πŸ”§ | | | -| [prefer-to-have-length](docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | 🎨 | | πŸ”§ | | | -| [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | | πŸ”§ | | | -| [require-hook](docs/rules/require-hook.md) | Require setup and teardown code to be within a hook | | | | | | -| [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | | | | | -| [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | | | | | -| [valid-describe-callback](docs/rules/valid-describe-callback.md) | Enforce valid `describe()` callback | βœ… | | | | | -| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | βœ… | | | | | -| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Require promises that have expectations in their chain to be valid | βœ… | | | | | -| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | βœ… | | πŸ”§ | | | +πŸ’‘ Manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). + +| NameΒ Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β  | Description | πŸ’Ό | ⚠️ | πŸ”§ | πŸ’‘ | +| :--------------------------------------------------------------------------- | :------------------------------------------------------------------------ | :-- | :-- | :-- | :-- | +| [consistent-test-it](docs/rules/consistent-test-it.md) | Enforce `test` and `it` usage conventions | | | πŸ”§ | | +| [expect-expect](docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | | βœ… | | | +| [max-expects](docs/rules/max-expects.md) | Enforces a maximum number assertion calls in a test body | | | | | +| [max-nested-describe](docs/rules/max-nested-describe.md) | Enforces a maximum depth to nested describe calls | | | | | +| [no-alias-methods](docs/rules/no-alias-methods.md) | Disallow alias methods | βœ… | 🎨 | πŸ”§ | | +| [no-commented-out-tests](docs/rules/no-commented-out-tests.md) | Disallow commented out tests | | βœ… | | | +| [no-conditional-expect](docs/rules/no-conditional-expect.md) | Disallow calling `expect` conditionally | βœ… | | | | +| [no-conditional-in-test](docs/rules/no-conditional-in-test.md) | Disallow conditional logic in tests | | | | | +| [no-confusing-set-timeout](docs/rules/no-confusing-set-timeout.md) | Disallow confusing usages of jest.setTimeout | | | | | +| [no-deprecated-functions](docs/rules/no-deprecated-functions.md) | Disallow use of deprecated functions | βœ… | | πŸ”§ | | +| [no-disabled-tests](docs/rules/no-disabled-tests.md) | Disallow disabled tests | | βœ… | | | +| [no-done-callback](docs/rules/no-done-callback.md) | Disallow using a callback in asynchronous tests and hooks | βœ… | | | πŸ’‘ | +| [no-duplicate-hooks](docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | | | | +| [no-export](docs/rules/no-export.md) | Disallow using `exports` in files containing tests | βœ… | | | | +| [no-focused-tests](docs/rules/no-focused-tests.md) | Disallow focused tests | βœ… | | | πŸ’‘ | +| [no-hooks](docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | | | | +| [no-identical-title](docs/rules/no-identical-title.md) | Disallow identical titles | βœ… | | | | +| [no-interpolation-in-snapshots](docs/rules/no-interpolation-in-snapshots.md) | Disallow string interpolation inside snapshots | βœ… | | | | +| [no-jasmine-globals](docs/rules/no-jasmine-globals.md) | Disallow Jasmine globals | βœ… | | πŸ”§ | | +| [no-large-snapshots](docs/rules/no-large-snapshots.md) | Disallow large snapshots | | | | | +| [no-mocks-import](docs/rules/no-mocks-import.md) | Disallow manually importing from `__mocks__` | βœ… | | | | +| [no-restricted-jest-methods](docs/rules/no-restricted-jest-methods.md) | Disallow specific `jest.` methods | | | | | +| [no-restricted-matchers](docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | | | | +| [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | βœ… | | | | +| [no-test-prefixes](docs/rules/no-test-prefixes.md) | Require using `.only` and `.skip` over `f` and `x` | βœ… | | πŸ”§ | | +| [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | | | | +| [no-untyped-mock-factory](docs/rules/no-untyped-mock-factory.md) | Disallow using `jest.mock()` factories without an explicit type parameter | | | πŸ”§ | | +| [prefer-called-with](docs/rules/prefer-called-with.md) | Suggest using `toBeCalledWith()` or `toHaveBeenCalledWith()` | | | | | +| [prefer-comparison-matcher](docs/rules/prefer-comparison-matcher.md) | Suggest using the built-in comparison matchers | | | πŸ”§ | | +| [prefer-each](docs/rules/prefer-each.md) | Prefer using `.each` rather than manual loops | | | | | +| [prefer-equality-matcher](docs/rules/prefer-equality-matcher.md) | Suggest using the built-in equality matchers | | | | πŸ’‘ | +| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | | | πŸ’‘ | +| [prefer-expect-resolves](docs/rules/prefer-expect-resolves.md) | Prefer `await expect(...).resolves` over `expect(await ...)` syntax | | | πŸ”§ | | +| [prefer-hooks-in-order](docs/rules/prefer-hooks-in-order.md) | Prefer having hooks in a consistent order | | | | | +| [prefer-hooks-on-top](docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | | | +| [prefer-lowercase-title](docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | | πŸ”§ | | +| [prefer-mock-promise-shorthand](docs/rules/prefer-mock-promise-shorthand.md) | Prefer mock resolved/rejected shorthands for promises | | | πŸ”§ | | +| [prefer-snapshot-hint](docs/rules/prefer-snapshot-hint.md) | Prefer including a hint with external snapshots | | | | | +| [prefer-spy-on](docs/rules/prefer-spy-on.md) | Suggest using `jest.spyOn()` | | | πŸ”§ | | +| [prefer-strict-equal](docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | | | πŸ’‘ | +| [prefer-to-be](docs/rules/prefer-to-be.md) | Suggest using `toBe()` for primitive literals | 🎨 | | πŸ”§ | | +| [prefer-to-contain](docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | 🎨 | | πŸ”§ | | +| [prefer-to-have-length](docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | 🎨 | | πŸ”§ | | +| [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | | πŸ”§ | | +| [require-hook](docs/rules/require-hook.md) | Require setup and teardown code to be within a hook | | | | | +| [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | | | | +| [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | | | | +| [valid-describe-callback](docs/rules/valid-describe-callback.md) | Enforce valid `describe()` callback | βœ… | | | | +| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | βœ… | | | | +| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Require promises that have expectations in their chain to be valid | βœ… | | | | +| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | βœ… | | πŸ”§ | | ### Requires Type Checking -| NameΒ Β Β Β Β Β Β Β Β Β  | Description | πŸ’Ό | ⚠️ | πŸ”§ | πŸ’‘ | ❌ | -| :--------------------------------------------- | :----------------------------------------------------------- | :-- | :-- | :-- | :-- | :-- | -| [unbound-method](docs/rules/unbound-method.md) | Enforce unbound methods are called with their expected scope | | | | | | +| NameΒ Β Β Β Β Β Β Β Β Β  | Description | πŸ’Ό | ⚠️ | πŸ”§ | πŸ’‘ | +| :--------------------------------------------- | :----------------------------------------------------------- | :-- | :-- | :-- | :-- | +| [unbound-method](docs/rules/unbound-method.md) | Enforce unbound methods are called with their expected scope | | | | | diff --git a/docs/rules/no-if.md b/docs/rules/no-if.md deleted file mode 100644 index c5add5524..000000000 --- a/docs/rules/no-if.md +++ /dev/null @@ -1,58 +0,0 @@ -# Disallow conditional logic (`no-if`) - -❌ This rule is deprecated. It was replaced by -[`jest/no-conditional-in-test`](no-conditional-in-test.md). - - - -Conditional logic in tests is usually an indication that a test is attempting to -cover too much, and not testing the logic it intends to. Each branch of code -executing within an if statement will usually be better served by a test devoted -to it. - -Conditionals are often used to satisfy the typescript type checker. In these -cases, using the non-null assertion operator (!) would be best. - -## Rule details - -This rule prevents the use of if/ else statements and conditional (ternary) -operations in tests. - -The following patterns are considered warnings: - -```js -it('foo', () => { - if ('bar') { - // an if statement here is invalid - // you are probably testing too much - } -}); - -it('foo', () => { - const bar = foo ? 'bar' : null; -}); -``` - -These patterns would not be considered warnings: - -```js -it('foo', () => { - // only test the 'foo' case -}); - -it('bar', () => { - // test the 'bar' case separately -}); - -it('foo', () => { - function foo(bar) { - // nested functions are valid - return foo ? bar : null; - } -}); -``` - -## When Not To Use It - -If you do not wish to prevent the use of if statements in tests, you can safely -disable this rule. diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index 59a6e3beb..8236727bd 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -2,7 +2,7 @@ import { existsSync } from 'fs'; import { resolve } from 'path'; import plugin from '../'; -const numberOfRules = 53; +const numberOfRules = 52; const ruleNames = Object.keys(plugin.rules); const deprecatedRules = Object.entries(plugin.rules) .filter(([, rule]) => rule.meta.deprecated) diff --git a/src/rules/__tests__/no-if.test.ts b/src/rules/__tests__/no-if.test.ts deleted file mode 100644 index 786ace8e5..000000000 --- a/src/rules/__tests__/no-if.test.ts +++ /dev/null @@ -1,866 +0,0 @@ -import { TSESLint } from '@typescript-eslint/utils'; -import dedent from 'dedent'; -import rule from '../no-if'; -import { espreeParser } from './test-utils'; - -const ruleTester = new TSESLint.RuleTester({ - parser: espreeParser, - parserOptions: { - ecmaVersion: 2015, - }, -}); - -ruleTester.run('conditional expressions', rule, { - valid: [ - 'const x = y ? 1 : 0', - dedent` - it('foo', () => { - const foo = function (bar) { - return foo ? bar : null; - }; - }); - `, - dedent` - it('foo', function () { - const foo = function (bar) { - return foo ? bar : null; - }; - }); - `, - dedent` - it.each()('foo', function () { - const foo = function (bar) { - return foo ? bar : null; - }; - }); - `, - ], - invalid: [ - { - code: dedent` - it('foo', () => { - expect(bar ? foo : baz).toBe(boo); - }) - `, - errors: [ - { - data: { condition: 'conditional' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it('foo', () => { - const foo = bar ? foo : baz; - }) - `, - errors: [ - { - data: { condition: 'conditional' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it('foo', () => { - const foo = bar ? foo : baz; - }) - const foo = bar ? foo : baz; - `, - errors: [ - { - data: { condition: 'conditional' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it('foo', () => { - const foo = bar ? foo : baz; - const anotherFoo = anotherBar ? anotherFoo : anotherBaz; - }) - `, - errors: [ - { - data: { condition: 'conditional' }, - messageId: 'conditionalInTest', - }, - { - data: { condition: 'conditional' }, - messageId: 'conditionalInTest', - }, - ], - }, - ], -}); - -ruleTester.run('switch statements', rule, { - valid: [ - dedent` - switch (true) { - case true: {} - } - `, - `it('foo', () => {})`, - dedent` - it('foo', () => {}); - function myTest() { - switch ('bar') { - } - } - `, - dedent` - foo('bar', () => { - switch(baz) {} - }) - `, - dedent` - describe('foo', () => { - switch('bar') {} - }) - `, - dedent` - describe.skip('foo', () => { - switch('bar') {} - }) - `, - dedent` - describe.skip.each()('foo', () => { - switch('bar') {} - }) - `, - dedent` - xdescribe('foo', () => { - switch('bar') {} - }) - `, - dedent` - fdescribe('foo', () => { - switch('bar') {} - }) - `, - dedent` - describe('foo', () => { - switch('bar') {} - }) - switch('bar') {} - `, - dedent` - describe('foo', () => { - afterEach(() => { - switch('bar') {} - }); - }); - `, - dedent` - it('valid', () => { - const values = something.map(thing => { - switch (thing.isFoo) { - case true: - return thing.foo; - default: - return thing.bar; - } - }); - - expect(values).toStrictEqual(['foo']); - }); - `, - dedent` - describe('valid', () => { - it('still valid', () => { - const values = something.map(thing => { - switch (thing.isFoo) { - case true: - return thing.foo; - default: - return thing.bar; - } - }); - - expect(values).toStrictEqual(['foo']); - }); - }); - `, - ], - invalid: [ - { - code: dedent` - it('foo', () => { - switch (true) { - case true: {} - } - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it('foo', () => { - switch('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it.skip('foo', () => { - switch('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it.only('foo', () => { - switch('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - xit('foo', () => { - switch('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - fit('foo', () => { - switch('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - fit.concurrent('foo', () => { - switch('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - test('foo', () => { - switch('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - test.skip('foo', () => { - switch('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - test.only('foo', () => { - switch('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - xtest('foo', () => { - switch('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - xtest('foo', function () { - switch('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - describe('foo', () => { - it('bar', () => { - - switch('bar') {} - }) - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: "it('foo', myTest); function myTest() { switch ('bar') {} }", - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - describe('foo', () => { - it('bar', () => { - switch('bar') {} - }) - it('baz', () => { - switch('qux') {} - switch('quux') {} - }) - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it('foo', () => { - callExpression() - switch ('bar') {} - }) - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - describe('valid', () => { - describe('still valid', () => { - it('really still valid', () => { - const values = something.map((thing) => { - switch (thing.isFoo) { - case true: - return thing.foo; - default: - return thing.bar; - } - }); - - switch('invalid') { - case true: - expect(values).toStrictEqual(['foo']); - } - }); - }); - }); - `, - errors: [ - { - data: { condition: 'switch' }, - messageId: 'conditionalInTest', - }, - ], - }, - ], -}); - -ruleTester.run('if statements', rule, { - valid: [ - 'if(foo) {}', - "it('foo', () => {})", - 'it("foo", function () {})', - "it('foo', () => {}); function myTest() { if('bar') {} }", - dedent` - foo('bar', () => { - if(baz) {} - }) - `, - dedent` - describe('foo', () => { - if('bar') {} - }) - `, - dedent` - describe.skip('foo', () => { - if('bar') {} - }) - `, - dedent` - xdescribe('foo', () => { - if('bar') {} - }) - `, - dedent` - fdescribe('foo', () => { - if('bar') {} - }) - `, - dedent` - describe('foo', () => { - if('bar') {} - }) - if('baz') {} - `, - dedent` - describe('foo', () => { - afterEach(() => { - if('bar') {} - }); - }) - `, - dedent` - describe.each\`\`('foo', () => { - afterEach(() => { - if('bar') {} - }); - }) - `, - dedent` - describe('foo', () => { - beforeEach(() => { - if('bar') {} - }); - }) - `, - 'const foo = bar ? foo : baz;', - dedent` - it('valid', () => { - const values = something.map((thing) => { - if (thing.isFoo) { - return thing.foo - } else { - return thing.bar; - } - }); - - expect(values).toStrictEqual(['foo']); - }); - `, - dedent` - describe('valid', () => { - it('still valid', () => { - const values = something.map((thing) => { - if (thing.isFoo) { - return thing.foo - } else { - return thing.bar; - } - }); - - expect(values).toStrictEqual(['foo']); - }); - }); - `, - dedent` - describe('valid', () => { - describe('still valid', () => { - it('really still valid', () => { - const values = something.map((thing) => { - if (thing.isFoo) { - return thing.foo - } else { - return thing.bar; - } - }); - - expect(values).toStrictEqual(['foo']); - }); - }); - }); - `, - dedent` - it('foo', () => { - const foo = function(bar) { - if (bar) { - return 1; - } else { - return 2; - } - }; - }); - `, - dedent` - it('foo', () => { - function foo(bar) { - if (bar) { - return 1; - } else { - return 2; - } - }; - }); - `, - ], - invalid: [ - { - code: dedent` - it('foo', () => { - if('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it.skip('foo', () => { - if('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it.skip('foo', function () { - if('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it.only('foo', () => { - if('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - xit('foo', () => { - if('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - fit('foo', () => { - if('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - fit.concurrent('foo', () => { - if('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - test('foo', () => { - if('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - test.skip('foo', () => { - if('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - test.only('foo', () => { - if('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - xtest('foo', () => { - if('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - describe('foo', () => { - it('bar', () => { - if('bar') {} - }) - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: "it('foo', myTest); function myTest() { if ('bar') {} }", - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - describe('foo', () => { - it('bar', () => { - if('bar') {} - }) - it('baz', () => { - if('qux') {} - if('quux') {} - }) - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it('foo', () => { - callExpression() - if ('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it.each\`\`('foo', () => { - callExpression() - if ('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it.each()('foo', () => { - callExpression() - if ('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it.only.each\`\`('foo', () => { - callExpression() - if ('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - it.only.each()('foo', () => { - callExpression() - if ('bar') {} - }) - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - { - code: dedent` - describe('valid', () => { - describe('still valid', () => { - it('really still valid', () => { - const values = something.map((thing) => { - if (thing.isFoo) { - return thing.foo - } else { - return thing.bar; - } - }); - - if('invalid') { - expect(values).toStrictEqual(['foo']); - } - }); - }); - }); - `, - errors: [ - { - data: { condition: 'if' }, - messageId: 'conditionalInTest', - }, - ], - }, - ], -}); diff --git a/src/rules/no-if.ts b/src/rules/no-if.ts deleted file mode 100644 index 880c44b1c..000000000 --- a/src/rules/no-if.ts +++ /dev/null @@ -1,121 +0,0 @@ -import { AST_NODE_TYPES, type TSESTree } from '@typescript-eslint/utils'; -import { - TestCaseName, - createRule, - getAccessorValue, - getDeclaredVariables, - getNodeName, - getTestCallExpressionsFromDeclaredVariables, - parseJestFnCall, -} from './utils'; - -const testCaseNames = new Set([ - ...Object.keys(TestCaseName), - 'it.only', - 'it.only', - 'it.skip', - 'it.skip', - 'test.only', - 'test.only', - 'test.skip', - 'test.skip', - 'fit.concurrent', -]); - -const isTestFunctionExpression = ( - node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, -) => - node.parent !== undefined && - node.parent.type === AST_NODE_TYPES.CallExpression && - testCaseNames.has(getNodeName(node.parent.callee)); - -const conditionName = { - [AST_NODE_TYPES.ConditionalExpression]: 'conditional', - [AST_NODE_TYPES.SwitchStatement]: 'switch', - [AST_NODE_TYPES.IfStatement]: 'if', -}; - -export default createRule({ - name: __filename, - meta: { - docs: { - description: 'Disallow conditional logic', - category: 'Best Practices', - recommended: false, - }, - messages: { - conditionalInTest: 'Test should not contain {{ condition }} statements', - }, - deprecated: true, - replacedBy: ['no-conditional-in-test'], - schema: [], - type: 'suggestion', - }, - defaultOptions: [], - create(context) { - const stack: boolean[] = []; - - function validate( - node: - | TSESTree.ConditionalExpression - | TSESTree.SwitchStatement - | TSESTree.IfStatement, - ) { - const lastElementInStack = stack[stack.length - 1]; - - if (stack.length === 0 || !lastElementInStack) { - return; - } - - context.report({ - data: { condition: conditionName[node.type] }, - messageId: 'conditionalInTest', - node, - }); - } - - return { - CallExpression(node) { - const jestFnCall = parseJestFnCall(node, context); - - if (jestFnCall?.type === 'test') { - stack.push(true); - - if (jestFnCall.members.some(s => getAccessorValue(s) === 'each')) { - stack.push(true); - } - } - }, - FunctionExpression(node) { - stack.push(isTestFunctionExpression(node)); - }, - FunctionDeclaration(node) { - const declaredVariables = getDeclaredVariables(context, node); - const testCallExpressions = getTestCallExpressionsFromDeclaredVariables( - declaredVariables, - context, - ); - - stack.push(testCallExpressions.length > 0); - }, - ArrowFunctionExpression(node) { - stack.push(isTestFunctionExpression(node)); - }, - IfStatement: validate, - SwitchStatement: validate, - ConditionalExpression: validate, - 'CallExpression:exit'() { - stack.pop(); - }, - 'FunctionExpression:exit'() { - stack.pop(); - }, - 'FunctionDeclaration:exit'() { - stack.pop(); - }, - 'ArrowFunctionExpression:exit'() { - stack.pop(); - }, - }; - }, -});