Add error for missing await in conditionals

Jack-Works · elibarzilay · commit bb8b9db9f7ca · 2021-02-16T12:00:41.000-05:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -30731,7 +30731,7 @@ namespace ts {
                         if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken) {
                             if (operator === SyntaxKind.AmpersandAmpersandToken) {
                                 const parent = walkUpParenthesizedExpressions(node.parent);
-                                checkTestingKnownTruthyCallableType(node.left, leftType, isIfStatement(parent) ? parent.thenStatement : undefined);
+                                checkTestingKnownTruthyCallableOrAwaitableType(node.left, leftType, isIfStatement(parent) ? parent.thenStatement : undefined);
                             }
                             checkTruthinessOfType(leftType, node.left);
                         }
@@ -31275,7 +31275,7 @@ namespace ts {
 
         function checkConditionalExpression(node: ConditionalExpression, checkMode?: CheckMode): Type {
             const type = checkTruthinessExpression(node.condition);
-            checkTestingKnownTruthyCallableType(node.condition, type, node.whenTrue);
+            checkTestingKnownTruthyCallableOrAwaitableType(node.condition, type, node.whenTrue);
             const type1 = checkExpression(node.whenTrue, checkMode);
             const type2 = checkExpression(node.whenFalse, checkMode);
             return getUnionType([type1, type2], UnionReduction.Subtype);
@@ -34526,7 +34526,7 @@ namespace ts {
             // Grammar checking
             checkGrammarStatementInAmbientContext(node);
             const type = checkTruthinessExpression(node.expression);
-            checkTestingKnownTruthyCallableType(node.expression, type, node.thenStatement);
+            checkTestingKnownTruthyCallableOrAwaitableType(node.expression, type, node.thenStatement);
             checkSourceElement(node.thenStatement);
 
             if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
@@ -34536,8 +34536,16 @@ namespace ts {
             checkSourceElement(node.elseStatement);
         }
 
-        function checkTestingKnownTruthyCallableType(condExpr: Expression, type: Type, body: Statement | Expression | undefined) {
-            if (!strictNullChecks) {
+        function checkTestingKnownTruthyCallableOrAwaitableType(condExpr: Expression, type: Type, body?: Statement | Expression) {
+            if (!strictNullChecks) return;
+            if (getFalsyFlags(type)) return;
+
+            if (getAwaitedTypeOfPromise(type)) {
+                errorAndMaybeSuggestAwait(
+                    condExpr,
+                    /*maybeMissingAwait*/ true,
+                    Diagnostics.This_condition_will_always_return_0_since_the_types_1_and_2_have_no_overlap,
+                    "true", getTypeNameForErrorDisplay(type), "false");
                 return;
             }
 
@@ -34552,11 +34560,6 @@ namespace ts {
                 return;
             }
 
-            const possiblyFalsy = getFalsyFlags(type);
-            if (possiblyFalsy) {
-                return;
-            }
-
             // While it technically should be invalid for any known-truthy value
             // to be tested, we de-scope to functions unrefenced in the block as a
             // heuristic to identify the most common bugs. There are too many
diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -3256,6 +3256,10 @@
         "category": "Error",
         "code": 2800
     },
+    "This condition will always return true since the Promise is always truthy.": {
+        "category": "Error",
+        "code": 2801
+    },
 
     "Import declaration '{0}' is using private name '{1}'.": {
         "category": "Error",
diff --git a/tests/baselines/reference/truthinessPromiseCoercion.errors.txt b/tests/baselines/reference/truthinessPromiseCoercion.errors.txt
@@ -0,0 +1,24 @@
+tests/cases/compiler/truthinessPromiseCoercion.ts(5,9): error TS2367: This condition will always return 'true' since the types 'Promise<number>' and 'false' have no overlap.
+tests/cases/compiler/truthinessPromiseCoercion.ts(9,5): error TS2367: This condition will always return 'true' since the types 'Promise<number>' and 'false' have no overlap.
+
+
+==== tests/cases/compiler/truthinessPromiseCoercion.ts (2 errors) ====
+    declare const p: Promise<number>
+    declare const p2: null | Promise<number>
+    
+    async function f() {
+        if (p) {} // err
+            ~
+!!! error TS2367: This condition will always return 'true' since the types 'Promise<number>' and 'false' have no overlap.
+!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:5:9: Did you forget to use 'await'?
+        if (!!p) {} // no err
+        if (p2) {} // no err
+    
+        p ? f.arguments : f.arguments;
+        ~
+!!! error TS2367: This condition will always return 'true' since the types 'Promise<number>' and 'false' have no overlap.
+!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:9:5: Did you forget to use 'await'?
+        !!p ? f.arguments : f.arguments;
+        p2 ? f.arguments : f.arguments;
+    }
+    
diff --git a/tests/baselines/reference/truthinessPromiseCoercion.js b/tests/baselines/reference/truthinessPromiseCoercion.js
@@ -0,0 +1,24 @@
+//// [truthinessPromiseCoercion.ts]
+declare const p: Promise<number>
+declare const p2: null | Promise<number>
+
+async function f() {
+    if (p) {} // err
+    if (!!p) {} // no err
+    if (p2) {} // no err
+
+    p ? f.arguments : f.arguments;
+    !!p ? f.arguments : f.arguments;
+    p2 ? f.arguments : f.arguments;
+}
+
+
+//// [truthinessPromiseCoercion.js]
+async function f() {
+    if (p) { } // err
+    if (!!p) { } // no err
+    if (p2) { } // no err
+    p ? f.arguments : f.arguments;
+    !!p ? f.arguments : f.arguments;
+    p2 ? f.arguments : f.arguments;
+}
diff --git a/tests/baselines/reference/truthinessPromiseCoercion.symbols b/tests/baselines/reference/truthinessPromiseCoercion.symbols
@@ -0,0 +1,49 @@
+=== tests/cases/compiler/truthinessPromiseCoercion.ts ===
+declare const p: Promise<number>
+>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
+>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.promise.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2018.promise.d.ts, --, --))
+
+declare const p2: null | Promise<number>
+>p2 : Symbol(p2, Decl(truthinessPromiseCoercion.ts, 1, 13))
+>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.promise.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2018.promise.d.ts, --, --))
+
+async function f() {
+>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
+
+    if (p) {} // err
+>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
+
+    if (!!p) {} // no err
+>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
+
+    if (p2) {} // no err
+>p2 : Symbol(p2, Decl(truthinessPromiseCoercion.ts, 1, 13))
+
+    p ? f.arguments : f.arguments;
+>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
+>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
+>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
+>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+
+    !!p ? f.arguments : f.arguments;
+>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
+>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
+>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
+>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+
+    p2 ? f.arguments : f.arguments;
+>p2 : Symbol(p2, Decl(truthinessPromiseCoercion.ts, 1, 13))
+>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
+>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
+>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
+}
+
diff --git a/tests/baselines/reference/truthinessPromiseCoercion.types b/tests/baselines/reference/truthinessPromiseCoercion.types
@@ -0,0 +1,55 @@
+=== tests/cases/compiler/truthinessPromiseCoercion.ts ===
+declare const p: Promise<number>
+>p : Promise<number>
+
+declare const p2: null | Promise<number>
+>p2 : Promise<number> | null
+>null : null
+
+async function f() {
+>f : () => Promise<void>
+
+    if (p) {} // err
+>p : Promise<number>
+
+    if (!!p) {} // no err
+>!!p : true
+>!p : false
+>p : Promise<number>
+
+    if (p2) {} // no err
+>p2 : Promise<number> | null
+
+    p ? f.arguments : f.arguments;
+>p ? f.arguments : f.arguments : any
+>p : Promise<number>
+>f.arguments : any
+>f : () => Promise<void>
+>arguments : any
+>f.arguments : any
+>f : () => Promise<void>
+>arguments : any
+
+    !!p ? f.arguments : f.arguments;
+>!!p ? f.arguments : f.arguments : any
+>!!p : true
+>!p : false
+>p : Promise<number>
+>f.arguments : any
+>f : () => Promise<void>
+>arguments : any
+>f.arguments : any
+>f : () => Promise<void>
+>arguments : any
+
+    p2 ? f.arguments : f.arguments;
+>p2 ? f.arguments : f.arguments : any
+>p2 : Promise<number> | null
+>f.arguments : any
+>f : () => Promise<void>
+>arguments : any
+>f.arguments : any
+>f : () => Promise<void>
+>arguments : any
+}
+
diff --git a/tests/cases/compiler/truthinessPromiseCoercion.ts b/tests/cases/compiler/truthinessPromiseCoercion.ts
@@ -0,0 +1,15 @@
+// @strictNullChecks:true
+// @target:esnext
+
+declare const p: Promise<number>
+declare const p2: null | Promise<number>
+
+async function f() {
+    if (p) {} // err
+    if (!!p) {} // no err
+    if (p2) {} // no err
+
+    p ? f.arguments : f.arguments;
+    !!p ? f.arguments : f.arguments;
+    p2 ? f.arguments : f.arguments;
+}
diff --git a/tests/cases/fourslash/codeFixAddMissingAwait_condition.ts b/tests/cases/fourslash/codeFixAddMissingAwait_condition.ts
@@ -0,0 +1,27 @@
+/// <reference path="fourslash.ts" />
+
+// @strictNullChecks: true
+////async function fn(a: Promise<string[]>) {
+////  if (a) {};
+////  a ? fn.call() : fn.call();
+////}
+
+verify.codeFix({
+  description: ts.Diagnostics.Add_await.message,
+  index: 0,
+  newFileContent:
+`async function fn(a: Promise<string[]>) {
+  if (await a) {};
+  a ? fn.call() : fn.call();
+}`
+});
+
+verify.codeFix({
+  description: ts.Diagnostics.Add_await.message,
+  index: 1,
+  newFileContent:
+`async function fn(a: Promise<string[]>) {
+  if (a) {};
+  await a ? fn.call() : fn.call();
+}`
+});
