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 work when the variable is referenced in the block #43211

Closed
jonhue opened this issue Mar 12, 2021 · 3 comments
Closed
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@jonhue
Copy link
Contributor

jonhue commented Mar 12, 2021

Bug Report

πŸ”Ž Search Terms

uncalled function checks, conjunction, &&

πŸ•— Version & Regression Information

This is the behavior in every version I tried

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

// @strictNullChecks: true

declare function isFoo(): boolean;
declare function isBar(): boolean;

if (isFoo) {
    // uncalled function checks already work well here
}

if (isFoo && isBar()) {
    // uncalled function checks already work well here
}

if (isFoo && isFoo()) {
    // uncalled function checks should cover this case too
}

if (isFoo && isBar()) {
    isFoo;
    // uncalled function checks should cover this case too
}

πŸ™ Actual behavior

In the last two cases we didn't detect that the first occurrence of isFoo in the condition is uncalled.

πŸ™‚ Expected behavior

When isFoo cannot possibly be a falsy value (i.e. undefined) we should highlight this as an error even if isFoo occurs again later on. This is a follow-up to a conversation in #42835 (comment).
I would be happy to enable this check, then we could run this against some user tests and decide whether there are too many false positives πŸ™‚

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Mar 12, 2021
@RyanCavanaugh
Copy link
Member

A reference should be sufficient here to not complain, since you might write something like

if (isFoo) {
  invokeThisPlease(isFoo);
}

@jonhue
Copy link
Contributor Author

jonhue commented Mar 12, 2021

@RyanCavanaugh I'm probably misunderstanding your example, but if isFoo cannot possible be undefined, isn't the condition in the example you gave above redundant?

@RyanCavanaugh
Copy link
Member

It's intentional that the rule is not ironclad. This is because there are some places in the language where TS thinks something is always not-undefined, but it's not necessarily the case.

@jonhue jonhue closed this as completed Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

2 participants