Uncalled function checks only works with single conditional by jonhue · Pull Request #42835 · microsoft/TypeScript

Jonas Hübotter (jonhue) · 2021-02-17T10:51:29Z

This pull request makes two changes:

it checks all operands of || binary expressions for uncalled functions (not just the rightmost ones)
it checks operands of ! unary expressions for uncalled functions (if the function cannot be undefined)

Fixes #35584

Nathan Shively-Sanders (sandersn) · 2021-03-02T14:50:19Z

TypeScript Bot (@typescript-bot) user test this
TypeScript Bot (@typescript-bot) run dt

Jonas Hübotter (@jonhue) A couple of things to do next:

Look to see if there are new errors from the user tests or DT tests. Report on whether the new errors are useful or not. (The DT tests may have lots of false positives, since they're often not real code.)
Open a separate bug for "uncalled function checks don't work with negation".

TypeScript Bot (typescript-bot) · 2021-03-02T14:50:23Z

Heya Nathan Shively-Sanders (@sandersn), I've started to run the parallelized Definitely Typed test suite on this PR at 745a34c. You can monitor the build here.

TypeScript Bot (typescript-bot) · 2021-03-02T14:50:23Z

Heya Nathan Shively-Sanders (@sandersn), I've started to run the parallelized community code test suite on this PR at 745a34c. You can monitor the build here.

TypeScript Bot (typescript-bot) · 2021-03-02T15:44:04Z

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

Jonas Hübotter (jonhue) · 2021-03-05T14:03:13Z

Nathan Shively-Sanders (@sandersn) Thank you for your comment! I skimmed through the test suite run and the errors look reasonable to me.
I also split this pr up into two, as you suggested. This one, and #43097. 🙂

Nathan Shively-Sanders (sandersn) · 2021-03-10T00:51:03Z


            const location = isBinaryExpression(condExpr) ? condExpr.right : condExpr;
+            if (isBinaryExpression(condExpr) && condExpr.operatorToken.kind === SyntaxKind.BarBarToken) {
+                checkTestingKnownTruthyCallableOrAwaitableType(condExpr.left, type, body);


I don't think recursion is a good idea here, because binary expressions might be pretty long, and because the core of checkTestingKnownTruthyCallableOrAwaitableType is not too complex.

I'd move the getSignatureOfType check before location's declaration, then put the rest of the function in a helper that's called in a loop, something like:

helper(condExpr, body) while (isBinaryExpression(condExpr) && condExpr.operatorToken.kind === SyntaxKind.BarBarToken) { condExpr = condExpr.left helper(condExpr, body) }

(although you might be able to write it more elegantly)

Nathan Shively-Sanders (sandersn) · 2021-03-10T00:52:43Z

+        // error
+    }
+
+    if (isFoo || isBar) {


just for safety, can you add a test like x || y || z() || foo?

Nathan Shively-Sanders (sandersn) · 2021-03-10T01:06:11Z

Actually, looking at the whole call chain, I think type needs to be checked per-condExpr as well. At least there needs to be tests for multiple, alternating defined/undefined uncalled functions, like x || uy || z || ufoo or ux || y || uz || foo, where u- means that the variable has undefined in its type.

Jonas Hübotter (jonhue) · 2021-03-12T09:13:40Z

Nathan Shively-Sanders (@sandersn) Thank you for your comments! You're right, we do need to check the type per condExpr for this to work. I added some more test cases and it does work as we would expect now 🙂.
I also implemented the refactoring you suggested.

Nathan Shively-Sanders (sandersn) · 2021-03-19T21:24:08Z

I like the code as-is, but it checks a lot more than before. I want to see whether there are many new errors in the user tests:

TypeScript Bot (@typescript-bot) user test this
TypeScript Bot (@typescript-bot) run dt

Jonas Hübotter (@jonhue) can you take a look at those after they're done running?

Edit: I'll sign off if you find that there aren't a lot of bogus errors appearing.

TypeScript Bot (typescript-bot) · 2021-03-19T21:24:12Z

Heya Nathan Shively-Sanders (@sandersn), I've started to run the parallelized community code test suite on this PR at de23735. You can monitor the build here.

TypeScript Bot (typescript-bot) · 2021-03-19T21:24:12Z

Heya Nathan Shively-Sanders (@sandersn), I've started to run the parallelized Definitely Typed test suite on this PR at de23735. You can monitor the build here.

Jonas Hübotter (jonhue) · 2021-03-20T15:23:32Z

Nathan Shively-Sanders (@sandersn) looks like something went wrong while running the tests. I can't really gather from the logs what exactly went wrong. Could you take a brief look/rerun the tests? 🙂

Nathan Shively-Sanders (sandersn) · 2021-04-02T17:20:52Z

TypeScript Bot (@typescript-bot) user test this
TypeScript Bot (@typescript-bot) run dt

TypeScript Bot (typescript-bot) · 2021-04-02T17:20:55Z

Heya Nathan Shively-Sanders (@sandersn), I've started to run the parallelized Definitely Typed test suite on this PR at c973b9b. You can monitor the build here.

TypeScript Bot (typescript-bot) · 2021-04-02T17:20:55Z

Heya Nathan Shively-Sanders (@sandersn), I've started to run the parallelized community code test suite on this PR at c973b9b. You can monitor the build here.

Nathan Shively-Sanders (sandersn) · 2021-04-02T22:21:37Z

No report from typescript-bot again =( I manually checked the DT run and there weren't any new failures. It looks like the user test run failed to post a comment after it successfully pushed to the existing PR at https://github.com/typescript-bot/TypeScript/pull/158/files

I looked at the existing PR and saw two new errors: the fluentui one is good. It looks like the author forgot that hasSuggestionSelected was a method since it's used right before an accessor. The clone error is on module.exports -- TS generally doesn't understand module checks, so it's not a problem.

Nathan Shively-Sanders (sandersn) · 2021-04-02T22:22:40Z

Since this is a new feature, I'll merge this after the 4.3 beta is done.

TypeScript Bot (typescript-bot) · 2022-02-17T18:48:06Z

The TypeScript team hasn't accepted the linked issue #35584. If you can get it accepted, this PR will have a better chance of being reviewed.

Brian Clark (clarkio) · 2022-02-17T19:42:29Z

Thank you all for your work in this 👍 Glad to see it added

TypeScript Bot (typescript-bot) added the For Backlog Bug PRs that fix a backlog bug label Feb 17, 2021

Jonas Hübotter (jonhue) commented Feb 17, 2021

View reviewed changes

Comment thread src/compiler/checker.ts Outdated

Jonas Hübotter (jonhue) mentioned this pull request Feb 17, 2021

Uncalled function checks only works with single conditional #35584

Closed

Jonas Hübotter (jonhue) marked this pull request as draft February 17, 2021 11:13

Jonas Hübotter (jonhue) force-pushed the fix-35584 branch 3 times, most recently from 611f5b3 to e848beb Compare February 17, 2021 11:32

Jonas Hübotter (jonhue) commented Feb 17, 2021

View reviewed changes

Comment thread tests/baselines/reference/truthinessCallExpressionCoercion2.errors.txt Outdated

Jonas Hübotter (jonhue) marked this pull request as ready for review February 17, 2021 11:34

Nathan Shively-Sanders (sandersn) self-assigned this Mar 2, 2021

TypeScript Bot (typescript-bot) mentioned this pull request Mar 2, 2021

🤖 User test baselines have changed for fix-35584 typescript-bot/TypeScript#158

Open

This was referenced Mar 5, 2021

Uncalled function checks don't work with negation #43096

Open

Uncalled function checks don't work with negation #43097

Closed

Nathan Shively-Sanders (sandersn) requested changes Mar 10, 2021

View reviewed changes

Jonas Hübotter (jonhue) requested a review from Nathan Shively-Sanders (sandersn) March 12, 2021 09:13

Jonas Hübotter (jonhue) added 4 commits March 12, 2021 10:44

Uncalled function checks only works with single conditional

189b302

fix type errors in compiler

c57aec3

remove uncalled function checks with negations

b17401e

review

e2e6330

Jonas Hübotter (jonhue) force-pushed the fix-35584 branch from 8ca5cdf to e2e6330 Compare March 12, 2021 10:15

fix test

de23735

Jonas Hübotter (jonhue) mentioned this pull request Mar 12, 2021

Uncalled function checks don't work when the variable is referenced in the block #43211

Closed

Merge branch 'master' into fix-35584

c973b9b

Nathan Shively-Sanders (sandersn) approved these changes Apr 2, 2021

View reviewed changes

Merge branch 'main' into fix-35584

a5e7a6a

Cleanup after merge, accept baselines

177cea0

Nathan Shively-Sanders (sandersn) merged commit fb1066e into microsoft:main Feb 17, 2022

Andrew Branch (andrewbranch) mentioned this pull request May 18, 2022

Fix uncalled function check usage detection for && expressions #49157

Merged

Andrew Branch (andrewbranch) mentioned this pull request Jul 12, 2022

More fixes to uncalled function checks in && expressions #49868

Merged

This was referenced Jul 12, 2022

TypeScript 4.7.4: Exponential behavior in strict type checking #49845

Closed

Reuse computed type of condition expressions #49881

Merged

Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025

Uh oh!

Conversation

Jonas Hübotter (jonhue) commented Feb 17, 2021

Uh oh!

Uh oh!

Uh oh!

Nathan Shively-Sanders (sandersn) commented Mar 2, 2021

Uh oh!

TypeScript Bot (typescript-bot) commented Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TypeScript Bot (typescript-bot) commented Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TypeScript Bot (typescript-bot) commented Mar 2, 2021

Uh oh!

Jonas Hübotter (jonhue) commented Mar 5, 2021

Uh oh!

Uh oh!

Nathan Shively-Sanders (sandersn) Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

Nathan Shively-Sanders (sandersn) Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

Nathan Shively-Sanders (sandersn) commented Mar 10, 2021

Uh oh!

Jonas Hübotter (jonhue) commented Mar 12, 2021

Uh oh!

Nathan Shively-Sanders (sandersn) commented Mar 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TypeScript Bot (typescript-bot) commented Mar 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TypeScript Bot (typescript-bot) commented Mar 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jonas Hübotter (jonhue) commented Mar 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nathan Shively-Sanders (sandersn) commented Apr 2, 2021

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TypeScript Bot (typescript-bot) commented Apr 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nathan Shively-Sanders (sandersn) commented Apr 2, 2021

Uh oh!

Nathan Shively-Sanders (sandersn) commented Apr 2, 2021

Uh oh!

TypeScript Bot (typescript-bot) commented Feb 17, 2022

Uh oh!

Brian Clark (clarkio) commented Feb 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

TypeScript Bot (typescript-bot) commented Mar 2, 2021 •

edited

Loading

TypeScript Bot (typescript-bot) commented Mar 2, 2021 •

edited

Loading

Nathan Shively-Sanders (sandersn) commented Mar 19, 2021 •

edited

Loading

TypeScript Bot (typescript-bot) commented Mar 19, 2021 •

edited

Loading

TypeScript Bot (typescript-bot) commented Mar 19, 2021 •

edited

Loading

Jonas Hübotter (jonhue) commented Mar 20, 2021 •

edited

Loading

TypeScript Bot (typescript-bot) commented Apr 2, 2021 •

edited

Loading

TypeScript Bot (typescript-bot) commented Apr 2, 2021 •

edited

Loading