From 1bcc7f7da57ef8fc18b76d3aef2e7cf8f07ffa3b Mon Sep 17 00:00:00 2001 From: Svyatoslav Zaytsev Date: Wed, 2 Nov 2022 09:46:30 +0300 Subject: [PATCH 1/4] fix(eslint-plugin): [prefer-optional-chain] fixer produces wrong logic (#1438) --- .../src/rules/prefer-optional-chain.ts | 15 ++++++++++++--- .../tests/rules/prefer-optional-chain.test.ts | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index dc9b514e353..0889ba32478 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -221,10 +221,19 @@ export default util.createRule({ if (expressionCount > 1) { if (previous.right.type === AST_NODE_TYPES.BinaryExpression) { + let operator = previous.right.operator; + if ( + previous.right.operator === '!==' && + previous.right.right.type === AST_NODE_TYPES.Literal && + previous.right.right.raw === 'null' + ) { + // case like foo !== null && foo.bar !== null + operator = '!='; + } // case like foo && foo.bar !== someValue - optionallyChainedCode += ` ${ - previous.right.operator - } ${sourceCode.getText(previous.right.right)}`; + optionallyChainedCode += ` ${operator} ${sourceCode.getText( + previous.right.right, + )}`; } context.report({ diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index 0358c065870..46e70b08b30 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -123,6 +123,22 @@ const baseCases = [ code: 'foo.bar && foo.bar?.() && foo.bar?.().baz', output: 'foo.bar?.()?.baz', }, + { + code: 'foo !== null && foo.bar !== null', + output: 'foo?.bar != null', + }, + { + code: 'foo != null && foo.bar != null', + output: 'foo?.bar != null', + }, + { + code: 'foo != null && foo.bar !== null', + output: 'foo?.bar != null', + }, + { + code: 'foo !== null && foo.bar != null', + output: 'foo?.bar != null', + }, ].map( c => ({ From 27a22ae4932db205d47a9c0967970287cfec7ae3 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 13 Nov 2022 18:48:50 -0500 Subject: [PATCH 2/4] Update packages/eslint-plugin/src/rules/prefer-optional-chain.ts --- packages/eslint-plugin/src/rules/prefer-optional-chain.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index 0889ba32478..c7638924f65 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -224,6 +224,7 @@ export default util.createRule({ let operator = previous.right.operator; if ( previous.right.operator === '!==' && + // TODO(#4820): Use the type checker to know whether this is `null` previous.right.right.type === AST_NODE_TYPES.Literal && previous.right.right.raw === 'null' ) { From 787f93844cfe9221c1fa0c4437e56a688569a249 Mon Sep 17 00:00:00 2001 From: Svyatoslav Zaytsev Date: Tue, 15 Nov 2022 07:16:23 +0300 Subject: [PATCH 3/4] fix(eslint-plugin): [prefer-optional-chain] fix tests --- .../tests/rules/prefer-optional-chain.test.ts | 429 ++++++++++-------- 1 file changed, 248 insertions(+), 181 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index 404826f6a43..0e7d1849ea9 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -11,165 +11,227 @@ const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', }); -const baseCases = [ - // chained members - { - code: 'foo && foo.bar', - output: 'foo?.bar', - }, - { - code: 'foo.bar && foo.bar.baz', - output: 'foo.bar?.baz', - }, - { - code: 'foo && foo()', - output: 'foo?.()', - }, - { - code: 'foo.bar && foo.bar()', - output: 'foo.bar?.()', - }, - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz', - output: 'foo?.bar?.baz?.buzz', - }, - { - code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz', - output: 'foo.bar?.baz?.buzz', - }, - // case with a jump (i.e. a non-nullish prop) - { - code: 'foo && foo.bar && foo.bar.baz.buzz', - output: 'foo?.bar?.baz.buzz', - }, - { - code: 'foo.bar && foo.bar.baz.buzz', - output: 'foo.bar?.baz.buzz', - }, - // case where for some reason there is a doubled up expression - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz', - output: 'foo?.bar?.baz?.buzz', - }, - { - code: 'foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz', - output: 'foo.bar?.baz?.buzz', - }, - // chained members with element access - { - code: 'foo && foo[bar] && foo[bar].baz && foo[bar].baz.buzz', - output: 'foo?.[bar]?.baz?.buzz', - }, - { +// eslint-disable-next-line @typescript-eslint/no-namespace +namespace BaseCases { + type InvalidTestCase = TSESLint.InvalidTestCase< + InferMessageIdsTypeFromRule, + InferOptionsTypeFromRule + >; + + interface BaseCase { + canReplaceAndWithOr: boolean; + output: string; + code: string; + } + + const mapper = (c: BaseCase): InvalidTestCase => ({ + code: c.code.trim(), + output: null, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: c.output.trim(), + }, + ], + }, + ], + }); + + const baseCases: Array = [ + // chained members + { + code: 'foo && foo.bar', + output: 'foo?.bar', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz', + output: 'foo.bar?.baz', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo()', + output: 'foo?.()', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar()', + output: 'foo.bar?.()', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz', + output: 'foo?.bar?.baz?.buzz', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz', + output: 'foo.bar?.baz?.buzz', + canReplaceAndWithOr: true, + }, // case with a jump (i.e. a non-nullish prop) - code: 'foo && foo[bar].baz && foo[bar].baz.buzz', - output: 'foo?.[bar].baz?.buzz', - }, - // case with a property access in computed property - { - code: 'foo && foo[bar.baz] && foo[bar.baz].buzz', - output: 'foo?.[bar.baz]?.buzz', - }, - // case with this keyword - { - code: 'foo[this.bar] && foo[this.bar].baz', - output: 'foo[this.bar]?.baz', - }, - // chained calls - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz()', - output: 'foo?.bar?.baz?.buzz()', - }, - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()', - output: 'foo?.bar?.baz?.buzz?.()', - }, - { - code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()', - output: 'foo.bar?.baz?.buzz?.()', - }, - // case with a jump (i.e. a non-nullish prop) - { - code: 'foo && foo.bar && foo.bar.baz.buzz()', - output: 'foo?.bar?.baz.buzz()', - }, - { - code: 'foo.bar && foo.bar.baz.buzz()', - output: 'foo.bar?.baz.buzz()', - }, - { + { + code: 'foo && foo.bar && foo.bar.baz.buzz', + output: 'foo?.bar?.baz.buzz', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz.buzz', + output: 'foo.bar?.baz.buzz', + canReplaceAndWithOr: true, + }, + // case where for some reason there is a doubled up expression + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz', + output: 'foo?.bar?.baz?.buzz', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz', + output: 'foo.bar?.baz?.buzz', + canReplaceAndWithOr: true, + }, + // chained members with element access + { + code: 'foo && foo[bar] && foo[bar].baz && foo[bar].baz.buzz', + output: 'foo?.[bar]?.baz?.buzz', + canReplaceAndWithOr: true, + }, + { + // case with a jump (i.e. a non-nullish prop) + code: 'foo && foo[bar].baz && foo[bar].baz.buzz', + output: 'foo?.[bar].baz?.buzz', + canReplaceAndWithOr: true, + }, + // case with a property access in computed property + { + code: 'foo && foo[bar.baz] && foo[bar.baz].buzz', + output: 'foo?.[bar.baz]?.buzz', + canReplaceAndWithOr: true, + }, + // case with this keyword + { + code: 'foo[this.bar] && foo[this.bar].baz', + output: 'foo[this.bar]?.baz', + canReplaceAndWithOr: true, + }, + // chained calls + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz()', + output: 'foo?.bar?.baz?.buzz()', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()', + output: 'foo?.bar?.baz?.buzz?.()', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()', + output: 'foo.bar?.baz?.buzz?.()', + canReplaceAndWithOr: true, + }, // case with a jump (i.e. a non-nullish prop) - code: 'foo && foo.bar && foo.bar.baz.buzz && foo.bar.baz.buzz()', - output: 'foo?.bar?.baz.buzz?.()', - }, - { - // case with a call expr inside the chain for some inefficient reason - code: 'foo && foo.bar() && foo.bar().baz && foo.bar().baz.buzz && foo.bar().baz.buzz()', - output: 'foo?.bar()?.baz?.buzz?.()', - }, - // chained calls with element access - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz[buzz]()', - output: 'foo?.bar?.baz?.[buzz]()', - }, - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz[buzz] && foo.bar.baz[buzz]()', - output: 'foo?.bar?.baz?.[buzz]?.()', - }, - // (partially) pre-optional chained - { - code: 'foo && foo?.bar && foo?.bar.baz && foo?.bar.baz[buzz] && foo?.bar.baz[buzz]()', - output: 'foo?.bar?.baz?.[buzz]?.()', - }, - { - code: 'foo && foo?.bar.baz && foo?.bar.baz[buzz]', - output: 'foo?.bar.baz?.[buzz]', - }, - { - code: 'foo && foo?.() && foo?.().bar', - output: 'foo?.()?.bar', - }, - { - code: 'foo.bar && foo.bar?.() && foo.bar?.().baz', - output: 'foo.bar?.()?.baz', - }, - { - code: 'foo !== null && foo.bar !== null', - output: 'foo?.bar != null', - }, - { - code: 'foo != null && foo.bar != null', - output: 'foo?.bar != null', - }, - { - code: 'foo != null && foo.bar !== null', - output: 'foo?.bar != null', - }, - { - code: 'foo !== null && foo.bar != null', - output: 'foo?.bar != null', - }, -].map( - c => - ({ - code: c.code.trim(), - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: c.output.trim(), - }, - ], - }, - ], - } as TSESLint.InvalidTestCase< - InferMessageIdsTypeFromRule, - InferOptionsTypeFromRule - >), -); + { + code: 'foo && foo.bar && foo.bar.baz.buzz()', + output: 'foo?.bar?.baz.buzz()', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz.buzz()', + output: 'foo.bar?.baz.buzz()', + canReplaceAndWithOr: true, + }, + { + // case with a jump (i.e. a non-nullish prop) + code: 'foo && foo.bar && foo.bar.baz.buzz && foo.bar.baz.buzz()', + output: 'foo?.bar?.baz.buzz?.()', + canReplaceAndWithOr: true, + }, + { + // case with a call expr inside the chain for some inefficient reason + code: 'foo && foo.bar() && foo.bar().baz && foo.bar().baz.buzz && foo.bar().baz.buzz()', + output: 'foo?.bar()?.baz?.buzz?.()', + canReplaceAndWithOr: true, + }, + // chained calls with element access + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz[buzz]()', + output: 'foo?.bar?.baz?.[buzz]()', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz[buzz] && foo.bar.baz[buzz]()', + output: 'foo?.bar?.baz?.[buzz]?.()', + canReplaceAndWithOr: true, + }, + // (partially) pre-optional chained + { + code: 'foo && foo?.bar && foo?.bar.baz && foo?.bar.baz[buzz] && foo?.bar.baz[buzz]()', + output: 'foo?.bar?.baz?.[buzz]?.()', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo?.bar.baz && foo?.bar.baz[buzz]', + output: 'foo?.bar.baz?.[buzz]', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo?.() && foo?.().bar', + output: 'foo?.()?.bar', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar?.() && foo.bar?.().baz', + output: 'foo.bar?.()?.baz', + canReplaceAndWithOr: true, + }, + { + code: 'foo !== null && foo.bar !== null', + output: 'foo?.bar != null', + canReplaceAndWithOr: false, + }, + { + code: 'foo != null && foo.bar != null', + output: 'foo?.bar != null', + canReplaceAndWithOr: false, + }, + { + code: 'foo != null && foo.bar !== null', + output: 'foo?.bar != null', + canReplaceAndWithOr: false, + }, + { + code: 'foo !== null && foo.bar != null', + output: 'foo?.bar != null', + canReplaceAndWithOr: false, + }, + ]; + + interface Selector { + all(): Array; + select>( + key: K, + value: BaseCase[K], + ): Selector; + } + + const selector = (cases: Array): Selector => ({ + all: () => cases.map(mapper), + select: >( + key: K, + value: BaseCase[K], + ): Selector => { + const selectedCases = baseCases.filter(c => c[key] === value); + return selector(selectedCases); + }, + }); + + export const { all, select } = selector(baseCases); +} ruleTester.run('prefer-optional-chain', rule, { valid: [ @@ -227,18 +289,18 @@ ruleTester.run('prefer-optional-chain', rule, { '!foo.bar!.baz || !foo.bar!.baz!.paz;', ], invalid: [ - ...baseCases, + ...BaseCases.all(), // it should ignore whitespace in the expressions - ...baseCases.map(c => ({ + ...BaseCases.all().map(c => ({ ...c, code: c.code.replace(/\./g, '. '), })), - ...baseCases.map(c => ({ + ...BaseCases.all().map(c => ({ ...c, code: c.code.replace(/\./g, '.\n'), })), // it should ignore parts of the expression that aren't part of the expression chain - ...baseCases.map(c => ({ + ...BaseCases.all().map(c => ({ ...c, code: `${c.code} && bing`, errors: [ @@ -253,7 +315,7 @@ ruleTester.run('prefer-optional-chain', rule, { }, ], })), - ...baseCases.map(c => ({ + ...BaseCases.all().map(c => ({ ...c, code: `${c.code} && bing.bong`, errors: [ @@ -269,22 +331,42 @@ ruleTester.run('prefer-optional-chain', rule, { ], })), // strict nullish equality checks x !== null && x.y !== null - ...baseCases.map(c => ({ + ...BaseCases.all().map(c => ({ ...c, code: c.code.replace(/&&/g, '!== null &&'), })), - ...baseCases.map(c => ({ + ...BaseCases.all().map(c => ({ ...c, code: c.code.replace(/&&/g, '!= null &&'), })), - ...baseCases.map(c => ({ + ...BaseCases.all().map(c => ({ ...c, code: c.code.replace(/&&/g, '!== undefined &&'), })), - ...baseCases.map(c => ({ + ...BaseCases.all().map(c => ({ ...c, code: c.code.replace(/&&/g, '!= undefined &&'), })), + + // replace && with ||: foo && foo.bar -> !foo || !foo.bar + ...BaseCases.select('canReplaceAndWithOr', true) + .all() + .map(c => ({ + ...c, + code: c.code.replace(/(^|\s)foo/g, '$1!foo').replace(/&&/g, '||'), + errors: [ + { + ...c.errors[0], + suggestions: [ + { + ...c.errors[0].suggestions![0], + output: `!${c.errors[0].suggestions![0].output}`, + }, + ], + }, + ], + })), + // two errors { code: noFormat`foo && foo.bar && foo.bar.baz || baz && baz.bar && baz.bar.foo`, @@ -1219,21 +1301,6 @@ foo?.bar(/* comment */a, }, ], }, - ...baseCases.map(c => ({ - ...c, - code: c.code.replace(/foo/g, '!foo').replace(/&&/g, '||'), - errors: [ - { - ...c.errors[0], - suggestions: [ - { - ...c.errors[0].suggestions![0], - output: `!${c.errors[0].suggestions![0].output}`, - }, - ], - }, - ], - })), // case with this keyword at the start of expression { code: '!this.bar || !this.bar.baz;', From 874c21a280aca5241f906e591d6ac1c0e9f9e676 Mon Sep 17 00:00:00 2001 From: Sviatoslav Miller Date: Mon, 21 Nov 2022 07:52:33 +0300 Subject: [PATCH 4/4] fix(eslint-plugin): [prefer-optional-chain] fix tests --- .../rules/prefer-optional-chain/base-cases.ts | 228 +++++++++++++++++ .../prefer-optional-chain.test.ts | 233 +----------------- 2 files changed, 231 insertions(+), 230 deletions(-) create mode 100644 packages/eslint-plugin/tests/rules/prefer-optional-chain/base-cases.ts rename packages/eslint-plugin/tests/rules/{ => prefer-optional-chain}/prefer-optional-chain.test.ts (80%) diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain/base-cases.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain/base-cases.ts new file mode 100644 index 00000000000..99cfe6b0ff9 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain/base-cases.ts @@ -0,0 +1,228 @@ +import type { TSESLint } from '@typescript-eslint/utils'; + +import type rule from '../../../src/rules/prefer-optional-chain'; +import type { + InferMessageIdsTypeFromRule, + InferOptionsTypeFromRule, +} from '../../../src/util'; + +type InvalidTestCase = TSESLint.InvalidTestCase< + InferMessageIdsTypeFromRule, + InferOptionsTypeFromRule +>; + +interface BaseCase { + canReplaceAndWithOr: boolean; + output: string; + code: string; +} + +const mapper = (c: BaseCase): InvalidTestCase => ({ + code: c.code.trim(), + output: null, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: c.output.trim(), + }, + ], + }, + ], +}); + +const baseCases: Array = [ + // chained members + { + code: 'foo && foo.bar', + output: 'foo?.bar', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz', + output: 'foo.bar?.baz', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo()', + output: 'foo?.()', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar()', + output: 'foo.bar?.()', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz', + output: 'foo?.bar?.baz?.buzz', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz', + output: 'foo.bar?.baz?.buzz', + canReplaceAndWithOr: true, + }, + // case with a jump (i.e. a non-nullish prop) + { + code: 'foo && foo.bar && foo.bar.baz.buzz', + output: 'foo?.bar?.baz.buzz', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz.buzz', + output: 'foo.bar?.baz.buzz', + canReplaceAndWithOr: true, + }, + // case where for some reason there is a doubled up expression + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz', + output: 'foo?.bar?.baz?.buzz', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz', + output: 'foo.bar?.baz?.buzz', + canReplaceAndWithOr: true, + }, + // chained members with element access + { + code: 'foo && foo[bar] && foo[bar].baz && foo[bar].baz.buzz', + output: 'foo?.[bar]?.baz?.buzz', + canReplaceAndWithOr: true, + }, + { + // case with a jump (i.e. a non-nullish prop) + code: 'foo && foo[bar].baz && foo[bar].baz.buzz', + output: 'foo?.[bar].baz?.buzz', + canReplaceAndWithOr: true, + }, + // case with a property access in computed property + { + code: 'foo && foo[bar.baz] && foo[bar.baz].buzz', + output: 'foo?.[bar.baz]?.buzz', + canReplaceAndWithOr: true, + }, + // case with this keyword + { + code: 'foo[this.bar] && foo[this.bar].baz', + output: 'foo[this.bar]?.baz', + canReplaceAndWithOr: true, + }, + // chained calls + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz()', + output: 'foo?.bar?.baz?.buzz()', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()', + output: 'foo?.bar?.baz?.buzz?.()', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()', + output: 'foo.bar?.baz?.buzz?.()', + canReplaceAndWithOr: true, + }, + // case with a jump (i.e. a non-nullish prop) + { + code: 'foo && foo.bar && foo.bar.baz.buzz()', + output: 'foo?.bar?.baz.buzz()', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar.baz.buzz()', + output: 'foo.bar?.baz.buzz()', + canReplaceAndWithOr: true, + }, + { + // case with a jump (i.e. a non-nullish prop) + code: 'foo && foo.bar && foo.bar.baz.buzz && foo.bar.baz.buzz()', + output: 'foo?.bar?.baz.buzz?.()', + canReplaceAndWithOr: true, + }, + { + // case with a call expr inside the chain for some inefficient reason + code: 'foo && foo.bar() && foo.bar().baz && foo.bar().baz.buzz && foo.bar().baz.buzz()', + output: 'foo?.bar()?.baz?.buzz?.()', + canReplaceAndWithOr: true, + }, + // chained calls with element access + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz[buzz]()', + output: 'foo?.bar?.baz?.[buzz]()', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz[buzz] && foo.bar.baz[buzz]()', + output: 'foo?.bar?.baz?.[buzz]?.()', + canReplaceAndWithOr: true, + }, + // (partially) pre-optional chained + { + code: 'foo && foo?.bar && foo?.bar.baz && foo?.bar.baz[buzz] && foo?.bar.baz[buzz]()', + output: 'foo?.bar?.baz?.[buzz]?.()', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo?.bar.baz && foo?.bar.baz[buzz]', + output: 'foo?.bar.baz?.[buzz]', + canReplaceAndWithOr: true, + }, + { + code: 'foo && foo?.() && foo?.().bar', + output: 'foo?.()?.bar', + canReplaceAndWithOr: true, + }, + { + code: 'foo.bar && foo.bar?.() && foo.bar?.().baz', + output: 'foo.bar?.()?.baz', + canReplaceAndWithOr: true, + }, + { + code: 'foo !== null && foo.bar !== null', + output: 'foo?.bar != null', + canReplaceAndWithOr: false, + }, + { + code: 'foo != null && foo.bar != null', + output: 'foo?.bar != null', + canReplaceAndWithOr: false, + }, + { + code: 'foo != null && foo.bar !== null', + output: 'foo?.bar != null', + canReplaceAndWithOr: false, + }, + { + code: 'foo !== null && foo.bar != null', + output: 'foo?.bar != null', + canReplaceAndWithOr: false, + }, +]; + +interface Selector { + all(): Array; + select>( + key: K, + value: BaseCase[K], + ): Selector; +} + +const selector = (cases: Array): Selector => ({ + all: () => cases.map(mapper), + select: >( + key: K, + value: BaseCase[K], + ): Selector => { + const selectedCases = baseCases.filter(c => c[key] === value); + return selector(selectedCases); + }, +}); + +const { all, select } = selector(baseCases); + +export { all, select }; diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain/prefer-optional-chain.test.ts similarity index 80% rename from packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts rename to packages/eslint-plugin/tests/rules/prefer-optional-chain/prefer-optional-chain.test.ts index 0e7d1849ea9..1c2d14b03ce 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain/prefer-optional-chain.test.ts @@ -1,238 +1,11 @@ -import type { TSESLint } from '@typescript-eslint/utils'; - -import rule from '../../src/rules/prefer-optional-chain'; -import type { - InferMessageIdsTypeFromRule, - InferOptionsTypeFromRule, -} from '../../src/util'; -import { noFormat, RuleTester } from '../RuleTester'; +import rule from '../../../src/rules/prefer-optional-chain'; +import { noFormat, RuleTester } from '../../RuleTester'; +import * as BaseCases from './base-cases'; const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', }); -// eslint-disable-next-line @typescript-eslint/no-namespace -namespace BaseCases { - type InvalidTestCase = TSESLint.InvalidTestCase< - InferMessageIdsTypeFromRule, - InferOptionsTypeFromRule - >; - - interface BaseCase { - canReplaceAndWithOr: boolean; - output: string; - code: string; - } - - const mapper = (c: BaseCase): InvalidTestCase => ({ - code: c.code.trim(), - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: c.output.trim(), - }, - ], - }, - ], - }); - - const baseCases: Array = [ - // chained members - { - code: 'foo && foo.bar', - output: 'foo?.bar', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz', - output: 'foo.bar?.baz', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo()', - output: 'foo?.()', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar()', - output: 'foo.bar?.()', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz', - output: 'foo?.bar?.baz?.buzz', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz', - output: 'foo.bar?.baz?.buzz', - canReplaceAndWithOr: true, - }, - // case with a jump (i.e. a non-nullish prop) - { - code: 'foo && foo.bar && foo.bar.baz.buzz', - output: 'foo?.bar?.baz.buzz', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz.buzz', - output: 'foo.bar?.baz.buzz', - canReplaceAndWithOr: true, - }, - // case where for some reason there is a doubled up expression - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz', - output: 'foo?.bar?.baz?.buzz', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz', - output: 'foo.bar?.baz?.buzz', - canReplaceAndWithOr: true, - }, - // chained members with element access - { - code: 'foo && foo[bar] && foo[bar].baz && foo[bar].baz.buzz', - output: 'foo?.[bar]?.baz?.buzz', - canReplaceAndWithOr: true, - }, - { - // case with a jump (i.e. a non-nullish prop) - code: 'foo && foo[bar].baz && foo[bar].baz.buzz', - output: 'foo?.[bar].baz?.buzz', - canReplaceAndWithOr: true, - }, - // case with a property access in computed property - { - code: 'foo && foo[bar.baz] && foo[bar.baz].buzz', - output: 'foo?.[bar.baz]?.buzz', - canReplaceAndWithOr: true, - }, - // case with this keyword - { - code: 'foo[this.bar] && foo[this.bar].baz', - output: 'foo[this.bar]?.baz', - canReplaceAndWithOr: true, - }, - // chained calls - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz()', - output: 'foo?.bar?.baz?.buzz()', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()', - output: 'foo?.bar?.baz?.buzz?.()', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()', - output: 'foo.bar?.baz?.buzz?.()', - canReplaceAndWithOr: true, - }, - // case with a jump (i.e. a non-nullish prop) - { - code: 'foo && foo.bar && foo.bar.baz.buzz()', - output: 'foo?.bar?.baz.buzz()', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz.buzz()', - output: 'foo.bar?.baz.buzz()', - canReplaceAndWithOr: true, - }, - { - // case with a jump (i.e. a non-nullish prop) - code: 'foo && foo.bar && foo.bar.baz.buzz && foo.bar.baz.buzz()', - output: 'foo?.bar?.baz.buzz?.()', - canReplaceAndWithOr: true, - }, - { - // case with a call expr inside the chain for some inefficient reason - code: 'foo && foo.bar() && foo.bar().baz && foo.bar().baz.buzz && foo.bar().baz.buzz()', - output: 'foo?.bar()?.baz?.buzz?.()', - canReplaceAndWithOr: true, - }, - // chained calls with element access - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz[buzz]()', - output: 'foo?.bar?.baz?.[buzz]()', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz[buzz] && foo.bar.baz[buzz]()', - output: 'foo?.bar?.baz?.[buzz]?.()', - canReplaceAndWithOr: true, - }, - // (partially) pre-optional chained - { - code: 'foo && foo?.bar && foo?.bar.baz && foo?.bar.baz[buzz] && foo?.bar.baz[buzz]()', - output: 'foo?.bar?.baz?.[buzz]?.()', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo?.bar.baz && foo?.bar.baz[buzz]', - output: 'foo?.bar.baz?.[buzz]', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo?.() && foo?.().bar', - output: 'foo?.()?.bar', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar?.() && foo.bar?.().baz', - output: 'foo.bar?.()?.baz', - canReplaceAndWithOr: true, - }, - { - code: 'foo !== null && foo.bar !== null', - output: 'foo?.bar != null', - canReplaceAndWithOr: false, - }, - { - code: 'foo != null && foo.bar != null', - output: 'foo?.bar != null', - canReplaceAndWithOr: false, - }, - { - code: 'foo != null && foo.bar !== null', - output: 'foo?.bar != null', - canReplaceAndWithOr: false, - }, - { - code: 'foo !== null && foo.bar != null', - output: 'foo?.bar != null', - canReplaceAndWithOr: false, - }, - ]; - - interface Selector { - all(): Array; - select>( - key: K, - value: BaseCase[K], - ): Selector; - } - - const selector = (cases: Array): Selector => ({ - all: () => cases.map(mapper), - select: >( - key: K, - value: BaseCase[K], - ): Selector => { - const selectedCases = baseCases.filter(c => c[key] === value); - return selector(selectedCases); - }, - }); - - export const { all, select } = selector(baseCases); -} - ruleTester.run('prefer-optional-chain', rule, { valid: [ '!a || !b;',