Using an internal ReflectionType to preload target `ReflectionClass…

…` for `ReflectionClass` (where applicable) to avoid passing around a `Reflector`
Roave · Apr 15, 2018 · e07a9ce · e07a9ce
1 parent f3efd43
commit e07a9ce
Show file tree

Hide file tree

Showing 5 changed files with 133 additions and 32 deletions.
diff --git a/src/Comparator/Support/ReflectionType.php b/src/Comparator/Support/ReflectionType.php
@@ -0,0 +1,82 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Roave\ApiCompare\Comparator\Support;
+
+use Assert\Assert;
+use Roave\BetterReflection\Reflection\ReflectionClass;
+use Roave\BetterReflection\Reflection\ReflectionType as BetterReflectionType;
+use Roave\BetterReflection\Reflector\ClassReflector;
+
+/**
+ * Wrapper class used to resolve target ReflectionClass instances without having to have
+ * a {@see \Roave\BetterReflection\Reflector\ClassReflector} thrown around at runtime all
+ * over the place.
+ *
+ * @internal
+ */
+final class ReflectionType
+{
+    /** @var BetterReflectionType */
+    private $betterReflectionType;
+
+    /** @var ReflectionClass|null */
+    private $targetClass;
+
+    private function __construct()
+    {
+    }
+
+    public static function fromBetterReflectionTypeAndReflector(
+        ?BetterReflectionType $reflectionType,
+        ClassReflector $classReflector
+    ) : ?self {
+        if (! $reflectionType) {
+            return null;
+        }
+
+        $instance = new self();
+
+        $instance->betterReflectionType = $reflectionType;
+
+        if ($reflectionType->isBuiltin()) {
+            return $instance;
+        }
+
+        $targetClass = $classReflector->reflect($reflectionType->__toString());
+
+        assert($targetClass instanceof ReflectionClass);
+
+        $instance->targetClass = $targetClass;
+
+        return $instance;
+    }
+
+    /** @see \Roave\BetterReflection\Reflection\ReflectionType::allowsNull() */
+    public function allowsNull() : bool
+    {
+        return $this->betterReflectionType->allowsNull();
+    }
+
+    /** @see \Roave\BetterReflection\Reflection\ReflectionType::allowsNull() */
+    public function isBuiltin() : bool
+    {
+        return $this->betterReflectionType->isBuiltin();
+    }
+
+    /** @see \Roave\BetterReflection\Reflection\ReflectionType::__toString() */
+    public function targetClass() : ReflectionClass
+    {
+        Assert::that($this->targetClass)->notNull();
+        assert($this->targetClass instanceof ReflectionClass);
+
+        return $this->targetClass;
+    }
+
+    /** @see \Roave\BetterReflection\Reflection\ReflectionType::__toString() */
+    public function __toString() : string
+    {
+        return $this->betterReflectionType->__toString();
+    }
+}
diff --git a/src/Comparator/Variance/TypeIsContravariant.php b/src/Comparator/Variance/TypeIsContravariant.php
@@ -4,8 +4,7 @@
 
 namespace Roave\ApiCompare\Comparator\Variance;
 
-use Roave\BetterReflection\Reflection\ReflectionClass;
-use Roave\BetterReflection\Reflection\ReflectionType;
+use Roave\ApiCompare\Comparator\Support\ReflectionType;
 use Roave\BetterReflection\Reflector\ClassReflector;
 use function in_array;
 use function strtolower;
@@ -19,7 +18,6 @@
 final class TypeIsContravariant
 {
     public function __invoke(
-        ClassReflector $reflector,
         ?ReflectionType $type,
         ?ReflectionType $comparedType
     ) : bool {
@@ -66,10 +64,8 @@ public function __invoke(
             return false;
         }
 
-        /** @var ReflectionClass $typeReflectionClass */
-        $typeReflectionClass = $reflector->reflect($typeAsString);
-        /** @var ReflectionClass $comparedTypeReflectionClass */
-        $comparedTypeReflectionClass = $reflector->reflect($comparedTypeAsString);
+        $typeReflectionClass         = $type->targetClass();
+        $comparedTypeReflectionClass = $comparedType->targetClass();
 
         if ($comparedTypeReflectionClass->isInterface()) {
             return $typeReflectionClass->implementsInterface($comparedTypeAsString);

diff --git a/src/Comparator/Variance/TypeIsCovariant.php b/src/Comparator/Variance/TypeIsCovariant.php
@@ -4,11 +4,10 @@
 
 namespace Roave\ApiCompare\Comparator\Variance;
 
+use Roave\ApiCompare\Comparator\Support\ReflectionType;
 use Roave\BetterReflection\Reflection\ReflectionClass;
-use Roave\BetterReflection\Reflection\ReflectionType;
-use Roave\BetterReflection\Reflector\ClassReflector;
-use function strtolower;
 use function in_array;
+use function strtolower;
 
 /**
  * This is a simplistic covariant type check. A more appropriate approach would be to
@@ -19,7 +18,6 @@
 final class TypeIsCovariant
 {
     public function __invoke(
-        ClassReflector $reflector,
         ?ReflectionType $type,
         ?ReflectionType $comparedType
     ) : bool {
@@ -61,7 +59,7 @@ public function __invoke(
 
         if (strtolower($typeAsString) === 'iterable' && ! $comparedType->isBuiltin()) {
             /** @var ReflectionClass $comparedTypeReflectionClass */
-            $comparedTypeReflectionClass = $reflector->reflect($comparedTypeAsString);
+            $comparedTypeReflectionClass = $comparedType->targetClass();
 
             if ($comparedTypeReflectionClass->implementsInterface(\Traversable::class)) {
                 // `iterable` can be restricted via any `Iterator` implementation
@@ -79,10 +77,8 @@ public function __invoke(
             return false;
         }
 
-        /** @var ReflectionClass $typeReflectionClass */
-        $typeReflectionClass = $reflector->reflect($typeAsString);
-        /** @var ReflectionClass $comparedTypeReflectionClass */
-        $comparedTypeReflectionClass = $reflector->reflect($comparedTypeAsString);
+        $typeReflectionClass         = $type->targetClass();
+        $comparedTypeReflectionClass = $comparedType->targetClass();
 
         if ($typeReflectionClass->isInterface()) {
             return $comparedTypeReflectionClass->implementsInterface($typeAsString);

diff --git a/test/unit/Comparator/Variance/TypeIsContravariantTest.php b/test/unit/Comparator/Variance/TypeIsContravariantTest.php
@@ -5,6 +5,7 @@
 namespace RoaveTest\ApiCompare\Comparator\BackwardsCompatibility\ClassBased;
 
 use PHPUnit\Framework\TestCase;
+use Roave\ApiCompare\Comparator\Support\ReflectionType as InternalReflectionType;
 use Roave\ApiCompare\Comparator\Variance\TypeIsContravariant;
 use Roave\BetterReflection\BetterReflection;
 use Roave\BetterReflection\Reflection\ReflectionType;
@@ -40,7 +41,10 @@ class CClass extends BClass {}
         self::assertSame(
             $expectedToBeContravariant,
             (new TypeIsContravariant())
-                ->__invoke($reflector, $type, $newType)
+                ->__invoke(
+                    InternalReflectionType::fromBetterReflectionTypeAndReflector($type, $reflector),
+                    InternalReflectionType::fromBetterReflectionTypeAndReflector($newType, $reflector)
+                )
         );
     }
 
@@ -92,12 +96,12 @@ public function checkedTypes() : array
                 null,
                 true,
             ],
-            'iterable to array is not contravariant'                 => [
+            'iterable to array is not contravariant'                                   => [
                 ReflectionType::createFromType('iterable', false),
                 ReflectionType::createFromType('array', false),
                 false,
             ],
-            'array to iterable is contravariant'                 => [
+            'array to iterable is contravariant'                                       => [
                 ReflectionType::createFromType('array', false),
                 ReflectionType::createFromType('iterable', false),
                 true,
@@ -107,7 +111,7 @@ public function checkedTypes() : array
                 ReflectionType::createFromType('AnotherClassWithMultipleInterfaces', false),
                 false,
             ],
-            'iterable to iterable class type is not contravariant'                         => [
+            'iterable to iterable class type is not contravariant'                     => [
                 ReflectionType::createFromType('iterable', false),
                 ReflectionType::createFromType('Iterator', false),
                 false,
@@ -211,7 +215,10 @@ class AClass {}
 
         self::assertTrue(
             (new TypeIsContravariant())
-                ->__invoke($reflector, $type, $type)
+                ->__invoke(
+                    InternalReflectionType::fromBetterReflectionTypeAndReflector($type, $reflector),
+                    InternalReflectionType::fromBetterReflectionTypeAndReflector($type, $reflector)
+                )
         );
     }
 
@@ -244,9 +251,7 @@ function (string $type) : array {
     /** @dataProvider existingNullableTypeStrings */
     public function testContravarianceConsidersNullability(string $type) : void
     {
-        $nullable = ReflectionType::createFromType($type, true);
-        $notNullable = ReflectionType::createFromType($type, false);
-        $reflector = new ClassReflector(new StringSourceLocator(
+        $reflector   = new ClassReflector(new StringSourceLocator(
             <<<'PHP'
 <?php
 
@@ -256,11 +261,19 @@ class AClass {}
             ,
             (new BetterReflection())->astLocator()
         ));
+        $nullable    = InternalReflectionType::fromBetterReflectionTypeAndReflector(
+            ReflectionType::createFromType($type, true),
+            $reflector
+        );
+        $notNullable = InternalReflectionType::fromBetterReflectionTypeAndReflector(
+            ReflectionType::createFromType($type, false),
+            $reflector
+        );
 
         $isContravariant = new TypeIsContravariant();
 
-        self::assertFalse($isContravariant->__invoke($reflector, $nullable, $notNullable));
-        self::assertTrue($isContravariant->__invoke($reflector, $notNullable, $nullable));
+        self::assertFalse($isContravariant->__invoke($nullable, $notNullable));
+        self::assertTrue($isContravariant->__invoke($notNullable, $nullable));
     }
 
     /** @return string[][] */

diff --git a/test/unit/Comparator/Variance/TypeIsCovariantTest.php b/test/unit/Comparator/Variance/TypeIsCovariantTest.php
@@ -5,6 +5,7 @@
 namespace RoaveTest\ApiCompare\Comparator\BackwardsCompatibility\ClassBased;
 
 use PHPUnit\Framework\TestCase;
+use Roave\ApiCompare\Comparator\Support\ReflectionType as InternalReflectionType;
 use Roave\ApiCompare\Comparator\Variance\TypeIsCovariant;
 use Roave\BetterReflection\BetterReflection;
 use Roave\BetterReflection\Reflection\ReflectionType;
@@ -40,7 +41,10 @@ class CClass extends BClass {}
         self::assertSame(
             $expectedToBeContravariant,
             (new TypeIsCovariant())
-                ->__invoke($reflector, $type, $newType)
+                ->__invoke(
+                    InternalReflectionType::fromBetterReflectionTypeAndReflector($type, $reflector),
+                    InternalReflectionType::fromBetterReflectionTypeAndReflector($newType, $reflector)
+                )
         );
     }
 
@@ -202,7 +206,10 @@ class AClass {}
 
         self::assertTrue(
             (new TypeIsCovariant())
-                ->__invoke($reflector, $type, $type)
+                ->__invoke(
+                    InternalReflectionType::fromBetterReflectionTypeAndReflector($type, $reflector),
+                    InternalReflectionType::fromBetterReflectionTypeAndReflector($type, $reflector)
+                )
         );
     }
 
@@ -235,8 +242,6 @@ function (string $type) : array {
     /** @dataProvider existingNullableTypeStrings */
     public function testCovarianceConsidersNullability(string $type) : void
     {
-        $nullable = ReflectionType::createFromType($type, true);
-        $notNullable = ReflectionType::createFromType($type, false);
         $reflector = new ClassReflector(new StringSourceLocator(
             <<<'PHP'
 <?php
@@ -248,10 +253,19 @@ class AClass {}
             (new BetterReflection())->astLocator()
         ));
 
+        $nullable    = InternalReflectionType::fromBetterReflectionTypeAndReflector(
+            ReflectionType::createFromType($type, true),
+            $reflector
+        );
+        $notNullable = InternalReflectionType::fromBetterReflectionTypeAndReflector(
+            ReflectionType::createFromType($type, false),
+            $reflector
+        );
+
         $isCovariant = new TypeIsCovariant();
 
-        self::assertTrue($isCovariant->__invoke($reflector, $nullable, $notNullable));
-        self::assertFalse($isCovariant->__invoke($reflector, $notNullable, $nullable));
+        self::assertTrue($isCovariant->__invoke($nullable, $notNullable));
+        self::assertFalse($isCovariant->__invoke($notNullable, $nullable));
     }
 
     /** @return string[][] */