Skip to content

Commit bb8b9db

Browse files
Jack-Workselibarzilay
Jack-Works
authored and
elibarzilay
committed
Add error for missing await in conditionals
1 parent ba994f1 commit bb8b9db

File tree

8 files changed

+211
-10
lines changed

8 files changed

+211
-10
lines changed

‎src/compiler/checker.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30731,7 +30731,7 @@ namespace ts {
3073130731
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken) {
3073230732
if (operator === SyntaxKind.AmpersandAmpersandToken) {
3073330733
const parent = walkUpParenthesizedExpressions(node.parent);
30734-
checkTestingKnownTruthyCallableType(node.left, leftType, isIfStatement(parent) ? parent.thenStatement : undefined);
30734+
checkTestingKnownTruthyCallableOrAwaitableType(node.left, leftType, isIfStatement(parent) ? parent.thenStatement : undefined);
3073530735
}
3073630736
checkTruthinessOfType(leftType, node.left);
3073730737
}
@@ -31275,7 +31275,7 @@ namespace ts {
3127531275

3127631276
function checkConditionalExpression(node: ConditionalExpression, checkMode?: CheckMode): Type {
3127731277
const type = checkTruthinessExpression(node.condition);
31278-
checkTestingKnownTruthyCallableType(node.condition, type, node.whenTrue);
31278+
checkTestingKnownTruthyCallableOrAwaitableType(node.condition, type, node.whenTrue);
3127931279
const type1 = checkExpression(node.whenTrue, checkMode);
3128031280
const type2 = checkExpression(node.whenFalse, checkMode);
3128131281
return getUnionType([type1, type2], UnionReduction.Subtype);
@@ -34526,7 +34526,7 @@ namespace ts {
3452634526
// Grammar checking
3452734527
checkGrammarStatementInAmbientContext(node);
3452834528
const type = checkTruthinessExpression(node.expression);
34529-
checkTestingKnownTruthyCallableType(node.expression, type, node.thenStatement);
34529+
checkTestingKnownTruthyCallableOrAwaitableType(node.expression, type, node.thenStatement);
3453034530
checkSourceElement(node.thenStatement);
3453134531

3453234532
if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
@@ -34536,8 +34536,16 @@ namespace ts {
3453634536
checkSourceElement(node.elseStatement);
3453734537
}
3453834538

34539-
function checkTestingKnownTruthyCallableType(condExpr: Expression, type: Type, body: Statement | Expression | undefined) {
34540-
if (!strictNullChecks) {
34539+
function checkTestingKnownTruthyCallableOrAwaitableType(condExpr: Expression, type: Type, body?: Statement | Expression) {
34540+
if (!strictNullChecks) return;
34541+
if (getFalsyFlags(type)) return;
34542+
34543+
if (getAwaitedTypeOfPromise(type)) {
34544+
errorAndMaybeSuggestAwait(
34545+
condExpr,
34546+
/*maybeMissingAwait*/ true,
34547+
Diagnostics.This_condition_will_always_return_0_since_the_types_1_and_2_have_no_overlap,
34548+
"true", getTypeNameForErrorDisplay(type), "false");
3454134549
return;
3454234550
}
3454334551

@@ -34552,11 +34560,6 @@ namespace ts {
3455234560
return;
3455334561
}
3455434562

34555-
const possiblyFalsy = getFalsyFlags(type);
34556-
if (possiblyFalsy) {
34557-
return;
34558-
}
34559-
3456034563
// While it technically should be invalid for any known-truthy value
3456134564
// to be tested, we de-scope to functions unrefenced in the block as a
3456234565
// heuristic to identify the most common bugs. There are too many

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3256,6 +3256,10 @@
32563256
"category": "Error",
32573257
"code": 2800
32583258
},
3259+
"This condition will always return true since the Promise is always truthy.": {
3260+
"category": "Error",
3261+
"code": 2801
3262+
},
32593263

32603264
"Import declaration '{0}' is using private name '{1}'.": {
32613265
"category": "Error",

‎tests/baselines/reference/truthinessPromiseCoercion.errors.txt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
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.
2+
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.
3+
4+
5+
==== tests/cases/compiler/truthinessPromiseCoercion.ts (2 errors) ====
6+
declare const p: Promise<number>
7+
declare const p2: null | Promise<number>
8+
9+
async function f() {
10+
if (p) {} // err
11+
~
12+
!!! error TS2367: This condition will always return 'true' since the types 'Promise<number>' and 'false' have no overlap.
13+
!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:5:9: Did you forget to use 'await'?
14+
if (!!p) {} // no err
15+
if (p2) {} // no err
16+
17+
p ? f.arguments : f.arguments;
18+
~
19+
!!! error TS2367: This condition will always return 'true' since the types 'Promise<number>' and 'false' have no overlap.
20+
!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:9:5: Did you forget to use 'await'?
21+
!!p ? f.arguments : f.arguments;
22+
p2 ? f.arguments : f.arguments;
23+
}
24+

‎tests/baselines/reference/truthinessPromiseCoercion.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//// [truthinessPromiseCoercion.ts]
2+
declare const p: Promise<number>
3+
declare const p2: null | Promise<number>
4+
5+
async function f() {
6+
if (p) {} // err
7+
if (!!p) {} // no err
8+
if (p2) {} // no err
9+
10+
p ? f.arguments : f.arguments;
11+
!!p ? f.arguments : f.arguments;
12+
p2 ? f.arguments : f.arguments;
13+
}
14+
15+
16+
//// [truthinessPromiseCoercion.js]
17+
async function f() {
18+
if (p) { } // err
19+
if (!!p) { } // no err
20+
if (p2) { } // no err
21+
p ? f.arguments : f.arguments;
22+
!!p ? f.arguments : f.arguments;
23+
p2 ? f.arguments : f.arguments;
24+
}

‎tests/baselines/reference/truthinessPromiseCoercion.symbols

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
=== tests/cases/compiler/truthinessPromiseCoercion.ts ===
2+
declare const p: Promise<number>
3+
>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
4+
>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, --, --))
5+
6+
declare const p2: null | Promise<number>
7+
>p2 : Symbol(p2, Decl(truthinessPromiseCoercion.ts, 1, 13))
8+
>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, --, --))
9+
10+
async function f() {
11+
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
12+
13+
if (p) {} // err
14+
>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
15+
16+
if (!!p) {} // no err
17+
>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
18+
19+
if (p2) {} // no err
20+
>p2 : Symbol(p2, Decl(truthinessPromiseCoercion.ts, 1, 13))
21+
22+
p ? f.arguments : f.arguments;
23+
>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
24+
>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
25+
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
26+
>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
27+
>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
28+
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
29+
>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
30+
31+
!!p ? f.arguments : f.arguments;
32+
>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
33+
>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
34+
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
35+
>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
36+
>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
37+
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
38+
>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
39+
40+
p2 ? f.arguments : f.arguments;
41+
>p2 : Symbol(p2, Decl(truthinessPromiseCoercion.ts, 1, 13))
42+
>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
43+
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
44+
>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
45+
>f.arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
46+
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 1, 40))
47+
>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
48+
}
49+

‎tests/baselines/reference/truthinessPromiseCoercion.types

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
=== tests/cases/compiler/truthinessPromiseCoercion.ts ===
2+
declare const p: Promise<number>
3+
>p : Promise<number>
4+
5+
declare const p2: null | Promise<number>
6+
>p2 : Promise<number> | null
7+
>null : null
8+
9+
async function f() {
10+
>f : () => Promise<void>
11+
12+
if (p) {} // err
13+
>p : Promise<number>
14+
15+
if (!!p) {} // no err
16+
>!!p : true
17+
>!p : false
18+
>p : Promise<number>
19+
20+
if (p2) {} // no err
21+
>p2 : Promise<number> | null
22+
23+
p ? f.arguments : f.arguments;
24+
>p ? f.arguments : f.arguments : any
25+
>p : Promise<number>
26+
>f.arguments : any
27+
>f : () => Promise<void>
28+
>arguments : any
29+
>f.arguments : any
30+
>f : () => Promise<void>
31+
>arguments : any
32+
33+
!!p ? f.arguments : f.arguments;
34+
>!!p ? f.arguments : f.arguments : any
35+
>!!p : true
36+
>!p : false
37+
>p : Promise<number>
38+
>f.arguments : any
39+
>f : () => Promise<void>
40+
>arguments : any
41+
>f.arguments : any
42+
>f : () => Promise<void>
43+
>arguments : any
44+
45+
p2 ? f.arguments : f.arguments;
46+
>p2 ? f.arguments : f.arguments : any
47+
>p2 : Promise<number> | null
48+
>f.arguments : any
49+
>f : () => Promise<void>
50+
>arguments : any
51+
>f.arguments : any
52+
>f : () => Promise<void>
53+
>arguments : any
54+
}
55+

‎tests/cases/compiler/truthinessPromiseCoercion.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// @strictNullChecks:true
2+
// @target:esnext
3+
4+
declare const p: Promise<number>
5+
declare const p2: null | Promise<number>
6+
7+
async function f() {
8+
if (p) {} // err
9+
if (!!p) {} // no err
10+
if (p2) {} // no err
11+
12+
p ? f.arguments : f.arguments;
13+
!!p ? f.arguments : f.arguments;
14+
p2 ? f.arguments : f.arguments;
15+
}

‎tests/cases/fourslash/codeFixAddMissingAwait_condition.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @strictNullChecks: true
4+
////async function fn(a: Promise<string[]>) {
5+
//// if (a) {};
6+
//// a ? fn.call() : fn.call();
7+
////}
8+
9+
verify.codeFix({
10+
description: ts.Diagnostics.Add_await.message,
11+
index: 0,
12+
newFileContent:
13+
`async function fn(a: Promise<string[]>) {
14+
if (await a) {};
15+
a ? fn.call() : fn.call();
16+
}`
17+
});
18+
19+
verify.codeFix({
20+
description: ts.Diagnostics.Add_await.message,
21+
index: 1,
22+
newFileContent:
23+
`async function fn(a: Promise<string[]>) {
24+
if (a) {};
25+
await a ? fn.call() : fn.call();
26+
}`
27+
});

0 commit comments

Comments
 (0)