Skip to content

Commit

Permalink
Issue unawaited promise error on symbol-less expressions (#44491)
Browse files Browse the repository at this point in the history
* Issue unawaited promise error on symbol-less expressions

* Use same behavior for call expressions

* Revert "Use same behavior for call expressions"

This reverts commit 60d5813.
  • Loading branch information
andrewbranch committed Jun 28, 2021
1 parent 4890312 commit 54b913c
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 25 deletions.
17 changes: 8 additions & 9 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35870,15 +35870,14 @@ namespace ts {
if (getFalsyFlags(type)) return;

const location = isBinaryExpression(condExpr) ? condExpr.right : condExpr;
if (isPropertyAccessExpression(location) && isAssertionExpression(skipParentheses(location.expression))) {
return;
}

const testedNode = isIdentifier(location) ? location
: isPropertyAccessExpression(location) ? location.name
: isBinaryExpression(location) && isIdentifier(location.right) ? location.right
: undefined;
const isPropertyExpressionCast = isPropertyAccessExpression(location)
&& isAssertionExpression(skipParentheses(location.expression));
if (!testedNode || isPropertyExpressionCast) {
return;
}

// While it technically should be invalid for any known-truthy value
// to be tested, we de-scope to functions and Promises unreferenced in
Expand All @@ -35891,13 +35890,13 @@ namespace ts {
return;
}

const testedSymbol = getSymbolAtLocation(testedNode);
if (!testedSymbol) {
const testedSymbol = testedNode && getSymbolAtLocation(testedNode);
if (!testedSymbol && !isPromise) {
return;
}

const isUsed = isBinaryExpression(condExpr.parent) && isSymbolUsedInBinaryExpressionChain(condExpr.parent, testedSymbol)
|| body && isSymbolUsedInConditionBody(condExpr, body, testedNode, testedSymbol);
const isUsed = testedSymbol && isBinaryExpression(condExpr.parent) && isSymbolUsedInBinaryExpressionChain(condExpr.parent, testedSymbol)
|| testedSymbol && body && isSymbolUsedInConditionBody(condExpr, body, testedNode!, testedSymbol);
if (!isUsed) {
if (isPromise) {
errorAndMaybeSuggestAwait(
Expand Down
33 changes: 26 additions & 7 deletions tests/baselines/reference/truthinessPromiseCoercion.errors.txt
Original file line number Diff line number Diff line change
@@ -1,25 +1,28 @@
tests/cases/compiler/truthinessPromiseCoercion.ts(6,9): error TS2801: This condition will always return true since this 'Promise<number>' is always defined.
tests/cases/compiler/truthinessPromiseCoercion.ts(10,5): error TS2801: This condition will always return true since this 'Promise<number>' is always defined.
tests/cases/compiler/truthinessPromiseCoercion.ts(31,9): error TS2801: This condition will always return true since this 'Promise<unknown>' is always defined.
tests/cases/compiler/truthinessPromiseCoercion.ts(7,9): error TS2801: This condition will always return true since this 'Promise<number>' is always defined.
tests/cases/compiler/truthinessPromiseCoercion.ts(11,5): error TS2801: This condition will always return true since this 'Promise<number>' is always defined.
tests/cases/compiler/truthinessPromiseCoercion.ts(32,9): error TS2801: This condition will always return true since this 'Promise<unknown>' is always defined.
tests/cases/compiler/truthinessPromiseCoercion.ts(40,9): error TS2801: This condition will always return true since this 'Promise<boolean>' is always defined.
tests/cases/compiler/truthinessPromiseCoercion.ts(43,9): error TS2801: This condition will always return true since this 'Promise<boolean>' is always defined.


==== tests/cases/compiler/truthinessPromiseCoercion.ts (3 errors) ====
==== tests/cases/compiler/truthinessPromiseCoercion.ts (5 errors) ====
declare const p: Promise<number>
declare const p2: null | Promise<number>
declare const obj: { p: Promise<unknown> }
declare function pf(): Promise<boolean>

async function f() {
if (p) {} // err
~
!!! error TS2801: This condition will always return true since this 'Promise<number>' is always defined.
!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:6:9: Did you forget to use 'await'?
!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:7:9: Did you forget to use 'await'?
if (!!p) {} // no err
if (p2) {} // no err

p ? f.arguments : f.arguments;
~
!!! error TS2801: This condition will always return true since this 'Promise<number>' is always defined.
!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:10:5: Did you forget to use 'await'?
!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:11:5: Did you forget to use 'await'?
!!p ? f.arguments : f.arguments;
p2 ? f.arguments : f.arguments;
}
Expand All @@ -43,10 +46,26 @@ tests/cases/compiler/truthinessPromiseCoercion.ts(31,9): error TS2801: This cond
if (obj.p) {} // error
~~~~~
!!! error TS2801: This condition will always return true since this 'Promise<unknown>' is always defined.
!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:31:9: Did you forget to use 'await'?
!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:32:9: Did you forget to use 'await'?
if (obj.p) { // ok
await obj.p;
}
if (obj.p && await obj.p) {} // ok
}

async function i(): Promise<string> {
if (pf()) { // error
~~~~
!!! error TS2801: This condition will always return true since this 'Promise<boolean>' is always defined.
!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:40:9: Did you forget to use 'await'?
return "true";
}
if (pf()) { // error
~~~~
!!! error TS2801: This condition will always return true since this 'Promise<boolean>' is always defined.
!!! related TS2773 tests/cases/compiler/truthinessPromiseCoercion.ts:43:9: Did you forget to use 'await'?
pf().then();
}
return "false";
}

20 changes: 20 additions & 0 deletions tests/baselines/reference/truthinessPromiseCoercion.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
declare const p: Promise<number>
declare const p2: null | Promise<number>
declare const obj: { p: Promise<unknown> }
declare function pf(): Promise<boolean>

async function f() {
if (p) {} // err
Expand Down Expand Up @@ -35,6 +36,16 @@ async function h() {
}
if (obj.p && await obj.p) {} // ok
}

async function i(): Promise<string> {
if (pf()) { // error
return "true";
}
if (pf()) { // error
pf().then();
}
return "false";
}


//// [truthinessPromiseCoercion.js]
Expand Down Expand Up @@ -67,3 +78,12 @@ async function h() {
}
if (obj.p && await obj.p) { } // ok
}
async function i() {
if (pf()) { // error
return "true";
}
if (pf()) { // error
pf().then();
}
return "false";
}
42 changes: 33 additions & 9 deletions tests/baselines/reference/truthinessPromiseCoercion.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ declare const obj: { p: Promise<unknown> }
>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 2, 20))
>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 function pf(): Promise<boolean>
>pf : Symbol(pf, Decl(truthinessPromiseCoercion.ts, 2, 42))
>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, 2, 42))
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 3, 39))

if (p) {} // err
>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
Expand All @@ -27,34 +31,34 @@ async function f() {
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, 2, 42))
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 3, 39))
>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, 2, 42))
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 3, 39))
>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, 2, 42))
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 3, 39))
>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, 2, 42))
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 3, 39))
>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, 2, 42))
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 3, 39))
>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, 2, 42))
>f : Symbol(f, Decl(truthinessPromiseCoercion.ts, 3, 39))
>arguments : Symbol(Function.arguments, Decl(lib.es5.d.ts, --, --))
}

// all ok
async function g() {
>g : Symbol(g, Decl(truthinessPromiseCoercion.ts, 12, 1))
>g : Symbol(g, Decl(truthinessPromiseCoercion.ts, 13, 1))

if (p) {
>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 0, 13))
Expand Down Expand Up @@ -87,7 +91,7 @@ async function g() {
}

async function h() {
>h : Symbol(h, Decl(truthinessPromiseCoercion.ts, 27, 1))
>h : Symbol(h, Decl(truthinessPromiseCoercion.ts, 28, 1))

if (obj.p) {} // error
>obj.p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 2, 20))
Expand All @@ -113,3 +117,23 @@ async function h() {
>p : Symbol(p, Decl(truthinessPromiseCoercion.ts, 2, 20))
}

async function i(): Promise<string> {
>i : Symbol(i, Decl(truthinessPromiseCoercion.ts, 36, 1))
>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, --, --))

if (pf()) { // error
>pf : Symbol(pf, Decl(truthinessPromiseCoercion.ts, 2, 42))

return "true";
}
if (pf()) { // error
>pf : Symbol(pf, Decl(truthinessPromiseCoercion.ts, 2, 42))

pf().then();
>pf().then : Symbol(Promise.then, Decl(lib.es5.d.ts, --, --))
>pf : Symbol(pf, Decl(truthinessPromiseCoercion.ts, 2, 42))
>then : Symbol(Promise.then, Decl(lib.es5.d.ts, --, --))
}
return "false";
}

28 changes: 28 additions & 0 deletions tests/baselines/reference/truthinessPromiseCoercion.types
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ declare const obj: { p: Promise<unknown> }
>obj : { p: Promise<unknown>; }
>p : Promise<unknown>

declare function pf(): Promise<boolean>
>pf : () => Promise<boolean>

async function f() {
>f : () => Promise<void>

Expand Down Expand Up @@ -132,3 +135,28 @@ async function h() {
>p : Promise<unknown>
}

async function i(): Promise<string> {
>i : () => Promise<string>

if (pf()) { // error
>pf() : Promise<boolean>
>pf : () => Promise<boolean>

return "true";
>"true" : "true"
}
if (pf()) { // error
>pf() : Promise<boolean>
>pf : () => Promise<boolean>

pf().then();
>pf().then() : Promise<boolean>
>pf().then : <TResult1 = boolean, TResult2 = never>(onfulfilled?: ((value: boolean) => TResult1 | PromiseLike<TResult1>) | null | undefined, onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | null | undefined) => Promise<TResult1 | TResult2>
>pf() : Promise<boolean>
>pf : () => Promise<boolean>
>then : <TResult1 = boolean, TResult2 = never>(onfulfilled?: ((value: boolean) => TResult1 | PromiseLike<TResult1>) | null | undefined, onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | null | undefined) => Promise<TResult1 | TResult2>
}
return "false";
>"false" : "false"
}

11 changes: 11 additions & 0 deletions tests/cases/compiler/truthinessPromiseCoercion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
declare const p: Promise<number>
declare const p2: null | Promise<number>
declare const obj: { p: Promise<unknown> }
declare function pf(): Promise<boolean>

async function f() {
if (p) {} // err
Expand Down Expand Up @@ -37,3 +38,13 @@ async function h() {
}
if (obj.p && await obj.p) {} // ok
}

async function i(): Promise<string> {
if (pf()) { // error
return "true";
}
if (pf()) { // error
pf().then();
}
return "false";
}

0 comments on commit 54b913c

Please sign in to comment.