Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix broken ReflectionUnionTypes #1132

Merged
merged 12 commits into from
Jan 11, 2021
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,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)

Expand Down
60 changes: 44 additions & 16 deletions src/Console/ModelsCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@
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;
Expand Down Expand Up @@ -576,7 +578,7 @@ protected function getPropertiesFromMethods($model)
$reflection = new \ReflectionMethod($model, $method);

if ($returnType = $reflection->getReturnType()) {
$type = $returnType instanceof \ReflectionNamedType
$type = $returnType instanceof ReflectionNamedType
? $returnType->getName()
: (string)$returnType;
} else {
Expand Down Expand Up @@ -1011,16 +1013,12 @@ protected function getReturnTypeFromReflection(\ReflectionMethod $reflection): ?
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;
Expand Down Expand Up @@ -1186,17 +1184,19 @@ protected function writeModelExternalBuilderMethods(Model $model): void
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()) {
return '?' . $parameterName;
if($paramType->allowsNull()){
if(count($types)==1){
$type = '?' . $type;
}else{
$type .='|null';
}
}

return $parameterName;
return $type;
}

$docComment = $method->getDocComment();
Expand Down Expand Up @@ -1261,4 +1261,32 @@ protected function getParamType(\ReflectionMethod $method, \ReflectionParameter
// 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;
}
}
32 changes: 32 additions & 0 deletions tests/Console/ModelsCommand/UnionTypes/Models/UnionTypeModel.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

declare(strict_types=1);

namespace Barryvdh\LaravelIdeHelper\Tests\Console\ModelsCommand\UnionTypes\Models;

use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Database\Query\Builder;

class UnionTypeModel extends Model
{
public function scopeWithUnionTypeParameter(Builder $query, string|int $bar): Builder
{
return $query->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');
}
}
34 changes: 34 additions & 0 deletions tests/Console/ModelsCommand/UnionTypes/Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

declare(strict_types=1);

namespace Barryvdh\LaravelIdeHelper\Tests\Console\ModelsCommand\UnionTypes;

use Barryvdh\LaravelIdeHelper\Console\ModelsCommand;
use Barryvdh\LaravelIdeHelper\Tests\Console\ModelsCommand\AbstractModelsCommand;
use Illuminate\Foundation\Application;

class Test extends AbstractModelsCommand
{
protected function setUp(): void
{
parent::setUp();

if (PHP_VERSION_ID < 80000) {
$this->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();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

declare(strict_types=1);

namespace Barryvdh\LaravelIdeHelper\Tests\Console\ModelsCommand\UnionTypes\Models;

use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Database\Query\Builder;

/**
* Barryvdh\LaravelIdeHelper\Tests\Console\ModelsCommand\UnionTypes\Models\UnionTypeModel
*
* @property-read string|int|null $foo
* @property-read \Illuminate\Database\Eloquent\Collection|UnionTypeModel[] $withUnionTypeReturn
* @property-read int|null $with_union_type_return_count
* @method static \Illuminate\Database\Eloquent\Builder|UnionTypeModel newModelQuery()
* @method static \Illuminate\Database\Eloquent\Builder|UnionTypeModel newQuery()
* @method static \Illuminate\Database\Eloquent\Builder|UnionTypeModel query()
* @method static \Illuminate\Database\Eloquent\Builder|UnionTypeModel withNullableUnionTypeParameter(string|int|null $bar)
* @method static \Illuminate\Database\Eloquent\Builder|UnionTypeModel withUnionTypeParameter(string|int $bar)
* @mixin \Eloquent
*/
class UnionTypeModel extends Model
{
public function scopeWithUnionTypeParameter(Builder $query, string|int $bar): Builder
{
return $query->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');
}
}