Skip to content
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

!constant in boolean expression not reported (function not called error) #45667

Open
concavelenz opened this issue Aug 31, 2021 · 5 comments
Open
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@concavelenz
Copy link

Bug Report

It is a common source of bugs that developers fail to call functions in boolean expressions. This, among other reasons, the compiler checks for constant boolean expressions. However, there seems to be a hole in the existing checks. TSC currently allows "!fn" where "fn" and "(!fn) == true" are not allowed. It would be beneficial to check this kind of expression as well.

function fn() { return 'abc'; }

if (fn) {}  // ERROR: This condition will always ...
if (!fn) {} // not reported
if (fn == true) {} // ERROR: This condition will always ...
if ((!fn) == true) {} // ERROR: This condition will always ...

🔎 Search Terms

function "condition always true"

function constant boolean expression

🕗 Version & Regression Information

unknown, presumably since the beginning of time, at least TS 3.3, related improvement in TS 3.7

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

⏯ Playground Link

Playground link with relevant code

https://www.typescriptlang.org/play?ts=3.7.5&q=247#code/GYVwdgxgLglg9mABMMAKAlIg3ogTgUyhFyQHIBDAIwlIG5EBfAWAChWZhFUVMtmWOXAIQ9s-QdyQBeKYii4Q+XuM6pUIsJhlyFSsa1ZA

💻 Code

function fn() { return 'abc'; }

if (fn) {}  // ERROR: This condition will always ...
if (!fn) {} // not reported
if (fn == true) {} // ERROR: This condition will always ...
if ((!fn) == true) {} // ERROR: This condition will always ...

🙁 Actual behavior

No warning

🙂 Expected behavior

"This condition will always ..." warning

@andrewbranch
Copy link
Member

Duplicate of #44840

@andrewbranch andrewbranch marked this as a duplicate of #44840 Aug 31, 2021
@andrewbranch andrewbranch added the Duplicate An existing issue was already created label Aug 31, 2021
@concavelenz
Copy link
Author

I'm not convince this is a duplicate of #44840 as it doesn't reference the existing checks. Would it be possible to explain why (!fn) == true (where fn is a function) is disallowed today but !fn is allowed? I would expect them to have the same constraints.

@andrewbranch
Copy link
Member

The fact that (!fn) == true is caught is actually a surprise to me... I thought we threw truthiness knowledge out with !. I’ll double check.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Sep 1, 2021

So this is definitely confusing, especially because the error messages are similar; but my understanding is

  • if (fn) is getting an error for always-defined functions. That's expected.
  • if (!fn == true) is getting an error for something called comparability which checks for overlap between two types. The check is expected , but I think the result of !fn isn't.

I think we're trying to be "too smart" in the second example though. I would expect !fn would just become boolean (not false), and would pass the check.

@concavelenz
Copy link
Author

Detecting fn when the author intended fn() is valuable. TSC already catches "if (fn) ..." is great, I'd hope that catching "if (!fn) " would also be seen as worthwhile as !x is a common use of a result value in a boolean expression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants