Uncalled function checks only works with single conditional #42835

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

src/compiler/checker.ts

tests/baselines/reference/truthinessCallExpressionCoercion2.errors.txt

sandersn · 2021-03-02T14:50:19Z

@typescript-bot user test this
@typescript-bot run dt

@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 · 2021-03-02T14:50:23Z

Heya @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 · 2021-03-02T14:50:23Z

Heya @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 · 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.

jonhue · 2021-03-05T14:03:13Z

@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. 🙂

src/compiler/checker.ts

sandersn · 2021-03-10T00:51:03Z

src/compiler/checker.ts

@@ -34550,6 +34550,9 @@ namespace ts {
            }

            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)

sandersn · 2021-03-10T00:52:43Z

tests/baselines/reference/uncalledFunctionChecksInConditional.errors.txt

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


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

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.

jonhue · 2021-03-12T09:13:40Z

@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.

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 user test this
@typescript-bot run dt

@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 · 2021-03-19T21:24:12Z

Heya @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 · 2021-03-19T21:24:12Z

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

jonhue · 2021-03-20T15:23:32Z

@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? 🙂

sandersn · 2021-04-02T17:20:52Z

@typescript-bot user test this
@typescript-bot run dt

typescript-bot · 2021-04-02T17:20:55Z

Heya @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 · 2021-04-02T17:20:55Z

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

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.

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 · 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.

clarkio · 2022-02-17T19:42:29Z

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

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

jonhue commented Feb 17, 2021

View reviewed changes

src/compiler/checker.ts Outdated Show resolved Hide resolved

jonhue mentioned this pull request Feb 17, 2021

Uncalled function checks only works with single conditional #35584

Closed

jonhue marked this pull request as draft February 17, 2021 11:13

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

jonhue commented Feb 17, 2021

View reviewed changes

tests/baselines/reference/truthinessCallExpressionCoercion2.errors.txt Outdated Show resolved Hide resolved

jonhue marked this pull request as ready for review February 17, 2021 11:34

sandersn self-assigned this Mar 2, 2021

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

sandersn requested changes Mar 10, 2021

View reviewed changes

jonhue requested a review from sandersn March 12, 2021 09:13

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

jonhue force-pushed the fix-35584 branch from 8ca5cdf to e2e6330 Compare March 12, 2021 10:15

fix test

de23735

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

sandersn approved these changes Apr 2, 2021

View reviewed changes

Merge branch 'main' into fix-35584

a5e7a6a

Cleanup after merge, accept baselines

177cea0

sandersn merged commit fb1066e into microsoft:main Feb 17, 2022

andrewbranch mentioned this pull request May 18, 2022

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

Merged

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uncalled function checks only works with single conditional #42835

Uncalled function checks only works with single conditional #42835

jonhue commented Feb 17, 2021

sandersn commented Mar 2, 2021

typescript-bot commented Mar 2, 2021 •

edited

Loading

typescript-bot commented Mar 2, 2021 •

edited

Loading

typescript-bot commented Mar 2, 2021

jonhue commented Mar 5, 2021

sandersn Mar 10, 2021

sandersn Mar 10, 2021

sandersn commented Mar 10, 2021

jonhue commented Mar 12, 2021

sandersn commented Mar 19, 2021 •

edited

Loading

typescript-bot commented Mar 19, 2021 •

edited

Loading

typescript-bot commented Mar 19, 2021 •

edited

Loading

jonhue commented Mar 20, 2021 •

edited

Loading

sandersn commented Apr 2, 2021

typescript-bot commented Apr 2, 2021 •

edited

Loading

typescript-bot commented Apr 2, 2021 •

edited

Loading

sandersn commented Apr 2, 2021

sandersn commented Apr 2, 2021

typescript-bot commented Feb 17, 2022

clarkio commented Feb 17, 2022

Uncalled function checks only works with single conditional #42835

Uncalled function checks only works with single conditional #42835

Conversation

jonhue commented Feb 17, 2021

sandersn commented Mar 2, 2021

typescript-bot commented Mar 2, 2021 • edited Loading

typescript-bot commented Mar 2, 2021 • edited Loading

typescript-bot commented Mar 2, 2021

jonhue commented Mar 5, 2021

sandersn Mar 10, 2021

Choose a reason for hiding this comment

sandersn Mar 10, 2021

Choose a reason for hiding this comment

sandersn commented Mar 10, 2021

jonhue commented Mar 12, 2021

sandersn commented Mar 19, 2021 • edited Loading

typescript-bot commented Mar 19, 2021 • edited Loading

typescript-bot commented Mar 19, 2021 • edited Loading

jonhue commented Mar 20, 2021 • edited Loading

sandersn commented Apr 2, 2021

typescript-bot commented Apr 2, 2021 • edited Loading

typescript-bot commented Apr 2, 2021 • edited Loading

sandersn commented Apr 2, 2021

sandersn commented Apr 2, 2021

typescript-bot commented Feb 17, 2022

clarkio commented Feb 17, 2022

typescript-bot commented Mar 2, 2021 •

edited

Loading

typescript-bot commented Mar 2, 2021 •

edited

Loading

sandersn commented Mar 19, 2021 •

edited

Loading

typescript-bot commented Mar 19, 2021 •

edited

Loading

typescript-bot commented Mar 19, 2021 •

edited

Loading

jonhue commented Mar 20, 2021 •

edited

Loading

typescript-bot commented Apr 2, 2021 •

edited

Loading

typescript-bot commented Apr 2, 2021 •

edited

Loading