Skip to content

Collapse ObjectShapeType against HasPropertyType regardless of member order #6184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SanderMuller wants to merge 3 commits into
base: 2.2.x
Choose a base branch
from
Open

Collapse ObjectShapeType against HasPropertyType regardless of member order #6184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e2cdf6d
Collapse ObjectShapeType against HasPropertyType regardless of member…
SanderMuller Aug 5, 2026
2674143
Add a rule test for the argument.type false positive
Aug 10, 2026
189ee43
Use the simpler reproducer without the abstract parent class
SanderMuller Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 40 additions & 14 deletions src/Type/TypeCombinator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1735,13 +1735,18 @@ public static function doIntersect(Type ...$types): Type
$hasOffsetValueTypeCount = 0;
$typesCount = count($types);
$typesNeedSorting = false;
$hasPropertyType = false;
for ($i = 0; $i < $typesCount; $i++) {
$type = $types[$i];

if ($type instanceof SubtractableType || $type instanceof ConstantArrayType) {
$typesNeedSorting = true;
}

if ($type instanceof HasPropertyType) {
$hasPropertyType = true;
}

if ($type instanceof IntersectionType && !$type instanceof TemplateType) {
// transform A & (B & C) to A & B & C
array_splice($types, $i--, 1, $type->getTypes());
Expand Down Expand Up @@ -1780,6 +1785,41 @@ public static function doIntersect(Type ...$types): Type
});
}

// Resolve object-shape optional keys that a HasPropertyType asserts are present before the
// reduction loop below. In that loop the generic supertype dedup can drop a HasPropertyType
// as redundant against a dynamic-property class such as stdClass (which reports every
// property as present) before it is ever paired with the object shape. Which of the two
// fires first depends on the member order, so the collapse runs here, where order does not
// change the result of what is meant to be an order-independent value. Gated on the presence
// of a HasPropertyType so the common intersection pays only the flag check set above.
if ($hasPropertyType) {
for ($i = 0; $i < $typesCount; $i++) {
for ($j = $i + 1; $j < $typesCount; $j++) {
if (
$types[$i] instanceof ObjectShapeType
&& $types[$j] instanceof HasPropertyType
&& !$types[$i]->hasInstanceProperty($types[$j]->getPropertyName())->no()
) {
$types[$i] = $types[$i]->makePropertyRequired($types[$j]->getPropertyName());
array_splice($types, $j--, 1);
$typesCount--;
continue;
}

if (
$types[$j] instanceof ObjectShapeType
&& $types[$i] instanceof HasPropertyType
&& !$types[$j]->hasInstanceProperty($types[$i]->getPropertyName())->no()
) {
$types[$j] = $types[$j]->makePropertyRequired($types[$i]->getPropertyName());
array_splice($types, $i--, 1);
$typesCount--;
continue 2;
}
}
}
}

// transform IntegerType & ConstantIntegerType to ConstantIntegerType
// transform Child & Parent to Child
// transform Object & ~null to Object
Expand Down Expand Up @@ -1944,20 +1984,6 @@ public static function doIntersect(Type ...$types): Type
continue 2;
}

if ($types[$i] instanceof ObjectShapeType && $types[$j] instanceof HasPropertyType) {

@staabm staabm Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we not have the same problem for if ($types[$i] instanceof ConstantArrayType && $types[$j] instanceof HasOffsetValueType) {?

@SanderMuller SanderMuller Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked it rather than reasoned about it: the array pair does not have the problem, in any combination I could build.

I brute-forced order independence — for each member set, run TypeCombinator::intersect() over every permutation and compare the results. On 2.2.x:

ORDER-DEP  OBJ shape + hasProperty + stdClass       => object{a: int}&stdClass  vs  object{a?: int}&stdClass
ORDER-DEP  OBJ shape + hasProperty + Traversable    => object{a: int}&Traversable  vs  Traversable&hasProperty(a)
OK         ARR constArray + hasOffsetValue
OK         ARR constArray + hasOffsetValue + array
OK         ARR constArray + hasOffsetValue + oversized
OK         ARR constArray + hasOffsetValue + nonEmpty
OK         ARR constArray + hasOffset      + nonEmpty
OK         ARR constList  + list           + nonEmpty
OK         ARR constList  + hasOffsetValue + nonEmpty + list
ARR constArray + hasOffsetValue + oversized + nonEmpty  => OK

With this PR all of them are order-independent.

The reason is not the position of the pairing — both pairs sit in the same $isSuperTypeB->maybe() block. It is that the object side has a third member that makes the accessory look redundant to the generic dedup above it, and the array side has no counterpart:

HasPropertyType(a)->isSuperTypeOf(stdClass)                  = Yes     <- drops the accessory
HasPropertyType(a)->isSuperTypeOf(Traversable)               = Maybe
HasOffsetValueType(a,int)->isSuperTypeOf(array)              = Maybe
HasOffsetValueType(a,int)->isSuperTypeOf(oversized)          = Maybe
HasOffsetValueType(a,int)->isSuperTypeOf(nonEmptyArray)      = Maybe
HasOffsetValueType(a,int)->isSuperTypeOf(constArray{a?:int}) = Maybe

stdClass reports every property as present, so hasProperty(a) is a definite supertype of it and $isSuperTypeA->yes() splices it out before it can be paired with the object shape. "Has offset a with value int" is not implied by any general array type, so nothing makes HasOffsetValueType definitely redundant, and the only type that would — a constant array that already has the offset — has already absorbed the information anyway.

One thing that did fall out of the sweep: the same bug fires with an ordinary interface, not just stdClass. object{a?: int} & hasProperty(a) & Traversable resolves three ways depending on order on 2.2.x, and in some of them the object shape is dropped entirely (Traversable&hasProperty(a)). This PR fixes that too. Say the word if you want that case added as a second fixture.

@staabm staabm Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please add a test

$types[$i] = $types[$i]->makePropertyRequired($types[$j]->getPropertyName());
array_splice($types, $j--, 1);
$typesCount--;
continue;
}

if ($types[$j] instanceof ObjectShapeType && $types[$i] instanceof HasPropertyType) {
$types[$j] = $types[$j]->makePropertyRequired($types[$i]->getPropertyName());
array_splice($types, $i--, 1);
$typesCount--;
continue 2;
}

$constArrayIsI = $types[$i] instanceof ConstantArrayType && ($types[$j] instanceof ArrayType || $types[$j] instanceof ConstantArrayType);
$constArrayIsJ = $types[$j] instanceof ConstantArrayType && ($types[$i] instanceof ArrayType || $types[$i] instanceof ConstantArrayType);
if ($constArrayIsI || $constArrayIsJ) {
Expand Down
39 changes: 39 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-15047.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php declare(strict_types = 1);

namespace Bug15047;

use stdClass;
use function PHPStan\Testing\assertType;

final class MissalYearLimits
{

private ?int $until_year;

private function __construct(?int $until_year)
{
$this->until_year = $until_year;
}

/**
* @param stdClass&object{since_year:int,until_year?:int} $data
*/
protected static function fromObjectInternal(stdClass $data): self
{
assertType('object{since_year: int, until_year?: int}&stdClass', $data);
Comment thread
staabm marked this conversation as resolved.
assertType('int', $data->since_year);
assertType('int|null', isset($data->until_year) ? $data->until_year : null);
assertType('int|null', $data->until_year ?? null);

// isset() adds a hasProperty() member to the intersection, and resolving the
// optional key against it must not depend on where that member lands in the
// member list - which is what this used to be sensitive to.
if (isset($data->until_year)) {
assertType('object{since_year: int, until_year: int}&stdClass', $data);
assertType('int', $data->until_year);
}

return new self(isset($data->until_year) ? $data->until_year : null);
}

}
10 changes: 9 additions & 1 deletion tests/PHPStan/Rules/Classes/InstantiationRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class InstantiationRuleTest extends RuleTestCase

private bool $checkExplicitMixed = false;

private bool $checkImplicitMixed = false;

protected function getRule(): Rule
{
$reflectionProvider = self::createReflectionProvider();
Expand All @@ -35,7 +37,7 @@ protected function getRule(): Rule
checkThisOnly: false,
checkUnionTypes: true,
checkExplicitMixed: $this->checkExplicitMixed,
checkImplicitMixed: false,
checkImplicitMixed: $this->checkImplicitMixed,
checkBenevolentUnionTypes: false,
discoveringSymbolsTip: true,
);
Expand Down Expand Up @@ -731,4 +733,10 @@ public function testInstantiationWithNonObjectType(): void
]);
}

public function testBug15047(): void
{
$this->checkImplicitMixed = true;
$this->analyse([__DIR__ . '/data/bug-15047.php'], []);
}

}
32 changes: 32 additions & 0 deletions tests/PHPStan/Rules/Classes/data/bug-15047.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php declare(strict_types = 1);

namespace Bug15047Instantiation;

use stdClass;

final class MissalYearLimits
{

private ?int $until_year;

private function __construct(?int $until_year)
{
$this->until_year = $until_year;
}

/**
* @param stdClass&object{since_year:int,until_year?:int} $data
*/
protected static function fromObjectInternal(stdClass $data): self
{
// the optional key must keep its declared type through the isset() narrowing,
// otherwise this reports "expects int|null, mixed given"
return new self(isset($data->until_year) ? $data->until_year : null);
}

public function getUntilYear(): ?int
{
return $this->until_year;
}

}
Loading