Skip to content

Type predicate silently ignored when used as non-statement expression #61105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rotu opened this issue Feb 3, 2025 · 10 comments
Closed

Type predicate silently ignored when used as non-statement expression #61105

rotu opened this issue Feb 3, 2025 · 10 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@rotu
Copy link

rotu commented Feb 3, 2025

🔎 Search Terms

asserts, return, void

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about type predicates

⏯ Playground Link

https://www.typescriptlang.org/play/?target=99&jsx=0&suppressImplicitAnyIndexErrors=false&ts=5.8.0-dev.20250202&ssl=19&ssc=1&pln=1&pc=1&q=5#code/GYVwdgxgLglg9mABAWwJ4EEDOmCmAnKACgEMAuRAIzjgBsdiwBKc47fKTRYxGTqPEDkQBvAFCIewRIQCExRoigALPHADuiMDg0BRPKryEARGDhRFAnDKOMA3KIC+o0aEiwEiHAA8AjCXJUtPRMIuIoGGwEJHZheDhQIHhIxPYSAPRpElkAegD8YRlZRdIADsR4xMjx+ApkFoKOzq7Q8EjeAEz+lNR0DApiEnTmXogAvOFYuFHyqYiFiNQlmDKIAJJc2DAA5loAJopKQnGYIDTmUHBciABu5TDEFHRcYPvKQlCoJUKsU+5IvJozIgIIcIABrHC7GSxeKJZKzeY5fLpTLFCSEMoVKpQGpcAI9YKNIA

💻 Code

function myAssert(a: boolean): asserts a is true {
  if (!a) throw new Error("not true!");
}

function ex1(a: boolean) {
  myAssert(a);
  return a;
  //     ^?
  //        (parameter) a: true
}

function ex2(a: boolean) {
  let x = myAssert(a);
  //  oops! I assigned the result to a variable and the type assertion is not checked!
  return a;
  //     ^?
  //        (parameter) a: boolean
}

🙁 Actual behavior

a is not narrowed to true.

This is odd because:

  1. The assertion is unconditionally called.
  2. There is no warning to indicate that the type assertion has been ignored as in other cases where CFA can't work.

🙂 Expected behavior

I expect a to be narrowed to true.

Additional information about the issue

It seems not all expressions cause the type assertion to be ignored.

With the comma operator, for instance, the type assertion is still respected:

console.log(), myAssert(a), console.log();
// a is regarded as "true" here

With the void operator, the type assertion is ignored.
Even when occurring as the parameter for an await, yield, or return statement, the type assertion is discarded.

@rotu
Copy link
Author

rotu commented Feb 3, 2025

Another weird gotcha is that an IIFE causes the result of the assertion to be discarded if you use a parenthesized arrow function but NOT a braced arrow function.

That is:

function myAssert(a: boolean): asserts a{}

declare const x:boolean
(()=>{myAssert(x)})(); // will narrow the type of x
x
// ^?
//    const x: true
declare const y:boolean;
(()=>(myAssert(y)))(); // won't narrow the type of x
y
// ^?
//    const y: boolean

@MartinJohns
Copy link
Contributor

This is working as intended. See #32695:

A function call is analyzed as an assertion call or never-returning call when

  • the call occurs as a top-level expression statement, and
  • the call specifies a single identifier or a dotted sequence of identifiers for the function name, and
  • each identifier in the function name references an entity with an explicit type, and
  • the function name resolves to a function type with an asserts return type or an explicit never return type annotation.

@rotu
Copy link
Author

rotu commented Feb 3, 2025

This is working as intended. See #32695:

I actually just ran across your comment in #60841 :-)

the call occurs as a top-level expression statement

That seems to indicate that this should NOT narrow but it seems to:

console.log(), myAssert(a), console.log();

@rotu
Copy link
Author

rotu commented Feb 3, 2025

It seems that that #33622 tried to clean up some of the funky corner cases. E.g.:

// fails to narrow, loudly (good!)
// "Assertions require the call target to be an identifier or qualified name.(2776)"
(myAssert || null)(a);

But I'm not sure if that PR meant to flag assertion types that are ignored because they are not expression statements (and if not, why not).

@whzx5byb
Copy link

whzx5byb commented Feb 3, 2025

That seems to indicate that this should NOT narrow but it seems to:
console.log(), myAssert(a), console.log();

This was not narrowed in 3.7 (where #32695 was first introduced) but worked in 4.6, so I guess it got improved then while I can't find the specific PR.

@Andarist
Copy link
Contributor

Andarist commented Feb 3, 2025

Comma expressions support was introduced first in #41312 . Then it was extended in #47049

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Feb 4, 2025
@rotu
Copy link
Author

rotu commented Feb 5, 2025

@RyanCavanaugh What's the design limitation? Is it impossible to flag when an asserts return type occurs in a non-statement expression?

@MartinJohns
Copy link
Contributor

It was mentioned in other issues that this is an intentional limitation done for performance reasons.

@rotu
Copy link
Author

rotu commented Feb 5, 2025

It was mentioned in other issues that this is an intentional limitation done for performance reasons.

Yes, that’s why expressions aren’t recursively explored during control-flow-analysis. not why they are dropped quietly during type analysis.

If a compound expression contains a subexpression of asserts type, we know that it is unenforced and can flag it as a mistake (rather than quietly decaying to type void). This doesn’t have the performance problems mentioned.

@rotu rotu changed the title Type predicate silently ignored when used as expression Type predicate silently ignored when used as non-statement expression Feb 5, 2025
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Design Limitation" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

6 participants