From 092c96967fd9b58fb2d8d325e1dbc750ccbeb746 Mon Sep 17 00:00:00 2001
From: Brad Zacher <brad.zacher@gmail.com>
Date: Tue, 11 Aug 2020 12:57:25 -0700
Subject: [PATCH] fix(eslint-plugin): [no-unnecessary-condition] fix false
 positive with nullish coalescing (#2385)

---
 .../src/rules/no-unnecessary-condition.ts     |  9 +++++-
 .../rules/no-unnecessary-condition.test.ts    | 28 +++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
index 0103919e14a..22fec531779 100644
--- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
+++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
@@ -175,7 +175,14 @@ export default createRule<Options, MessageId>({
 
       // When checking logical expressions, only check the right side
       //  as the left side has been checked by checkLogicalExpressionForUnnecessaryConditionals
-      if (node.type === AST_NODE_TYPES.LogicalExpression) {
+      //
+      // Unless the node is nullish coalescing, as it's common to use patterns like `nullBool ?? true` to to strict
+      //  boolean checks if we inspect the right here, it'll usually be a constant condition on purpose.
+      // In this case it's better to inspect the type of the expression as a whole.
+      if (
+        node.type === AST_NODE_TYPES.LogicalExpression &&
+        node.operator !== '??'
+      ) {
         return checkNode(node.right);
       }
 
diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts
index ce65e717fa0..55b7a0e7fb1 100644
--- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts
+++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts
@@ -445,6 +445,14 @@ declare const key: Key;
 
 foo?.[key]?.trim();
     `,
+    // https://github.com/typescript-eslint/typescript-eslint/issues/2384
+    `
+function test(testVal?: boolean) {
+  if (testVal ?? true) {
+    console.log('test');
+  }
+}
+    `,
   ],
   invalid: [
     // Ensure that it's checking in all the right places
@@ -1367,5 +1375,25 @@ function Foo(outer: Outer, key: Bar): number | undefined {
         },
       ],
     },
+    // https://github.com/typescript-eslint/typescript-eslint/issues/2384
+    {
+      code: `
+function test(testVal?: true) {
+  if (testVal ?? true) {
+    console.log('test');
+  }
+}
+      `,
+      output: null,
+      errors: [
+        {
+          messageId: 'alwaysTruthy',
+          line: 3,
+          endLine: 3,
+          column: 7,
+          endColumn: 22,
+        },
+      ],
+    },
   ],
 });