Add error for missing await in conditionals

2026-05-15 12:51:30 -05:00 · 2020-06-20 15:58:25 +08:00
parent ba994f1381
commit bb8b9db9f7
8 changed files with 211 additions and 10 deletions
--- 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
--- 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",
--- 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;
+    }
+    
--- 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;
+}
--- 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, --, --))
+}
+
--- 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
+}
+
--- 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;
+}
--- 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();
+}`
+});