From 28cbfca503cf0b8b4a80a0382ccfdf7edfe94c27 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Mon, 9 Sep 2024 14:01:40 +0200 Subject: [PATCH] Improve TIntermediateModels generics and add inheritDoc (#102) * Relation generics and PHPStan lvl 9 * Resolve TODO * Refactoring * Support PHPStan v1.12.1 * Apply review feedback of @calebdw * Resolve phpstan errors * Apply review feedback * Update tests/Models/Comment.php Co-authored-by: Caleb White * Handle end return type without docblock * Resolve phpstan errors * Update BelongsToThrough.php * Revert PHPStan version constraint after fix * Fix phpstan errors by removing TIntermediateModels * Refactoring * Improve code coverage --------- Co-authored-by: Jonas Staudenmeir Co-authored-by: Caleb White --- src/Relations/BelongsToThrough.php | 76 ++++++++++-------------------- src/Traits/BelongsToThrough.php | 15 +++--- tests/IdeHelper/Models/Comment.php | 2 +- tests/Models/Comment.php | 14 +++--- tests/Models/CustomerAddress.php | 2 +- 5 files changed, 40 insertions(+), 69 deletions(-) diff --git a/src/Relations/BelongsToThrough.php b/src/Relations/BelongsToThrough.php index 87ab3f6..f2b00b3 100644 --- a/src/Relations/BelongsToThrough.php +++ b/src/Relations/BelongsToThrough.php @@ -10,10 +10,10 @@ use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Database\Query\Expression; use Illuminate\Support\Str; +use RuntimeException; /** * @template TRelatedModel of \Illuminate\Database\Eloquent\Model - * @template TIntermediateModel of \Illuminate\Database\Eloquent\Model * @template TDeclaringModel of \Illuminate\Database\Eloquent\Model * * @extends \Illuminate\Database\Eloquent\Relations\Relation @@ -32,7 +32,7 @@ class BelongsToThrough extends Relation /** * The "through" parent model instances. * - * @var TIntermediateModel[] + * @var list<\Illuminate\Database\Eloquent\Model> */ protected $throughParents; @@ -62,7 +62,7 @@ class BelongsToThrough extends Relation * * @param \Illuminate\Database\Eloquent\Builder $query * @param TDeclaringModel $parent - * @param TIntermediateModel[] $throughParents + * @param list<\Illuminate\Database\Eloquent\Model> $throughParents * @param string|null $localKey * @param string $prefix * @param array $foreignKeyLookup @@ -88,11 +88,7 @@ public function __construct( parent::__construct($query, $parent); } - /** - * Set the base constraints on the relation query. - * - * @return void - */ + /** @inheritDoc */ public function addConstraints() { $this->performJoins(); @@ -179,12 +175,7 @@ public function hasSoftDeletes(Model $model) return in_array(SoftDeletes::class, class_uses_recursive($model)); } - /** - * Set the constraints for an eager load of the relation. - * - * @param array $models - * @return void - */ + /** @inheritDoc */ public function addEagerConstraints(array $models) { $keys = $this->getKeys($models, $this->getFirstForeignKeyName()); @@ -192,13 +183,7 @@ public function addEagerConstraints(array $models) $this->query->whereIn($this->getQualifiedFirstLocalKeyName(), $keys); } - /** - * Initialize the relation on a set of models. - * - * @param array $models - * @param string $relation - * @return array - */ + /** @inheritDoc */ public function initRelation(array $models, $relation) { foreach ($models as $model) { @@ -208,14 +193,7 @@ public function initRelation(array $models, $relation) return $models; } - /** - * Match the eagerly loaded results to their parents. - * - * @param array $models - * @param \Illuminate\Database\Eloquent\Collection $results - * @param string $relation - * @return array - */ + /** @inheritDoc */ public function match(array $models, Collection $results, $relation) { $dictionary = $this->buildDictionary($results); @@ -234,8 +212,8 @@ public function match(array $models, Collection $results, $relation) /** * Build model dictionary keyed by the relation's foreign key. * - * @param \Illuminate\Database\Eloquent\Collection $results - * @return \Illuminate\Database\Eloquent\Model[] + * @param \Illuminate\Database\Eloquent\Collection $results + * @return TRelatedModel[] */ protected function buildDictionary(Collection $results) { @@ -275,12 +253,7 @@ public function first($columns = ['*']) return $this->query->first($columns); } - /** - * Execute the query as a "select" statement. - * - * @param string[] $columns - * @return \Illuminate\Database\Eloquent\Collection - */ + /** @inheritDoc */ public function get($columns = ['*']) { $columns = $this->query->getQuery()->columns ? [] : $columns; @@ -296,14 +269,7 @@ public function get($columns = ['*']) return $this->query->get(); } - /** - * Add the constraints for a relationship query. - * - * @param \Illuminate\Database\Eloquent\Builder<\Illuminate\Database\Eloquent\Model> $query - * @param \Illuminate\Database\Eloquent\Builder<\Illuminate\Database\Eloquent\Model> $parentQuery - * @param string[]|mixed $columns - * @return \Illuminate\Database\Eloquent\Builder<\Illuminate\Database\Eloquent\Model> - */ + /** @inheritDoc */ public function getRelationExistenceQuery(Builder $query, Builder $parentQuery, $columns = ['*']) { $this->performJoins($query); @@ -318,7 +284,7 @@ public function getRelationExistenceQuery(Builder $query, Builder $parentQuery, $foreignKey = $from . '.' . $this->getFirstForeignKeyName(); - /** @var \Illuminate\Database\Eloquent\Builder<\Illuminate\Database\Eloquent\Model> $query */ + /** @var \Illuminate\Database\Eloquent\Builder $query */ $query = $query->select($columns)->whereColumn( $this->getQualifiedFirstLocalKeyName(), '=', @@ -358,7 +324,7 @@ public function withTrashed(...$columns) /** * Get the "through" parent model instances. * - * @return TIntermediateModel[] + * @return list<\Illuminate\Database\Eloquent\Model> */ public function getThroughParents() { @@ -372,9 +338,13 @@ public function getThroughParents() */ public function getFirstForeignKeyName() { - /** @var TIntermediateModel $firstThroughParent */ $firstThroughParent = end($this->throughParents); + if ($firstThroughParent === false) { + // @codeCoverageIgnore + throw new RuntimeException('No "through" parent models were specified.'); + } + return $this->prefix . $this->getForeignKeyName($firstThroughParent); } @@ -385,10 +355,14 @@ public function getFirstForeignKeyName() */ public function getQualifiedFirstLocalKeyName() { - /** @var TIntermediateModel $lastThroughParent */ - $lastThroughParent = end($this->throughParents); + $firstThroughParent = end($this->throughParents); + + if ($firstThroughParent === false) { + // @codeCoverageIgnore + throw new RuntimeException('No "through" parent models were specified.'); + } - return $lastThroughParent->qualifyColumn($this->getLocalKeyName($lastThroughParent)); + return $firstThroughParent->qualifyColumn($this->getLocalKeyName($firstThroughParent)); } /** diff --git a/src/Traits/BelongsToThrough.php b/src/Traits/BelongsToThrough.php index d029294..4f9042b 100644 --- a/src/Traits/BelongsToThrough.php +++ b/src/Traits/BelongsToThrough.php @@ -12,15 +12,14 @@ trait BelongsToThrough * Define a belongs-to-through relationship. * * @template TRelatedModel of \Illuminate\Database\Eloquent\Model - * @template TIntermediateModel of \Illuminate\Database\Eloquent\Model * * @param class-string $related - * @param class-string[]|array{0: class-string, 1: string}[]|class-string $through + * @param class-string<\Illuminate\Database\Eloquent\Model>[]|array{0: class-string<\Illuminate\Database\Eloquent\Model>, 1: string}[]|class-string<\Illuminate\Database\Eloquent\Model> $through * @param string|null $localKey * @param string $prefix * @param array, string> $foreignKeyLookup * @param array, string> $localKeyLookup - * @return \Znck\Eloquent\Relations\BelongsToThrough + * @return \Znck\Eloquent\Relations\BelongsToThrough */ public function belongsToThrough( $related, @@ -33,7 +32,7 @@ public function belongsToThrough( /** @var TRelatedModel $relatedInstance */ $relatedInstance = $this->newRelatedInstance($related); - /** @var TIntermediateModel[] $throughParents */ + /** @var list<\Illuminate\Database\Eloquent\Model> $throughParents */ $throughParents = []; $foreignKeys = []; @@ -44,11 +43,10 @@ public function belongsToThrough( /** @var string $foreignKey */ $foreignKey = $model[1]; - /** @var class-string $model */ + /** @var class-string<\Illuminate\Database\Eloquent\Model> $model */ $model = $model[0]; } - /** @var TIntermediateModel $instance */ $instance = $this->belongsToThroughParentInstance($model); if ($foreignKey) { @@ -122,17 +120,16 @@ protected function belongsToThroughParentInstance($model) * Instantiate a new BelongsToThrough relationship. * * @template TRelatedModel of \Illuminate\Database\Eloquent\Model - * @template TIntermediateModel of \Illuminate\Database\Eloquent\Model * @template TDeclaringModel of \Illuminate\Database\Eloquent\Model * * @param \Illuminate\Database\Eloquent\Builder $query * @param TDeclaringModel $parent - * @param TIntermediateModel[] $throughParents + * @param list<\Illuminate\Database\Eloquent\Model> $throughParents * @param string|null $localKey * @param string $prefix * @param array $foreignKeyLookup * @param array $localKeyLookup - * @return \Znck\Eloquent\Relations\BelongsToThrough + * @return \Znck\Eloquent\Relations\BelongsToThrough */ protected function newBelongsToThrough( Builder $query, diff --git a/tests/IdeHelper/Models/Comment.php b/tests/IdeHelper/Models/Comment.php index 2e11638..f53f230 100644 --- a/tests/IdeHelper/Models/Comment.php +++ b/tests/IdeHelper/Models/Comment.php @@ -11,7 +11,7 @@ class Comment extends Model use BelongsToThroughTrait; /** - * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\IdeHelper\Models\Country, \Tests\IdeHelper\Models\User|\Tests\IdeHelper\Models\Post, $this> + * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\IdeHelper\Models\Country, $this> */ public function country(): BelongsToThroughRelation { diff --git a/tests/Models/Comment.php b/tests/Models/Comment.php index 6fadde7..c5934b1 100644 --- a/tests/Models/Comment.php +++ b/tests/Models/Comment.php @@ -15,7 +15,7 @@ class Comment extends Model use HasTableAlias; /** - * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\Country, \Tests\Models\User|\Tests\Models\Post, $this> + * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\Country, $this> */ public function country(): BelongsToThrough { @@ -23,7 +23,7 @@ public function country(): BelongsToThrough } /** - * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\Country, \Tests\Models\User|\Tests\Models\Post, $this> + * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\Country, $this> */ public function countryWithCustomForeignKeys(): BelongsToThrough { @@ -37,7 +37,7 @@ public function countryWithCustomForeignKeys(): BelongsToThrough } /** - * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\Country, \Tests\Models\User|\Tests\Models\Post, $this> + * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\Country, $this> */ public function countryWithTrashedUser(): BelongsToThrough { @@ -46,7 +46,7 @@ public function countryWithTrashedUser(): BelongsToThrough } /** - * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\Country, \Tests\Models\User|\Tests\Models\Post, $this> + * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\Country, $this> */ public function countryWithPrefix(): BelongsToThrough { @@ -54,16 +54,16 @@ public function countryWithPrefix(): BelongsToThrough } /** - * @return \Znck\Eloquent\Relations\BelongsToThrough + * @return \Znck\Eloquent\Relations\BelongsToThrough */ public function grandparent(): BelongsToThrough { - /* @phpstan-ignore argument.templateType, argument.type, return.type */ + /* @phpstan-ignore argument.type */ return $this->belongsToThrough(self::class, self::class.' as alias', null, '', [self::class => 'parent_id']); } /** - * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\User, \Tests\Models\Post, $this> + * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\User, $this> */ public function user(): BelongsToThrough { diff --git a/tests/Models/CustomerAddress.php b/tests/Models/CustomerAddress.php index 43ea989..178d6cb 100644 --- a/tests/Models/CustomerAddress.php +++ b/tests/Models/CustomerAddress.php @@ -7,7 +7,7 @@ class CustomerAddress extends Model { /** - * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\VendorCustomer, \Tests\Models\VendorCustomerAddress, $this> + * @return \Znck\Eloquent\Relations\BelongsToThrough<\Tests\Models\VendorCustomer, $this> */ public function vendorCustomer(): BelongsToThrough {