diff --git a/src/Type/TypeCombinator.php b/src/Type/TypeCombinator.php index 6f36f1ceec4..736c9aec7e6 100644 --- a/src/Type/TypeCombinator.php +++ b/src/Type/TypeCombinator.php @@ -1735,6 +1735,7 @@ 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]; @@ -1742,6 +1743,10 @@ public static function doIntersect(Type ...$types): Type $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()); @@ -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 @@ -1944,20 +1984,6 @@ public static function doIntersect(Type ...$types): Type continue 2; } - if ($types[$i] instanceof ObjectShapeType && $types[$j] instanceof HasPropertyType) { - $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) { diff --git a/tests/PHPStan/Analyser/nsrt/bug-15047.php b/tests/PHPStan/Analyser/nsrt/bug-15047.php new file mode 100644 index 00000000000..5a9dead790a --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15047.php @@ -0,0 +1,39 @@ +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); + 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); + } + +} diff --git a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php index f4cc96575bf..5b7ccd3be4e 100644 --- a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php +++ b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php @@ -25,6 +25,8 @@ class InstantiationRuleTest extends RuleTestCase private bool $checkExplicitMixed = false; + private bool $checkImplicitMixed = false; + protected function getRule(): Rule { $reflectionProvider = self::createReflectionProvider(); @@ -35,7 +37,7 @@ protected function getRule(): Rule checkThisOnly: false, checkUnionTypes: true, checkExplicitMixed: $this->checkExplicitMixed, - checkImplicitMixed: false, + checkImplicitMixed: $this->checkImplicitMixed, checkBenevolentUnionTypes: false, discoveringSymbolsTip: true, ); @@ -731,4 +733,10 @@ public function testInstantiationWithNonObjectType(): void ]); } + public function testBug15047(): void + { + $this->checkImplicitMixed = true; + $this->analyse([__DIR__ . '/data/bug-15047.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Classes/data/bug-15047.php b/tests/PHPStan/Rules/Classes/data/bug-15047.php new file mode 100644 index 00000000000..6e1a259a73b --- /dev/null +++ b/tests/PHPStan/Rules/Classes/data/bug-15047.php @@ -0,0 +1,32 @@ +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; + } + +} diff --git a/tests/PHPStan/Type/TypeCombinatorTest.php b/tests/PHPStan/Type/TypeCombinatorTest.php index b4aec71755c..75262a96e6f 100644 --- a/tests/PHPStan/Type/TypeCombinatorTest.php +++ b/tests/PHPStan/Type/TypeCombinatorTest.php @@ -4965,6 +4965,26 @@ public static function dataIntersect(): iterable ObjectShapeType::class, 'object{foo: int}', ]; + // A third member the generic supertype dedup can weigh the HasPropertyType against must + // not change whether the optional key is resolved - the member order used to decide it. + yield [ + [ + new ObjectType('stdClass'), + new ObjectShapeType(['foo' => new IntegerType()], ['foo']), + new HasPropertyType('foo'), + ], + IntersectionType::class, + 'object{foo: int}&stdClass', + ]; + yield [ + [ + new ObjectShapeType(['foo' => new IntegerType()], ['foo']), + new ObjectType('Traversable'), + new HasPropertyType('foo'), + ], + IntersectionType::class, + 'object{foo: int}&Traversable', + ]; yield [ [ new ObjectShapeType(['foo' => new IntegerType()], []),