Merge pull request #803 from kukulich/array-callable

Fixed `ReflectionParameter::isArray()` and `isCallable()` to handle union types
Roave · Oct 12, 2021 · 313eaf9 · 313eaf9
2 parents a4c07be + be964c0
commit 313eaf9
Show file tree

Hide file tree

Showing 5 changed files with 123 additions and 25 deletions.
diff --git a/src/Reflection/ReflectionParameter.php b/src/Reflection/ReflectionParameter.php
@@ -30,7 +30,6 @@
 use function is_array;
 use function is_object;
 use function is_string;
-use function preg_match;
 use function sprintf;
 use function strtolower;
 
@@ -409,15 +408,49 @@ public function removeType(): void
      */
     public function isArray(): bool
     {
-        return preg_match('~^\??array$~i', (string) $this->getType()) === 1;
+        return $this->isType($this->getType(), 'array');
     }
 
     /**
      * Is this parameter a callable?
      */
     public function isCallable(): bool
     {
-        return preg_match('~^\??callable$~i', (string) $this->getType()) === 1;
+        return $this->isType($this->getType(), 'callable');
+    }
+
+    /**
+     * For isArray() and isCallable().
+     */
+    private function isType(ReflectionNamedType|ReflectionUnionType|null $typeReflection, string $type): bool
+    {
+        if ($typeReflection === null) {
+            return false;
+        }
+
+        $isOneOfAllowedTypes = static function (ReflectionNamedType $namedType, string ...$types): bool {
+            foreach ($types as $type) {
+                if (strtolower($namedType->getName()) === $type) {
+                    return true;
+                }
+            }
+
+            return false;
+        };
+
+        if ($typeReflection instanceof ReflectionUnionType) {
+            $unionTypes = $typeReflection->getTypes();
+
+            foreach ($unionTypes as $unionType) {
+                if (! $isOneOfAllowedTypes($unionType, $type, 'null')) {
+                    return false;
+                }
+            }
+
+            return true;
+        }
+
+        return $isOneOfAllowedTypes($typeReflection, $type);
     }
 
     /**
@@ -495,7 +528,6 @@ private function getClassName(): ?string
 
         if ($type instanceof ReflectionUnionType) {
             foreach ($type->getTypes() as $innerType) {
-                assert($innerType instanceof ReflectionNamedType);
                 $innerTypeClassName = $this->getClassNameFromNamedType($innerType);
                 if ($innerTypeClassName !== null) {
                     return $innerTypeClassName;

diff --git a/src/Reflection/ReflectionUnionType.php b/src/Reflection/ReflectionUnionType.php
@@ -7,21 +7,27 @@
 use PhpParser\Node\UnionType;
 
 use function array_map;
+use function assert;
 use function implode;
 
 class ReflectionUnionType extends ReflectionType
 {
-    /** @var list<ReflectionNamedType|ReflectionUnionType> */
+    /** @var list<ReflectionNamedType> */
     private array $types;
 
     public function __construct(UnionType $type, bool $allowsNull)
     {
         parent::__construct($allowsNull);
-        $this->types = array_map(static fn ($type): ReflectionNamedType|ReflectionUnionType => ReflectionType::createFromTypeAndReflector($type), $type->types);
+        $this->types = array_map(static function ($type): ReflectionNamedType {
+            $reflectionType = ReflectionType::createFromTypeAndReflector($type);
+            assert($reflectionType instanceof ReflectionNamedType);
+
+            return $reflectionType;
+        }, $type->types);
     }
 
     /**
-     * @return list<ReflectionNamedType|ReflectionUnionType>
+     * @return list<ReflectionNamedType>
      */
     public function getTypes(): array
     {

diff --git a/test/unit/Fixture/Methods.php b/test/unit/Fixture/Methods.php
@@ -63,6 +63,36 @@ public function methodWithExplicitTypedParameters(
     ) {
     }
 
+    public function methodIsArrayParameters(
+        $noTypeParameter,
+        bool $boolParameter,
+        array $arrayParameter,
+        ArRaY $arrayCaseInsensitiveParameter,
+        ?array $nullableArrayParameter,
+        null|array $unionArrayParameterNullFirst,
+        array|null $unionArrayParameterNullLast,
+        string|bool $unionNotArrayParameter,
+        array|string|null $unionWithArrayNotArrayParameter,
+        array|object $unionWithArrayAndObjectNotArrayParameter,
+    )
+    {
+    }
+
+    public function methodIsCallableParameters(
+        $noTypeParameter,
+        bool $boolParameter,
+        callable $callableParameter,
+        cAlLaBlE $callableCaseInsensitiveParameter,
+        ?callable $nullableCallableParameter,
+        null|callable $unionCallableParameterNullFirst,
+        callable|null $unionCallableParameterNullLast,
+        string|bool $unionNotCallableParameter,
+        callable|string|null $unionWithCallableNotCallableParameter,
+        callable|object $unionWithCallableAndObjectNotArrayParameter,
+    )
+    {
+    }
+
     public function methodWithVariadic($nonVariadicParameter, ...$variadicParameter)
     {
     }

diff --git a/test/unit/Reflection/ReflectionClassTest.php b/test/unit/Reflection/ReflectionClassTest.php
@@ -176,7 +176,7 @@ public function getMethodsWithFilterDataProvider(): array
             [CoreReflectionMethod::IS_STATIC, 1],
             [CoreReflectionMethod::IS_ABSTRACT, 1],
             [CoreReflectionMethod::IS_FINAL, 1],
-            [CoreReflectionMethod::IS_PUBLIC, 17],
+            [CoreReflectionMethod::IS_PUBLIC, 19],
             [CoreReflectionMethod::IS_PROTECTED, 1],
             [CoreReflectionMethod::IS_PRIVATE, 1],
             [
@@ -186,7 +186,7 @@ public function getMethodsWithFilterDataProvider(): array
                 CoreReflectionMethod::IS_PUBLIC |
                 CoreReflectionMethod::IS_PROTECTED |
                 CoreReflectionMethod::IS_PRIVATE,
-                19,
+                21,
             ],
         ];
     }

diff --git a/test/unit/Reflection/ReflectionParameterTest.php b/test/unit/Reflection/ReflectionParameterTest.php
@@ -401,30 +401,60 @@ public function testRemoveType(): void
         );
     }
 
-    public function testIsCallable(): void
+    public function isCallableProvider(): array
     {
-        $classInfo = $this->reflector->reflectClass(Methods::class);
-
-        $method = $classInfo->getMethod('methodWithExplicitTypedParameters');
+        return [
+            ['noTypeParameter', false],
+            ['boolParameter', false],
+            ['callableParameter', true],
+            ['callableCaseInsensitiveParameter', true],
+            ['nullableCallableParameter', true],
+            ['unionCallableParameterNullFirst', true],
+            ['unionCallableParameterNullLast', true],
+            ['unionNotCallableParameter', false],
+            ['unionWithCallableNotCallableParameter', false],
+            ['unionWithCallableAndObjectNotArrayParameter', false],
+        ];
+    }
 
-        $nonCallableParam = $method->getParameter('stdClassParameter');
-        self::assertFalse($nonCallableParam->isCallable());
+    /**
+     * @dataProvider isCallableProvider
+     */
+    public function testIsCallable(string $parameterName, bool $isCallable): void
+    {
+        $classReflection     = $this->reflector->reflectClass(Methods::class);
+        $methodReflection    = $classReflection->getMethod('methodIsCallableParameters');
+        $parameterReflection = $methodReflection->getParameter($parameterName);
 
-        $callableParam = $method->getParameter('callableParameter');
-        self::assertTrue($callableParam->isCallable());
+        self::assertSame($isCallable, $parameterReflection->isCallable());
     }
 
-    public function testIsArray(): void
+    public function isArrayProvider(): array
     {
-        $classInfo = $this->reflector->reflectClass(Methods::class);
-
-        $method = $classInfo->getMethod('methodWithExplicitTypedParameters');
+        return [
+            ['noTypeParameter', false],
+            ['boolParameter', false],
+            ['arrayParameter', true],
+            ['arrayCaseInsensitiveParameter', true],
+            ['nullableArrayParameter', true],
+            ['unionArrayParameterNullFirst', true],
+            ['unionArrayParameterNullLast', true],
+            ['unionNotArrayParameter', false],
+            ['unionWithArrayNotArrayParameter', false],
+            ['unionWithArrayAndObjectNotArrayParameter', false],
+        ];
+    }
 
-        $nonArrayParam = $method->getParameter('stdClassParameter');
-        self::assertFalse($nonArrayParam->isArray());
+    /**
+     * @dataProvider isArrayProvider
+     */
+    public function testIsArray(string $parameterName, bool $isArray): void
+    {
+        $classReflection     = $this->reflector->reflectClass(Methods::class);
+        $methodReflection    = $classReflection->getMethod('methodIsArrayParameters');
+        $parameterReflection = $methodReflection->getParameter($parameterName);
 
-        $arrayParam = $method->getParameter('arrayParameter');
-        self::assertTrue($arrayParam->isArray());
+        self::assertSame($isArray, $parameterReflection->isArray());
     }
 
     public function testIsVariadic(): void