Fix circularity errors in intra-binding-pattern references #59183
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
typescript-bot
added
Author: Team
For Backlog Bug
|
@typescript-bot test it |
|
Hey @ahejlsberg, the results of running the DT tests are ready. Everything looks the same! |
|
@ahejlsberg Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@ahejlsberg Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
|
@ahejlsberg Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
|
@typescript-bot test it |
|
Hey @ahejlsberg, the results of running the DT tests are ready. Everything looks the same! |
|
@ahejlsberg Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
|
@ahejlsberg Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@ahejlsberg Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
|
Tests are all clean and performance is unaffected. I think this is good to merge. |
Andarist
reviewed
Jul 9, 2024
| @@ -104,8 +104,8 @@ type Something = { foo: number }; | |||
|
|
|||
| // inference fails here, but ideally should not | |||
| const A = object<Something>()({ | |||
| >A : ObjectType<Something> | |||
| > : ^^^^^^^^^^^^^^^^^^^^^ | |||
| >A : any | |||
There was a problem hiding this comment.
The previous type was better here but i guess maybe that relied on broken assumptions from the PR that got reverted
There was a problem hiding this comment.
Right, when we report circularity errors we also assign the error type, but the reverted PR sometimes didn't do that.
|
@ahejlsberg Thanks for this fix, and sorry my PR caused inconsistencies. ❤️ |
RyanCavanaugh
approved these changes
Jul 15, 2024
|
@RyanCavanaugh The |
This was referenced Sep 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR reverts #56753 and implements a new fix for #49989.
Consider the example:
The array literal
[42]is contextually typed by the implied type of the binding pattern. To compute the implied type, we substituteanyfor for the first element (because it has no initializer) and the type ofa + 1for the second element. However, to compute the type ofa + 1we need the type ofa, which in turn means we need the type of[42][0], which means we need to know the type of the array literal[42], which is contextually typed by the implied type of the binding pattern... and we have a circularity.In #56753, logic was introduced that silently squelches circularity errors, but that isn't feasible because it is highly dependent on resolution order. In this PR we instead introduce logic that detects intra-binding-pattern references (such as the reference to
aina + 1) and types them asanywhen computing implied binding pattern types.Though strictly not necessary for the fix, the PR also revises the manner in which we "pad" types of initializer expressions for object destructuring patterns. This used to happen in
checkObjectLiteralbut now happens incheckDeclarationInitializer, aligning with the way we pad types of initializer expressions for array destructuring patterns. This change is responsible for the "Initializer provides no value for this binding element and the binding element has no default value" errors changing to messages like "Property 'a' does not exist on type '{}'" in the baselines.Fixes #49989.
Fixes #59177.