From d0bd13d20308a9df4dd79c9c06d7a55a032b2bd5 Mon Sep 17 00:00:00 2001 From: Daniel Badura Date: Mon, 28 Nov 2022 11:02:54 +0100 Subject: [PATCH 1/3] Add testcase for contravariant interface which is failing --- ...ParameterTypeContravarianceChangedTest.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterTypeContravarianceChangedTest.php b/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterTypeContravarianceChangedTest.php index b9ace510..32da3248 100644 --- a/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterTypeContravarianceChangedTest.php +++ b/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterTypeContravarianceChangedTest.php @@ -87,6 +87,13 @@ static function changed1($a, $b) {} function changed2($a, $b) {} } } + +namespace N5 { + interface A {} + class C { + public function changed(A $a) {} + } +} PHP , $astLocator, @@ -125,6 +132,14 @@ static function changed1(int $a, int $b) {} function changed2(int $a, int $b) {} } } + +namespace N5 { + interface A extends B {} + interface B {} + class C { + public function changed(B $a) {} + } +} PHP , $astLocator, @@ -184,6 +199,11 @@ function changed2(int $a, int $b) {} '[BC] CHANGED: The parameter $b of N4\C#changed2() changed from no type to a non-contravariant int', ], ], + 'N5\C#changed' => [ + self::getMethod($fromReflector->reflectClass('N5\C'), 'changed'), + self::getMethod($toReflector->reflectClass('N5\C'), 'changed'), + [], + ], ], ); } From c68165f87a7f56cc75f7c48de6f3b6dca0a0cd32 Mon Sep 17 00:00:00 2001 From: Daniel Badura Date: Mon, 28 Nov 2022 14:14:07 +0100 Subject: [PATCH 2/3] Add TestCase for TypeIsContravariant --- .../Variance/TypeIsContravariantTest.php | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/unit/DetectChanges/Variance/TypeIsContravariantTest.php b/test/unit/DetectChanges/Variance/TypeIsContravariantTest.php index 509b3535..5550d2c6 100644 --- a/test/unit/DetectChanges/Variance/TypeIsContravariantTest.php +++ b/test/unit/DetectChanges/Variance/TypeIsContravariantTest.php @@ -564,6 +564,46 @@ public static function existingNullableTypeStrings(): array ]; } + public function testContravarianceWhereClassWasMissingBefore(): void + { + $astLocator = (new BetterReflection())->astLocator(); + + $fromLocator = new StringSourceLocator( + <<<'PHP' +reflectClass('Service')->getMethod('__invoke')->getParameter('a')->getType(), + (new DefaultReflector($toLocator))->reflectClass('Service')->getMethod('__invoke')->getParameter('a')->getType() + )); + } + private static function identifierType( Reflector $reflector, ReflectionProperty $owner, From f5478e8ce898330848b1661d5d2276450fdbad43 Mon Sep 17 00:00:00 2001 From: Daniel Badura Date: Wed, 30 Sep 2026 23:50:48 +0200 Subject: [PATCH 3/3] Add a PoC solution which involves getting the reflector from the reflection type. This is internal to BetterReflection and there is currently no way of getting it via public api. I suppose this wont change though. --- .../Variance/TypeIsContravariant.php | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/DetectChanges/Variance/TypeIsContravariant.php b/src/DetectChanges/Variance/TypeIsContravariant.php index 06b13e61..ec061399 100644 --- a/src/DetectChanges/Variance/TypeIsContravariant.php +++ b/src/DetectChanges/Variance/TypeIsContravariant.php @@ -6,9 +6,14 @@ use Psl\Iter; use Psl\Str; +use ReflectionProperty; use Roave\BetterReflection\Reflection\ReflectionIntersectionType; use Roave\BetterReflection\Reflection\ReflectionNamedType; use Roave\BetterReflection\Reflection\ReflectionUnionType; +use Roave\BetterReflection\Reflector\Exception\IdentifierNotFound; +use Roave\BetterReflection\Reflector\Reflector; + +use function assert; /** * This is a simplistic contravariant type check. A more appropriate approach would be to @@ -101,8 +106,16 @@ private function compareNamedTypes(ReflectionNamedType $type, ReflectionNamedTyp return false; } - $typeReflectionClass = $type->getClass(); - $comparedTypeClass = $comparedType->getClass(); + $comparedTypeClass = $comparedType->getClass(); + + try { + // the old type has to be resolved in the new codebase, as its inheritance may have changed + $typeReflectionClass = $this->reflectorOf($comparedType) + ->reflectClass($type->getClass()->getName()); + } catch (IdentifierNotFound) { + // the old type no longer exists, so it cannot be a subtype of the new one + return false; + } if ($comparedTypeClass->isInterface()) { return $typeReflectionClass->implementsInterface($comparedTypeClass->getName()); @@ -110,4 +123,17 @@ private function compareNamedTypes(ReflectionNamedType $type, ReflectionNamedTyp return Iter\contains($typeReflectionClass->getParentClassNames(), $comparedTypeClass->getName()); } + + /** + * @todo better-reflection does not expose the reflector a type was created with, therefore + * it is read from its private state here + */ + private function reflectorOf(ReflectionNamedType $type): Reflector + { + $reflector = (new ReflectionProperty($type::class, 'reflector'))->getValue($type); + + assert($reflector instanceof Reflector); + + return $reflector; + } }