From a92881ea7d15224970e6509dd7bd925c375e9373 Mon Sep 17 00:00:00 2001 From: mdm317 Date: Thu, 24 Apr 2025 19:13:22 +0900 Subject: [PATCH 1/5] fix : Add parentheses to expressions that require them, even when they are missing in the original code --- .../src/rules/prefer-nullish-coalescing.ts | 18 ++++- .../rules/prefer-nullish-coalescing.test.ts | 79 +++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index b05e43d0431e..977155cee41e 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -18,7 +18,9 @@ import { nullThrows, NullThrowsReasons, skipChainExpression, + isParenthesized, } from '../util'; +import { SourceCode } from '@typescript-eslint/utils/ts-eslint'; const isMemberAccessLike = isNodeOfTypes([ AST_NODE_TYPES.ChainExpression, @@ -512,7 +514,7 @@ export default createRule({ `${getTextWithParentheses( context.sourceCode, nullishCoalescingParams.nullishCoalescingLeftNode, - )} ?? ${getTextWithParentheses( + )} ?? ${getTextWithEnsuredParentheses( context.sourceCode, getBranchNodes(node, operator).nullishBranch, )}`, @@ -932,3 +934,17 @@ function formatComments( ) .join(''); } + +function getTextWithEnsuredParentheses( + sourceCode: Readonly, + node: TSESTree.Node, +): string { + const replaceCode = getTextWithParentheses(sourceCode, node); + if ( + node.type === AST_NODE_TYPES.ConditionalExpression && + !isParenthesized(node, sourceCode) + ) { + return `(${replaceCode})`; + } + return replaceCode; +} diff --git a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts index 0926a839ee38..01834adde9e2 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -6248,5 +6248,84 @@ function weirdParens() { ], output: null, }, + { + code: ` +let a: string | undefined +let b: { message: string } | undefined + +const foo = a + ? a + : (b + ? 1 : 2) + `, + errors: [ + { + messageId: 'preferNullishOverTernary', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | undefined +let b: { message: string } | undefined + +const foo = a ?? (b + ? 1 : 2) + `, + }, + ], + }, + ], + output: null, + }, + { + code: ` +let a: string | undefined +let b: { message: string } | undefined + +const foo = a + ? a + : b + ? 1 : 2 + `, + errors: [ + { + messageId: 'preferNullishOverTernary', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | undefined +let b: { message: string } | undefined + +const foo = a ?? (b + ? 1 : 2) + `, + }, + ], + }, + ], + output: null, + }, + { + code: ` +declare const c: string | null; +c !== null ? c : c ? 1 : 2 + `, + errors: [ + { + messageId: 'preferNullishOverTernary', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +declare const c: string | null; +c ?? (c ? 1 : 2) + `, + }, + ], + }, + ], + output: null, + }, ], }); From eb8ef5c8deea322f12de14def7498ee5f50f4885 Mon Sep 17 00:00:00 2001 From: mdm317 Date: Thu, 24 Apr 2025 19:28:14 +0900 Subject: [PATCH 2/5] fix : format --- .../rules/prefer-nullish-coalescing.test.ts | 38 ++++++++----------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts index 01834adde9e2..618d1c244414 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -6250,13 +6250,10 @@ function weirdParens() { }, { code: ` -let a: string | undefined -let b: { message: string } | undefined +let a: string | undefined; +let b: { message: string } | undefined; -const foo = a - ? a - : (b - ? 1 : 2) +const foo = a ? a : b ? 1 : 2; `, errors: [ { @@ -6265,11 +6262,10 @@ const foo = a { messageId: 'suggestNullish', output: ` -let a: string | undefined -let b: { message: string } | undefined +let a: string | undefined; +let b: { message: string } | undefined; -const foo = a ?? (b - ? 1 : 2) +const foo = a ?? (b ? 1 : 2); `, }, ], @@ -6278,14 +6274,11 @@ const foo = a ?? (b output: null, }, { - code: ` -let a: string | undefined -let b: { message: string } | undefined + code: noFormat` +let a: string | undefined; +let b: { message: string } | undefined; -const foo = a - ? a - : b - ? 1 : 2 +const foo = a ? a : (b ? 1 : 2); `, errors: [ { @@ -6294,11 +6287,10 @@ const foo = a { messageId: 'suggestNullish', output: ` -let a: string | undefined -let b: { message: string } | undefined +let a: string | undefined; +let b: { message: string } | undefined; -const foo = a ?? (b - ? 1 : 2) +const foo = a ?? (b ? 1 : 2); `, }, ], @@ -6309,7 +6301,7 @@ const foo = a ?? (b { code: ` declare const c: string | null; -c !== null ? c : c ? 1 : 2 +c !== null ? c : c ? 1 : 2; `, errors: [ { @@ -6319,7 +6311,7 @@ c !== null ? c : c ? 1 : 2 messageId: 'suggestNullish', output: ` declare const c: string | null; -c ?? (c ? 1 : 2) +c ?? (c ? 1 : 2); `, }, ], From ed97c64b6652e7235d8d4b096a980e23a221879e Mon Sep 17 00:00:00 2001 From: mdm317 Date: Thu, 24 Apr 2025 21:46:14 +0900 Subject: [PATCH 3/5] fix : lint err --- packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 977155cee41e..4e3a76bc0aba 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -1,4 +1,5 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; +import type { SourceCode } from '@typescript-eslint/utils/ts-eslint'; import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; @@ -20,7 +21,6 @@ import { skipChainExpression, isParenthesized, } from '../util'; -import { SourceCode } from '@typescript-eslint/utils/ts-eslint'; const isMemberAccessLike = isNodeOfTypes([ AST_NODE_TYPES.ChainExpression, From 07a6d712498a0f6077e5c132b93885640fd21d94 Mon Sep 17 00:00:00 2001 From: mdm317 Date: Tue, 29 Apr 2025 00:46:38 +0900 Subject: [PATCH 4/5] fix: leverage existing infrastructure --- .../src/rules/prefer-nullish-coalescing.ts | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 4e3a76bc0aba..8b34c6d3d0cd 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -1,5 +1,4 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; -import type { SourceCode } from '@typescript-eslint/utils/ts-eslint'; import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; @@ -20,7 +19,10 @@ import { NullThrowsReasons, skipChainExpression, isParenthesized, + getOperatorPrecedenceForNode, + OperatorPrecedence, } from '../util'; +import { getWrappedCode } from '../util/getWrappedCode'; const isMemberAccessLike = isNodeOfTypes([ AST_NODE_TYPES.ChainExpression, @@ -514,10 +516,23 @@ export default createRule({ `${getTextWithParentheses( context.sourceCode, nullishCoalescingParams.nullishCoalescingLeftNode, - )} ?? ${getTextWithEnsuredParentheses( - context.sourceCode, - getBranchNodes(node, operator).nullishBranch, - )}`, + )} ?? ${(() => { + const branch = getBranchNodes( + node, + operator, + ).nullishBranch; + if (isParenthesized(branch, context.sourceCode)) { + return getTextWithParentheses( + context.sourceCode, + branch, + ); + } + return getWrappedCode( + getTextWithParentheses(context.sourceCode, branch), + getOperatorPrecedenceForNode(branch), + OperatorPrecedence.Coalesce, + ); + })()}`, ); }, }, @@ -934,17 +949,3 @@ function formatComments( ) .join(''); } - -function getTextWithEnsuredParentheses( - sourceCode: Readonly, - node: TSESTree.Node, -): string { - const replaceCode = getTextWithParentheses(sourceCode, node); - if ( - node.type === AST_NODE_TYPES.ConditionalExpression && - !isParenthesized(node, sourceCode) - ) { - return `(${replaceCode})`; - } - return replaceCode; -} From dd690c4a03808da9f040028452da44ce84e885b0 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Thu, 1 May 2025 08:01:01 -0600 Subject: [PATCH 5/5] simplify syntax --- .../src/rules/prefer-nullish-coalescing.ts | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 8b34c6d3d0cd..8f4cffd98ffc 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -493,9 +493,14 @@ export default createRule({ return; } + const { nonNullishBranch, nullishBranch } = getBranchNodes( + node, + operator, + ); + const nullishCoalescingParams = getNullishCoalescingParams( node, - getBranchNodes(node, operator).nonNullishBranch, + nonNullishBranch, nodesInsideTestExpression, operator, ); @@ -511,28 +516,27 @@ export default createRule({ messageId: 'suggestNullish', data: { equals: '' }, fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix { + const nullishBranchText = getTextWithParentheses( + context.sourceCode, + nullishBranch, + ); + const rightOperandReplacement = isParenthesized( + nullishBranch, + context.sourceCode, + ) + ? nullishBranchText + : getWrappedCode( + nullishBranchText, + getOperatorPrecedenceForNode(nullishBranch), + OperatorPrecedence.Coalesce, + ); + return fixer.replaceText( node, `${getTextWithParentheses( context.sourceCode, nullishCoalescingParams.nullishCoalescingLeftNode, - )} ?? ${(() => { - const branch = getBranchNodes( - node, - operator, - ).nullishBranch; - if (isParenthesized(branch, context.sourceCode)) { - return getTextWithParentheses( - context.sourceCode, - branch, - ); - } - return getWrappedCode( - getTextWithParentheses(context.sourceCode, branch), - getOperatorPrecedenceForNode(branch), - OperatorPrecedence.Coalesce, - ); - })()}`, + )} ?? ${rightOperandReplacement}`, ); }, },