Fix phpstan/phpstan#14301: Unioned array shapes are no longer disjunctive#5229
Conversation
ondrejmirtes
left a comment
ondrejmirtes
left a comment
There was a problem hiding this comment.
Also try to fix phpstan/phpstan#11488 at the same time. Add a regression test and make sure your fix fixes everything at once.
src/Analyser/TypeSpecifier.php
Outdated
| $specifiedTypes = $this->specifyTypesForCountFuncCall($unwrappedLeftExpr, $argType, $rightType, $context, $scope, $expr); | ||
| if ($specifiedTypes !== null) { | ||
| if ($argType->isConstantArray()->yes()) { | ||
| return $specifiedTypes->setAlwaysOverwriteTypes(); |
There was a problem hiding this comment.
setAlwaysOverwriteTypes is a hack that should not be used, and often makes the code worse and more buggy.
By default, SpecifiedTypes are intersected (or removed) from the type in Scope. setAlwaysOverwriteTypes makes it so that the type in SpecifiedTypes takes over the original Type in Scope, possibly losing information.
There was a problem hiding this comment.
I processed this review but have nothing to report.
ondrejmirtes
left a comment
ondrejmirtes
left a comment
There was a problem hiding this comment.
Try the review again, I made your timeout longer.
Also rebase on top of latest 2.1.x in phpstan/phpstan-src and fix conflicts.
ondrejmirtes
left a comment
ondrejmirtes
left a comment
There was a problem hiding this comment.
Try the review again, I made your timeout longer.
Also rebase on top of latest 2.1.x in phpstan/phpstan-src and fix conflicts.
ondrejmirtes
force-pushed
the
create-pull-request/patch-ox57lpy
branch
2 times, most recently
from
6c5f267 to
c858ed2
Compare
ondrejmirtes
force-pushed
the
create-pull-request/patch-ox57lpy
branch
from
c858ed2 to
7e6348d
Compare
|
I processed this review but have nothing to report. |
2 participants
This PR fixes phpstan/phpstan#14301 where count()-based type narrowing failed on unions of constant array shapes.
Problem
For a type like
array{bool}|array{mixed, string|null, mixed}, checkingcount($row) !== 1should narrow toarray{mixed, string|null, mixed}in the truthy branch andarray{bool}in the else branch. Instead, no narrowing occurred — the type remainedarray{bool}|array{mixed, string|null, mixed}in both branches.Root cause:
specifyTypesForCountFuncCall()correctly computed the narrowed type, but it was applied viaaddTypeToExpression()which intersects with the original type.TypeCombinator::intersect()couldn't filter union members becauseConstantArrayTypeuses open-shape semantics — intersecting two constant arrays with different key counts doesn't producenever.Similarly for issue #11488:
array{mixed}|array{mixed, string|null, mixed}withcount($row) !== 1failed becausearray{mixed}is a supertype ofarray{mixed, string|null, mixed}in open-shape semantics, so both intersection and removal approaches fail.Fix
For constant array list unions without optional keys, use
HasOffsetValueTypeconstraints instead of the computed result type:HasOffsetValueType(sizeMin - 1)— intersecting with this keeps only arrays that have at leastsizeMinkeys (e.g.,HasOffset(2)filters out 1-key arrays when narrowing for count === 3)HasOffsetValueType(sizeMax)— removing this filters out arrays with more thansizeMaxkeys (e.g., removingHasOffset(1)filters out 3-key arrays when narrowing for count === 1)This works because
HasOffsetValueTypecorrectly identifies whether a sealed constant array has a given offset (yes/no), unlike the open-shape intersection which considers extra keys compatible.For arrays with optional keys, the existing behavior is preserved (falls through to the original
create()path).Changes
src/Analyser/TypeSpecifier.phptests/PHPStan/Analyser/nsrt/bug-14301.php