From 73c502d5c50f3a574003c1bc5a5b8714375257ad Mon Sep 17 00:00:00 2001 From: Fabio Ivona Date: Mon, 11 Jan 2021 09:15:22 +0100 Subject: [PATCH] Fix broken ReflectionUnionTypes (#1132) * Fix exceptions on ReflectionUnionTypes this commit fixes exceptions thrown on ReflectionUnionType::isBuiltIn() and ReflectionUnionType::getNme() called on php8 union types * Fix exceptions on ReflectionUnionTypes this commit fixes exceptions thrown on ReflectionUnionType::isBuiltIn() and ReflectionUnionType::getNme() called on php8 union types * add check for php8.0 * fix static analysis error * fix $types variable undefined * fix failing test * fix failing test * fixed style with php-cs-fixer * add test for union types in parameters and return type * add test for nullable union types in parameters and return type * updated CHANGELOG.md --- CHANGELOG.md | 2 +- src/Console/ModelsCommand.php | 62 ++++++++++++++----- .../UnionTypes/Models/UnionTypeModel.php | 32 ++++++++++ .../Console/ModelsCommand/UnionTypes/Test.php | 34 ++++++++++ .../__snapshots__/Test__test__1.php | 45 ++++++++++++++ 5 files changed, 157 insertions(+), 18 deletions(-) create mode 100644 tests/Console/ModelsCommand/UnionTypes/Models/UnionTypeModel.php create mode 100644 tests/Console/ModelsCommand/UnionTypes/Test.php create mode 100644 tests/Console/ModelsCommand/UnionTypes/__snapshots__/Test__test__1.php diff --git a/CHANGELOG.md b/CHANGELOG.md index eaf61d0..91877d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,7 @@ All notable changes to this project will be documented in this file. - Allow model_locations to have glob patterns [\#1059 / saackearl](https://github.com/barryvdh/laravel-ide-helper/pull/1059) - Error when generating helper for macroable classes which are not facades and contain a "fake" method [\#1066 / domkrm] (https://github.com/barryvdh/laravel-ide-helper/pull/1066) - Casts with a return type of `static` or `$this` now resolve to an instance of the cast [\#1103 / riesjart](https://github.com/barryvdh/laravel-ide-helper/pull/1103) - +- Broken ReflectionUnionTypes [\#1132 / def-studio](https://github.com/barryvdh/laravel-ide-helper/pull/1132) ### Removed - Removed format and broken generateJsonHelper [\#1053 / mfn](https://github.com/barryvdh/laravel-ide-helper/pull/1053) diff --git a/src/Console/ModelsCommand.php b/src/Console/ModelsCommand.php index 862c7ed..4366f0e 100644 --- a/src/Console/ModelsCommand.php +++ b/src/Console/ModelsCommand.php @@ -35,7 +35,9 @@ use Illuminate\Filesystem\Filesystem; use Illuminate\Support\Str; use phpDocumentor\Reflection\Types\ContextFactory; use ReflectionClass; +use ReflectionNamedType; use ReflectionObject; +use ReflectionType; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -578,7 +580,7 @@ class ModelsCommand extends Command $reflection = new \ReflectionMethod($model, $method); if ($returnType = $reflection->getReturnType()) { - $type = $returnType instanceof \ReflectionNamedType + $type = $returnType instanceof ReflectionNamedType ? $returnType->getName() : (string)$returnType; } else { @@ -1013,16 +1015,12 @@ class ModelsCommand extends Command return null; } - $type = $returnType instanceof \ReflectionNamedType - ? $returnType->getName() - : (string)$returnType; + $types = $this->extractReflectionTypes($returnType); - if (!$returnType->isBuiltin()) { - $type = '\\' . $type; - } + $type = implode('|', $types); - if ($returnType->allowsNull()) { - $type .= '|null'; + if($returnType->allowsNull()){ + $type .='|null'; } return $type; @@ -1215,17 +1213,19 @@ class ModelsCommand extends Command protected function getParamType(\ReflectionMethod $method, \ReflectionParameter $parameter): ?string { if ($paramType = $parameter->getType()) { - $parameterName = $paramType->getName(); + $types = $this->extractReflectionTypes($paramType); - if (!$paramType->isBuiltin()) { - $parameterName = '\\' . $parameterName; + $type = implode('|', $types); + + if($paramType->allowsNull()){ + if(count($types)==1){ + $type = '?' . $type; + }else{ + $type .='|null'; + } } - if ($paramType->allowsNull()) { - return '?' . $parameterName; - } - - return $parameterName; + return $type; } $docComment = $method->getDocComment(); @@ -1290,4 +1290,32 @@ class ModelsCommand extends Command // then we have found the type of the variable if not we return null return $type; } + + protected function extractReflectionTypes(ReflectionType $reflection_type) + { + if($reflection_type instanceof ReflectionNamedType){ + $types[] = $this->getReflectionNamedType($reflection_type); + }else{ + $types = []; + foreach ($reflection_type->getTypes() as $named_type){ + if($named_type->getName()==='null'){ + continue; + } + + $types[] = $this->getReflectionNamedType($named_type); + } + } + + return $types; + } + + protected function getReflectionNamedType(ReflectionNamedType $paramType): string + { + $parameterName = $paramType->getName(); + if (!$paramType->isBuiltin()) { + $parameterName = '\\' . $parameterName; + } + + return $parameterName; + } } diff --git a/tests/Console/ModelsCommand/UnionTypes/Models/UnionTypeModel.php b/tests/Console/ModelsCommand/UnionTypes/Models/UnionTypeModel.php new file mode 100644 index 0000000..6f60d18 --- /dev/null +++ b/tests/Console/ModelsCommand/UnionTypes/Models/UnionTypeModel.php @@ -0,0 +1,32 @@ +where('foo', $bar); + } + + public function scopeWithNullableUnionTypeParameter(Builder $query, null|string|int $bar): Builder + { + return $query->where('foo', $bar); + } + + public function withUnionTypeReturn(): HasMany|UnionTypeModel + { + return $this->hasMany(UnionTypeModel::class); + } + + public function getFooAttribute(): string|int|null + { + return $this->getAttribute('foo'); + } +} diff --git a/tests/Console/ModelsCommand/UnionTypes/Test.php b/tests/Console/ModelsCommand/UnionTypes/Test.php new file mode 100644 index 0000000..5429340 --- /dev/null +++ b/tests/Console/ModelsCommand/UnionTypes/Test.php @@ -0,0 +1,34 @@ +markTestSkipped('This test requires PHP 8.0 or higher'); + } + } + + public function test(): void + { + $command = $this->app->make(ModelsCommand::class); + + $tester = $this->runCommand($command, [ + '--write' => true, + ]); + + $this->assertSame(0, $tester->getStatusCode()); + $this->assertStringContainsString('Written new phpDocBlock to', $tester->getDisplay()); + $this->assertMatchesMockedSnapshot(); + } +} diff --git a/tests/Console/ModelsCommand/UnionTypes/__snapshots__/Test__test__1.php b/tests/Console/ModelsCommand/UnionTypes/__snapshots__/Test__test__1.php new file mode 100644 index 0000000..8f62164 --- /dev/null +++ b/tests/Console/ModelsCommand/UnionTypes/__snapshots__/Test__test__1.php @@ -0,0 +1,45 @@ +where('foo', $bar); + } + + public function scopeWithNullableUnionTypeParameter(Builder $query, null|string|int $bar): Builder + { + return $query->where('foo', $bar); + } + + public function withUnionTypeReturn(): HasMany|UnionTypeModel + { + return $this->hasMany(UnionTypeModel::class); + } + + public function getFooAttribute(): string|int|null + { + return $this->getAttribute('foo'); + } +}