Merge pull request #388 from dedoc/219-reflectionuniontype-error

Proper handling of union types when used as a typehints for route parameters
dedoc · May 21, 2024 · d8d66b9 · d8d66b9
2 parents 0d107f8 + 28f2d1f
commit d8d66b9
Show file tree

Hide file tree

Showing 5 changed files with 135 additions and 62 deletions.
diff --git a/src/Support/OperationExtensions/RequestEssentialsExtension.php b/src/Support/OperationExtensions/RequestEssentialsExtension.php
@@ -4,23 +4,26 @@
 
 use Dedoc\Scramble\Extensions\OperationExtension;
 use Dedoc\Scramble\Infer;
+use Dedoc\Scramble\PhpDoc\PhpDocTypeHelper;
 use Dedoc\Scramble\Scramble;
 use Dedoc\Scramble\Support\Generator\OpenApi;
 use Dedoc\Scramble\Support\Generator\Operation;
 use Dedoc\Scramble\Support\Generator\Parameter;
 use Dedoc\Scramble\Support\Generator\Schema;
 use Dedoc\Scramble\Support\Generator\Server;
-use Dedoc\Scramble\Support\Generator\Types\BooleanType;
-use Dedoc\Scramble\Support\Generator\Types\IntegerType;
-use Dedoc\Scramble\Support\Generator\Types\NumberType;
 use Dedoc\Scramble\Support\Generator\Types\StringType;
 use Dedoc\Scramble\Support\Generator\Types\Type;
 use Dedoc\Scramble\Support\Generator\TypeTransformer;
 use Dedoc\Scramble\Support\Generator\UniqueNameOptions;
 use Dedoc\Scramble\Support\PhpDoc;
 use Dedoc\Scramble\Support\RouteInfo;
 use Dedoc\Scramble\Support\ServerFactory;
+use Dedoc\Scramble\Support\Type\IntegerType;
 use Dedoc\Scramble\Support\Type\ObjectType;
+use Dedoc\Scramble\Support\Type\Type as InferType;
+use Dedoc\Scramble\Support\Type\TypeHelper;
+use Dedoc\Scramble\Support\Type\Union;
+use Dedoc\Scramble\Support\Type\UnknownType;
 use Illuminate\Contracts\Container\BindingResolutionException;
 use Illuminate\Database\Eloquent\Concerns\HasUuids;
 use Illuminate\Database\Eloquent\Model;
@@ -30,6 +33,7 @@
 use Illuminate\Support\Str;
 use PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode;
 use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocNode;
+use ReflectionParameter;
 
 class RequestEssentialsExtension extends OperationExtension
 {
@@ -50,7 +54,10 @@ public function __construct(
 
     public function handle(Operation $operation, RouteInfo $routeInfo)
     {
-        [$pathParams, $pathAliases] = $this->getRoutePathParameters($routeInfo->route, $routeInfo->phpDoc());
+        [$pathParams, $pathAliases] = $this->getRoutePathParameters(
+            $routeInfo->route,
+            $routeInfo->phpDoc(),
+        );
 
         $tagResolver = Scramble::$tagResolver ?? function (RouteInfo $routeInfo) {
             return array_unique([
@@ -147,8 +154,8 @@ private function getRoutePathParameters(Route $route, ?PhpDocNode $methodPhpDocN
     {
         $paramNames = $route->parameterNames();
         $paramsWithRealNames = ($reflectionParams = collect($route->signatureParameters())
-            ->filter(function (\ReflectionParameter $v) {
-                if (($type = $v->getType()) && $typeName = $type->getName()) {
+            ->filter(function (ReflectionParameter $v) {
+                if (($type = $v->getType()) && ($type instanceof \ReflectionNamedType) && ($typeName = $type->getName())) {
                     if (is_a($typeName, Request::class, true)) {
                         return false;
                     }
@@ -157,7 +164,7 @@ private function getRoutePathParameters(Route $route, ?PhpDocNode $methodPhpDocN
                 return true;
             })
             ->values())
-            ->map(fn (\ReflectionParameter $v) => $v->name)
+            ->map(fn (ReflectionParameter $v) => $v->name)
             ->all();
 
         if (count($paramNames) !== count($paramsWithRealNames)) {
@@ -180,60 +187,56 @@ private function getRoutePathParameters(Route $route, ?PhpDocNode $methodPhpDocN
         $params = array_map(function (string $paramName) use ($route, $aliases, $reflectionParamsByKeys, $phpDocTypehintParam) {
             $paramName = $aliases[$paramName];
 
-            $description = '';
-            $type = null;
+            $description = $phpDocTypehintParam[$paramName]?->description ?? '';
+            [$schemaType, $description] = $this->getParameterType(
+                $paramName,
+                $description,
+                $route,
+                $phpDocTypehintParam[$paramName] ?? null,
+                $reflectionParamsByKeys[$paramName] ?? null,
+            );
 
-            if (isset($reflectionParamsByKeys[$paramName]) || isset($phpDocTypehintParam[$paramName])) {
-                /** @var ParamTagValueNode $docParam */
-                if ($docParam = $phpDocTypehintParam[$paramName] ?? null) {
-                    if ($docType = $docParam->type) {
-                        $type = (string) $docType;
-                    }
-                    if ($docParam->description) {
-                        $description = $docParam->description;
-                    }
-                }
+            return Parameter::make($paramName, 'path')
+                ->description($description)
+                ->setSchema(Schema::fromType($schemaType));
+        }, array_values(array_diff($route->parameterNames(), $this->getParametersFromString($route->getDomain()))));
 
-                if (
-                    ($reflectionParam = $reflectionParamsByKeys[$paramName] ?? null)
-                    && ($reflectionParam->hasType())
-                ) {
-                    /** @var \ReflectionParameter $reflectionParam */
-                    $type = $reflectionParam->getType()->getName();
-                }
-            }
+        return [$params, $aliases];
+    }
 
-            $schemaTypesMap = [
-                'int' => new IntegerType(),
-                'float' => new NumberType(),
-                'string' => new StringType(),
-                'bool' => new BooleanType(),
-            ];
+    private function getParameterType(string $paramName, string $description, Route $route, ?ParamTagValueNode $phpDocParam, ?ReflectionParameter $reflectionParam)
+    {
+        $type = new UnknownType;
 
-            $isEnum = function_exists('enum_exists') && enum_exists($type);
-            $schemaType = $isEnum
-                ? $this->openApiTransformer->transform(new ObjectType($type))
-                : ($type ? ($schemaTypesMap[$type] ?? new IntegerType) : new StringType);
+        if ($phpDocParam?->type) {
+            $type = PhpDocTypeHelper::toType($phpDocParam->type);
+        }
 
-            $isModelId = $type && ! $isEnum && ! isset($schemaTypesMap[$type]);
+        if ($reflectionParam?->hasType()) {
+            $type = TypeHelper::createTypeFromReflectionType($reflectionParam->getType());
+        }
 
-            if ($isModelId) {
-                [$schemaType, $description] = $this->getModelIdTypeAndDescription($schemaType, $type, $paramName, $description, $route->bindingFields()[$paramName] ?? null);
+        $simplifiedType = Union::wrap(array_map(
+            fn (InferType $t) => $t instanceof ObjectType
+                ? (enum_exists($t->name) ? $t : new \Dedoc\Scramble\Support\Type\StringType)
+                : $t,
+            $type instanceof Union ? $type->types : [$type],
+        ));
 
-                $schemaType->setAttribute('isModelId', true);
-            }
+        $schemaType = $this->openApiTransformer->transform($simplifiedType);
 
-            return Parameter::make($paramName, 'path')
-                ->description($description)
-                ->setSchema(Schema::fromType($schemaType));
-        }, array_values(array_diff($route->parameterNames(), $this->getParametersFromString($route->getDomain()))));
+        if ($isModelId = $type instanceof ObjectType) {
+            [$schemaType, $description] = $this->getModelIdTypeAndDescription($schemaType, $type, $paramName, $description, $route->bindingFields()[$paramName] ?? null);
 
-        return [$params, $aliases];
+            $schemaType->setAttribute('isModelId', true);
+        }
+
+        return [$schemaType, $description ?? ''];
     }
 
     private function getModelIdTypeAndDescription(
         Type $baseType,
-        string $type,
+        InferType $type,
         string $paramName,
         string $description,
         ?string $bindingField,
@@ -243,19 +246,20 @@ private function getModelIdTypeAndDescription(
             $description ?: 'The '.Str::of($paramName)->kebab()->replace(['-', '_'], ' ').' ID',
         ];
 
-        if (! is_a($type, Model::class, true)) {
+        if (! $type->isInstanceOf(Model::class)) {
             return $defaults;
         }
 
+        /** @var ObjectType $type */
+        $defaults[0] = $this->openApiTransformer->transform(new IntegerType);
+
         try {
             /** @var Model $modelInstance */
-            $modelInstance = resolve($type);
+            $modelInstance = resolve($type->name);
         } catch (BindingResolutionException) {
             return $defaults;
         }
 
-        $this->infer->analyzeClass($type);
-
         $modelKeyName = $modelInstance->getKeyName();
         $routeKeyName = $bindingField ?: $modelInstance->getRouteKeyName();
 
@@ -267,14 +271,17 @@ private function getModelIdTypeAndDescription(
             $description = 'The '.Str::of($paramName)->kebab()->replace(['-', '_'], ' ').' '.$keyDescriptionName;
         }
 
-        $modelTraits = class_uses($type);
+        $modelTraits = class_uses($type->name);
         if ($routeKeyName === $modelKeyName && Arr::has($modelTraits, HasUuids::class)) {
             return [(new StringType)->format('uuid'), $description];
         }
 
-        return [$this->openApiTransformer->transform(
-            (new ObjectType($type))->getPropertyType($routeKeyName),
-        ), $description];
+        $propertyType = $type->getPropertyType($routeKeyName);
+        if ($propertyType instanceof UnknownType) {
+            $propertyType = new IntegerType;
+        }
+
+        return [$this->openApiTransformer->transform($propertyType), $description];
     }
 
     private function getOperationId(RouteInfo $routeInfo)

diff --git a/src/Support/OperationExtensions/RulesExtractor/FormRequestRulesExtractor.php b/src/Support/OperationExtensions/RulesExtractor/FormRequestRulesExtractor.php
@@ -64,6 +64,10 @@ public function extract(Route $route)
 
     private function findCustomRequestParam(Param $param)
     {
+        if (! $param->type || ! method_exists($param->type, '__toString')) {
+            return false;
+        }
+
         $className = (string) $param->type;
 
         return method_exists($className, 'rules');

diff --git a/src/Support/Type/TypeHelper.php b/src/Support/Type/TypeHelper.php
@@ -9,6 +9,9 @@
 use Illuminate\Support\Collection;
 use PhpParser\Node;
 use PhpParser\PrettyPrinter\Standard;
+use ReflectionNamedType;
+use ReflectionType;
+use ReflectionUnionType;
 
 class TypeHelper
 {
@@ -146,4 +149,43 @@ public static function createTypeFromValue(mixed $value)
 
         return null; // @todo: object
     }
+
+    public static function createTypeFromReflectionType(ReflectionType $reflectionType, bool $handleNullable = true)
+    {
+        if ($reflectionType->allowsNull() && $handleNullable) {
+            return Union::wrap([
+                new NullType(),
+                static::createTypeFromReflectionType($reflectionType, handleNullable: false),
+            ]);
+        }
+
+        if ($reflectionType instanceof ReflectionUnionType) {
+            return Union::wrap(array_map(
+                fn ($node) => static::createTypeFromReflectionType($node, $handleNullable),
+                $reflectionType->getTypes(),
+            ));
+        }
+
+        if ($reflectionType instanceof ReflectionNamedType) {
+            if ($reflectionType->getName() === 'int') {
+                return new IntegerType();
+            }
+
+            if ($reflectionType->getName() === 'string') {
+                return new StringType();
+            }
+
+            if ($reflectionType->getName() === 'bool') {
+                return new BooleanType();
+            }
+
+            if ($reflectionType->getName() === 'float') {
+                return new FloatType();
+            }
+
+            return new ObjectType($reflectionType->getName());
+        }
+
+        return new UnknownType('Cannot create type from reflection type '.((string) $reflectionType));
+    }
 }
diff --git a/tests/ErrorsResponsesTest.php b/tests/ErrorsResponsesTest.php
@@ -57,11 +57,10 @@
 });
 
 it('adds not found error response', function () {
-    RouteFacade::get('api/test/{user}', [ErrorsResponsesTest_Controller::class, 'adds_not_found_error_response'])
-        ->middleware('can:update,post');
-
-    Scramble::routes(fn (Route $r) => $r->uri === 'api/test/{user}');
-    $openApiDocument = app()->make(\Dedoc\Scramble\Generator::class)();
+    $openApiDocument = generateForRoute(function () {
+        return RouteFacade::get('api/test/{user}', [ErrorsResponsesTest_Controller::class, 'adds_not_found_error_response'])
+            ->middleware('can:update,post');
+    });
 
     assertMatchesSnapshot($openApiDocument);
 });
@@ -114,7 +113,7 @@ public function phpdoc_exception_response(Illuminate\Http\Request $request)
     }
 }
 
-class UserModel_ErrorsResponsesTest
+class UserModel_ErrorsResponsesTest extends \Illuminate\Database\Eloquent\Model
 {
 }
 

diff --git a/tests/Support/OperationExtensions/RequestEssentialsExtensionTest.php b/tests/Support/OperationExtensions/RequestEssentialsExtensionTest.php
@@ -79,6 +79,27 @@ public function foo(SampleUserModel $user)
     {
     }
 }
+
+it('determines default route key type for union', function () {
+    $openApiDocument = generateForRoute(function () {
+        return RouteFacade::get('api/test/{user}', [UnionKey_RequestEssentialsExtensionTest_Controller::class, 'foo']);
+    });
+
+    expect($openApiDocument['paths']['/test/{user}']['get']['parameters'][0]['schema'])
+        ->toBe([
+            'anyOf' => [
+                ['type' => 'string'],
+                ['type' => 'integer'],
+            ],
+        ]);
+});
+class UnionKey_RequestEssentialsExtensionTest_Controller
+{
+    public function foo(string|int $user)
+    {
+    }
+}
+
 it('handles enum in route parameter', function () {
     $openApiDocument = generateForRoute(function () {
         return RouteFacade::get('api/test/{status}', [EnumParameter_RequestEssentialsExtensionTest_Controller::class, 'foo']);