fix(eslint-plugin): eslint --fix outputs broken code (#1290)

typescript-eslint · Dec 11, 2019 · 888f59a · 888f59a
1 parent 9f76095
commit 888f59a
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 4 deletions.
diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts
@@ -117,8 +117,14 @@ export default util.createRule<Options, MessageIds>({
                 ],
               } as const)
             : {
-                fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix {
-                  return fixer.replaceText(barBarOperator, '??');
+                *fix(
+                  fixer: TSESLint.RuleFixer,
+                ): IterableIterator<TSESLint.RuleFix> {
+                  if (node.parent && util.isLogicalOrOperator(node.parent)) {
+                    yield fixer.insertTextBefore(node.left, '(');
+                    yield fixer.insertTextAfter(node.right, ')');
+                  }
+                  yield fixer.replaceText(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
@@ -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,
+        },
+      ],
+    })),
   ],
 });