fix(eslint-plugin): [prefer-null-coal] fixer w/ mixed logicals (#1326)

dimabory · bradzacher · commit f9a9fbfc83fe · 2019-12-16T21:50:18.000+10:30
diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts
@@ -103,24 +103,36 @@ export default util.createRule<Options, MessageIds>({
           util.NullThrowsReasons.MissingToken('operator', node.type),
         );
 
+        function* fix(
+          fixer: TSESLint.RuleFixer,
+        ): IterableIterator<TSESLint.RuleFix> {
+          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
               ({
                 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
@@ -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
@@ -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,
+        },
+      ],
+    })),
   ],
 });
