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

Uncalled Function Checks don't check negative condition #34815

Open
DanielRosenwasser opened this issue Oct 29, 2019 · 4 comments
Open

Uncalled Function Checks don't check negative condition #34815

DanielRosenwasser opened this issue Oct 29, 2019 · 4 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@DanielRosenwasser
Copy link
Member

function foo() {
}

if (!foo) {
    let x = 10
}
else {
}

Expected: Error on !foo that the condition is always true because foo is defined.
Actual: No error.

Also, any work on this should include the following test:

function foo() {

}

if (!foo) {
}
else {
    foo()
}
@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Oct 30, 2019
@brandon-leapyear
Copy link

brandon-leapyear commented Feb 17, 2021

This is an old work account. Please reference @brandonchinn178 for all future communication


Very much agree with this. In our code, we just got bit by

if (!nock.isActive) {
  nock.activate()
}

In general, I would usually not intentionally want !foo, and if I did want to do something like that, I would do foo === undefined. Even more generally, I would personally disallow coercion from function to boolean completely. Forcing devs to write foo === undefined would make it clear what their intention is, and would force the dev to type foo as Function | undefined, if that's truly what they want.

@AlexMarkUSDS
Copy link

This bit me last week in one of the codebases I maintain. I also see that it's bitten a number of others:
There's relevant info in:

@DanielRosenwasser, would the TS team accept a contribution for this? I've optimistically opened a PR at #57114

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jan 23, 2024

There are many, many false positives turned up in that PR. Unless these were ~all good finds, I think this is appropriately left still in "awaiting more feedback" state

@AlexMarkUSDS
Copy link

The tool for finding false positives is super neat. It makes sense to me to not merge this -- I followed ~20 links and I think this was the only legitimate bug I found (it should have an await). The rest were all just undefined checks on things that the typings say should always be defined.

I'm happy to close out that PR (or have you close it). Whatever works for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants