Skip to content

Improve type inference for destructuring of shaped array assignments#4873

Draft
dasl- wants to merge 7 commits intophan:v5from
dasl-:fixuparraydestruc
Draft

Improve type inference for destructuring of shaped array assignments#4873
dasl- wants to merge 7 commits intophan:v5from
dasl-:fixuparraydestruc

Conversation

@dasl-
Copy link
Copy Markdown
Member

@dasl- dasl- commented Aug 7, 2024

Previously, when analyzing an assignment like [$a, $b] = expr, if expr had at least one shaped array in its UnionType, we would mostly ignore whatever generic arrays it had in its UnionType (e.g. Foo[]) when inferring the type of $a and $b. That is, if expr had a UnionType like Foo[]|array{0:null,1:null}, we would only infer that $a and $b can be null.

The goal of this changeset is to instead infer that $a and $b are Foo|null.

dasl- added 7 commits August 7, 2024 17:22
Previously, when analyzing an assignment like `[$a, $b] = expr`, if `expr` had at least one shaped array in its UnionType, we would mostly ignore whatever generic arrays it had in its UnionType (e.g. `Foo[]`) when inferring the type of `$a` and `$b`. That is, if `expr` had a UnionType like `Foo[]|array{0:null,1:null}`, we would only infer that `$a` and `$b` can be `null`.

The goal of this changeset is to instead infer that `$a` and `$b` are `Foo|null`.
@rlerdorf
Copy link
Copy Markdown
Member

rlerdorf commented Oct 5, 2025

I played around with this a bit on the v6 branch. The approach works for the specific destructuring issue but has unintended consequences for type precision across the codebase.
It changes fundamental type resolution semantics and reduces type precision for array shapes by pre-combining generic and shaped array types.
I see two paths forward on this one. Modify only the destructuring logic in the AssignmentVisitor to merge generic types when array shapes are present. Or Use the new approach only for destructuring and keep the old behavior for element access.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants