Collapse ObjectShapeType against HasPropertyType regardless of member order - #6184
Collapse ObjectShapeType against HasPropertyType regardless of member order#6184SanderMuller wants to merge 2 commits into
Conversation
| */ | ||
| protected static function fromObjectInternal(stdClass $data): self | ||
| { | ||
| assertType('object{since_year: int, until_year?: int}&stdClass', $data); |
There was a problem hiding this comment.
doesn't object{...} already imply stdClass? can't we just drop stdClass from the intersection when building the type for the phpdoc as it is already redundant?
skip that. I missremembered
… order
An intersection of an object shape carrying an optional key with a
HasPropertyType for that key - as produced by isset()/?? narrowing, e.g.
`stdClass&object{u?:int}` narrowed by `isset($x->u)` - is supposed to resolve
the optional key to its declared type, so that `$x->u ?? null` is `int|null`.
That relied on member order. The collapse
`ObjectShapeType & HasPropertyType -> makePropertyRequired()` sat in the
reduction loop, reached only after the generic supertype dedup. When the
intersection also contains a dynamic-property class such as stdClass, which
reports every property as present, HasPropertyType is a supertype of it and the
dedup splices HasPropertyType out before it is ever paired with the object
shape. Which of the two fires first depends on the order of the members, so the
optional key stayed optional whenever stdClass happened to come first and the
read fell back to the class's mixed. While intersection members were still
sorted in place this was masked - describing the type reordered them so the
shape came first; once that mutation was removed the construction order won.
Move the collapse into its own pass before the reduction loop so member order
no longer decides the result. Guard it with hasInstanceProperty(): when the
shape does not have the key it is left untouched, so a sealed shape intersected
with a HasPropertyType for a key it cannot have still reduces to never in the
loop below, as before.
The array analogue (ConstantArrayType & HasOffsetType) is unaffected: there is
no universal-offset crate reporting every offset as present, so nothing absorbs
the HasOffsetType before the offset is made required.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4372b52 to
fb4d864
Compare
The nsrt test covers the inferred type; this asserts the reported error is gone at the level that emitted it. The false positive only appears with checkImplicitMixed (level 10), so InstantiationRuleTest gains the same toggle the other rule tests already expose.
|
Good call, added in 29b5e27.
It comes from One wrinkle worth mentioning. The error only appears with Gates: full suite green (21304 tests), self-analysis clean, phpcs clean, |
When a parameter is typed
\stdClass&object{u?:int}and its value goes throughisset()/??narrowing, PHPStan should resolve the optional key to its declared type. Since 2.2.6 it fell back tomixedinstead:Root cause
isset($data->until_year)narrows$databy intersecting it with aHasPropertyTypefor the key, andTypeCombinator::intersect()is supposed to collapseobject{until_year?:int} & hasProperty(until_year)intoobject{until_year:int}, so the read isint.That collapse lived in the pairwise reduction loop, reached only after the generic supertype dedup. The parameter also carries
stdClass, a dynamic-property class that reports every property as present, sohasProperty(until_year)is a supertype of it and the dedup removes theHasPropertyTypeas redundant againststdClass. Whether the dedup drops it first or the object shape collapses it first depends on the order of the members in the intersection.While intersection members were still sorted in place, describing the type happened to reorder them so the object shape came first and won the race. Once that in-place mutation was removed (correctly, so a type's value no longer depends on what has been called on it) the construction order took over,
stdClasscame first, and itsmixeddominated.Note the bare read
$data->until_yearwithoutisset/??was alreadymixedbefore and after 2.2.6. That is a separate, pre-existing gap and not what this fixes; this change is about theisset/??narrowing path only.Fix
Move the object-shape /
HasPropertyTypecollapse into its own pass before the reduction loop, so member order no longer decides the outcome. Two details keep it behaviour-preserving otherwise:hasInstanceProperty(...)is notno). A sealed shape intersected with aHasPropertyTypefor a key it cannot have is left alone, so it still reduces toneverin the loop below, as it did before.HasPropertyTypebeing present in the intersection, a flag computed in the existing flatten loop, so an intersection without one pays only that flag check and never enters the extra loop.The array analogue (
ConstantArrayType & HasOffsetType) is not affected: there is no universal-offset type that reports every offset as present, so nothing absorbs theHasOffsetTypebefore the offset is made required. Verified against real 2.2.5/2.2.6 builds.Closes phpstan/phpstan#15047