diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 649c8b3db21..e0beb74c79c 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -103,6 +103,24 @@ export default util.createRule({ util.NullThrowsReasons.MissingToken('operator', node.type), ); + function* fix( + fixer: TSESLint.RuleFixer, + ): IterableIterator { + if (node.parent && util.isLogicalOrOperator(node.parent)) { + // '&&' and '??' operations cannot be mixed without parentheses (e.g. a && b ?? c) + if ( + node.left.type === AST_NODE_TYPES.LogicalExpression && + !util.isLogicalOrOperator(node.left.left) + ) { + yield fixer.insertTextBefore(node.left.right, '('); + } else { + yield fixer.insertTextBefore(node.left, '('); + } + yield fixer.insertTextAfter(node.right, ')'); + } + yield fixer.replaceText(barBarOperator, '??'); + } + const fixer = isMixedLogical || forceSuggestionFixer ? // suggestion instead for cases where we aren't sure if the fixer is completely safe @@ -110,17 +128,11 @@ export default util.createRule({ suggest: [ { messageId: 'preferNullish', - fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix { - return fixer.replaceText(barBarOperator, '??'); - }, + fix, }, ], } as const) - : { - fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix { - return fixer.replaceText(barBarOperator, '??'); - }, - }; + : { fix }; context.report({ node: barBarOperator, diff --git a/packages/eslint-plugin/src/util/astUtils.ts b/packages/eslint-plugin/src/util/astUtils.ts index 9398f4eb64b..328d25ae71c 100644 --- a/packages/eslint-plugin/src/util/astUtils.ts +++ b/packages/eslint-plugin/src/util/astUtils.ts @@ -1,7 +1,7 @@ import { - TSESTree, - AST_TOKEN_TYPES, AST_NODE_TYPES, + AST_TOKEN_TYPES, + TSESTree, } from '@typescript-eslint/experimental-utils'; const LINEBREAK_MATCHER = /\r\n|[\r\n\u2028\u2029]/; @@ -42,6 +42,15 @@ function isOptionalOptionalChain( ); } +/** + * Returns true if and only if the node represents logical OR + */ +function isLogicalOrOperator(node: TSESTree.Node): boolean { + return ( + node.type === AST_NODE_TYPES.LogicalExpression && node.operator === '||' + ); +} + /** * Determines whether two adjacent tokens are on the same line */ @@ -59,5 +68,6 @@ export { isOptionalChainPunctuator, isOptionalOptionalChain, isTokenOnSameLine, + isLogicalOrOperator, LINEBREAK_MATCHER, }; 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 7dd9212770c..015f7988fe1 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -317,7 +317,7 @@ declare const a: ${type} | ${nullish}; declare const b: ${type} | ${nullish}; declare const c: ${type} | ${nullish}; declare const d: ${type} | ${nullish}; -a ?? b || c && d; +(a ?? b) || c && d; `.trimRight(), }, ], @@ -367,7 +367,7 @@ declare const a: ${type} | ${nullish}; declare const b: ${type} | ${nullish}; declare const c: ${type} | ${nullish}; declare const d: ${type} | ${nullish}; -a && b ?? c || d; +a && (b ?? c) || d; `.trimRight(), }, ], @@ -463,5 +463,30 @@ x ?? 'foo'; }, ], }, + + // https://github.com/typescript-eslint/typescript-eslint/issues/1290 + ...nullishTypeInvalidTest((nullish, type) => ({ + code: ` +declare const a: ${type} | ${nullish}; +declare const b: ${type}; +declare const c: ${type}; +a || b || c; + `.trimRight(), + output: ` +declare const a: ${type} | ${nullish}; +declare const b: ${type}; +declare const c: ${type}; +(a ?? b) || c; + `.trimRight(), + errors: [ + { + messageId: 'preferNullish', + line: 5, + column: 3, + endLine: 5, + endColumn: 5, + }, + ], + })), ], });