Skip to content

Commit

Permalink
fix: respect ancestor property tags in model extensions (#1037)
Browse files Browse the repository at this point in the history
  • Loading branch information
spawnia committed Nov 19, 2021
1 parent 19098c7 commit 50ef079
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Fixed
* No return type should be enforced for closure of tap helper
* Ensure the model extensions considers PHPDoc `@property` tags from ancestors, not just the model class itself

### Changed
* Improved return type for Collection::first, last, get, pull when giving a default value.
Expand Down
5 changes: 3 additions & 2 deletions src/Properties/ModelPropertyExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use ArrayObject;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Support\Str;
use NunoMaduro\Larastan\Reflection\ReflectionHelper;
use PHPStan\PhpDoc\TypeStringResolver;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\PropertiesClassReflectionExtension;
Expand All @@ -19,7 +20,7 @@
*/
final class ModelPropertyExtension implements PropertiesClassReflectionExtension
{
/** @var SchemaTable[] */
/** @var array<string, SchemaTable> */
private $tables = [];

/** @var TypeStringResolver */
Expand Down Expand Up @@ -51,7 +52,7 @@ public function hasProperty(ClassReflection $classReflection, string $propertyNa
return false;
}

if (array_key_exists($propertyName, $classReflection->getPropertyTags())) {
if (ReflectionHelper::hasPropertyTag($classReflection, $propertyName)) {
return false;
}

Expand Down
3 changes: 2 additions & 1 deletion src/Properties/ModelRelationsExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Illuminate\Support\Str;
use NunoMaduro\Larastan\Concerns;
use NunoMaduro\Larastan\Methods\BuilderHelper;
use NunoMaduro\Larastan\Reflection\ReflectionHelper;
use NunoMaduro\Larastan\Types\RelationParserHelper;
use PHPStan\Analyser\OutOfClassScope;
use PHPStan\Reflection\ClassReflection;
Expand Down Expand Up @@ -49,7 +50,7 @@ public function hasProperty(ClassReflection $classReflection, string $propertyNa
return false;
}

if (array_key_exists($propertyName, $classReflection->getPropertyTags())) {
if (ReflectionHelper::hasPropertyTag($classReflection, $propertyName)) {
return false;
}

Expand Down
28 changes: 28 additions & 0 deletions src/Reflection/ReflectionHelper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace NunoMaduro\Larastan\Reflection;

use PHPStan\Reflection\ClassReflection;

final class ReflectionHelper
{
/**
* Does the given class or any of its ancestors have an `@property*` annotation with the given name?
*/
public static function hasPropertyTag(ClassReflection $classReflection, string $propertyName): bool
{
if (array_key_exists($propertyName, $classReflection->getPropertyTags())) {
return true;
}

foreach ($classReflection->getAncestors() as $ancestor) {
if (array_key_exists($propertyName, $ancestor->getPropertyTags())) {
return true;
}
}

return false;
}
}
54 changes: 54 additions & 0 deletions tests/Features/Models/Relations.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Database\Eloquent\Relations\HasOne;
use Illuminate\Database\Eloquent\Relations\MorphTo;
use Illuminate\Database\Eloquent\Relations\MorphToMany;
use function PHPStan\Testing\assertType;
Expand Down Expand Up @@ -232,6 +233,26 @@ public function testBelongsToManyCreateReturnsCorrectModel(User $user): Post

return $user->posts()->create();
}

public function testNullableUser(ExtendsModelWithPropertyAnnotations $model): bool
{
return $model->nullableUser === null;
}

public function testNonNullableUser(ExtendsModelWithPropertyAnnotations $model): User
{
return $model->nonNullableUser;
}

public function testNullableFoo(ExtendsModelWithPropertyAnnotations $model): bool
{
return $model->nullableFoo === null;
}

public function testNonNullableFoo(ExtendsModelWithPropertyAnnotations $model): string
{
return $model->nonNullableFoo;
}
}

/**
Expand All @@ -258,6 +279,39 @@ public function relation(): HasMany
}
}

/**
* @property-read User|null $nullableUser
* @property-read User $nonNullableUser
* @property-read string|null $nullableFoo
* @property-read string $nonNullableFoo
*/
class ModelWithPropertyAnnotations extends Model
{
public function nullableUser(): HasOne
{
return $this->hasOne(User::class);
}

public function nonNullableUser(): HasOne
{
return $this->hasOne(User::class);
}

public function getNullableFooAttribute(): ?string
{
return rand() ? 'foo' : null;
}

public function getNonNullableFooAttribute(): string
{
return 'foo';
}
}

class ExtendsModelWithPropertyAnnotations extends ModelWithPropertyAnnotations
{
}

class Tag extends Model
{
/**
Expand Down

0 comments on commit 50ef079

Please sign in to comment.