[11.x] Fix fluent syntax for HasManyThrough when combining HasMany followed by HasOne

[jnoordsij]

In #45894 'fluent' syntax for has-many-through and has-one-through relations was added. When combining has-one and has-many, one has 4 options:

1. HasOne followed by HasOne => HasOneThrough
2. HasOne followed by HasMany => HasManyThrough
3. HasMany followed by HasMany => HasManyThrough
4. HasMany followed by HasOne => HasManyThrough

All of these options work fine when using the 'classic' relation definitions. However, the 4th option does not work with the new 'fluent' syntax, as ending with a HasOne makes it incorrectly infer the type of the 'full' relation to be HasOneThrough rather than HasManyThrough.

This PR aims to fix this discrepancy between 'classic' and 'fluent' syntax, by fixing the latter to also allow the 4th option. This is illustrated by adding tests in the first commit that will fail without the fix.

Open issue: the inferred typing on the has method will still incorrectly report HasOneThrough with current changeset.

[github-actions]

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

[jnoordsij]

@calebdw Maybe I can ask you for help here on the typing, given all your (amazing!) work on adding generic types for all this.

Currently the type of the resulting relation is inferred based (solely) on the type of the $callback parameter in the has method:

framework/src/Illuminate/Database/Eloquent/PendingHasThroughRelationship.php

Lines 48 to 57 in f51b643

	* @param string|(callable(TIntermediateModel): (\Illuminate\Database\Eloquent\Relations\HasOne<TRelatedModel, TIntermediateModel>|\Illuminate\Database\Eloquent\Relations\HasMany<TRelatedModel, TIntermediateModel>|\Illuminate\Database\Eloquent\Relations\MorphOneOrMany<TRelatedModel, TIntermediateModel>)) $callback
	* @return (
	* $callback is string
	* ? \Illuminate\Database\Eloquent\Relations\HasManyThrough<\Illuminate\Database\Eloquent\Model, TIntermediateModel, TDeclaringModel>|\Illuminate\Database\Eloquent\Relations\HasOneThrough<\Illuminate\Database\Eloquent\Model, TIntermediateModel, TDeclaringModel>
	* : (
	* $callback is callable(TIntermediateModel): \Illuminate\Database\Eloquent\Relations\HasOne<TRelatedModel, TIntermediateModel>
	* ? \Illuminate\Database\Eloquent\Relations\HasOneThrough<TRelatedModel, TIntermediateModel, TDeclaringModel>
	* : \Illuminate\Database\Eloquent\Relations\HasManyThrough<TRelatedModel, TIntermediateModel, TDeclaringModel>
	* )
	* )

After my change, the resulting type is also influenced by $this->localRelationship which is not generic and therefore I can't access to update the resulting type. You have any idea on how to tackle this?

[calebdw]

After my change, the resulting type is also influenced by $this->localRelationship which is not generic and therefore I can't access to update the resulting type. You have any idea on how to tackle this?

Hmm, you might need to add a new TLocalRelationship template type to the class which should then be usable in the method @return

[calebdw]

You can probably clean up the conditions if you add a Relation template type here:

    /**
     * Create a pending has-many-through or has-one-through relationship.
     *
     * @template TIntermediateModel of \Illuminate\Database\Eloquent\Model
     * @template TLocalRelation of \Illuminate\Database\Eloquent\Relations\HasOneOrMany<TIntermediateModel, covariant $this>
     *
     * @param  string|TLocalRelation  $relationship
     * @return (
     *     $relationship is string
     *     ? \Illuminate\Database\Eloquent\PendingHasThroughRelationship<\Illuminate\Database\Eloquent\Model, $this, ...>
     *     : \Illuminate\Database\Eloquent\PendingHasThroughRelationship<TIntermediateModel, $this, TLocalRelation>
     * )
     */

[jnoordsij]

Wow, thanks for your quick feedback!

I must admit this was my first idea as well, but somehow I couldn't get it to play right. Using this I was faced with:

 ------ ------------------------------------------------------------------------------------------------------------------------------
  Line   Relations.php
 ------ ------------------------------------------------------------------------------------------------------------------------------
  195    Expected type
         Illuminate\Database\Eloquent\PendingHasThroughRelationship<Illuminate\Types\Relations\Mechanic,
         $this(Illuminate\Types\Relations\User),
         Illuminate\Database\Eloquent\Relations\HasOne<Illuminate\Types\Relations\Mechanic,
         $this(Illuminate\Types\Relations\User)>>, actual:
         Illuminate\Database\Eloquent\PendingHasThroughRelationship<Illuminate\Types\Relations\Mechanic,
         $this(Illuminate\Types\Relations\User),
         Illuminate\Database\Eloquent\Relations\HasOneOrMany<Illuminate\Types\Relations\Mechanic, covariant
         $this(Illuminate\Types\Relations\User)>>
  199    Expected type Illuminate\Database\Eloquent\Relations\HasOneThrough<Illuminate\Types\Relations\Car,
         Illuminate\Types\Relations\Mechanic, $this(Illuminate\Types\Relations\User)>, actual:
         Illuminate\Database\Eloquent\Relations\HasManyThrough<Illuminate\Types\Relations\Car,
         Illuminate\Types\Relations\Mechanic,
         $this(Illuminate\Types\Relations\User)>|Illuminate\Database\Eloquent\Relations\HasOneThrough<Illuminate\Types\Relations\Car,
         Illuminate\Types\Relations\Mechanic, $this(Illuminate\Types\Relations\User)>
  215    Expected type
         Illuminate\Database\Eloquent\PendingHasThroughRelationship<Illuminate\Types\Relations\Mechanic,
         $this(Illuminate\Types\Relations\User),
         Illuminate\Database\Eloquent\Relations\HasMany<Illuminate\Types\Relations\Mechanic,
         $this(Illuminate\Types\Relations\User)>>, actual:
         Illuminate\Database\Eloquent\PendingHasThroughRelationship<Illuminate\Types\Relations\Mechanic,
         $this(Illuminate\Types\Relations\User),
         Illuminate\Database\Eloquent\Relations\HasOneOrMany<Illuminate\Types\Relations\Mechanic, covariant
         $this(Illuminate\Types\Relations\User)>>
  224    Expected type Illuminate\Database\Eloquent\Relations\HasManyThrough<Illuminate\Types\Relations\Car,
         Illuminate\Types\Relations\Mechanic, $this(Illuminate\Types\Relations\User)>, actual:
         Illuminate\Database\Eloquent\Relations\HasManyThrough<Illuminate\Types\Relations\Car,
         Illuminate\Types\Relations\Mechanic,
         $this(Illuminate\Types\Relations\User)>|Illuminate\Database\Eloquent\Relations\HasOneThrough<Illuminate\Types\Relations\Car,
         Illuminate\Types\Relations\Mechanic, $this(Illuminate\Types\Relations\User)>
 ------ ------------------------------------------------------------------------------------------------------------------------------

This seems to suggest that somehow TLocalRelation is not properly inherited as either HasOne or HasMany, but keeps using HasOneOrMany which is kind of unfortunate.

[calebdw]

I'm not sure if you need all three templates---you might can do with just the relation template given that both the model types can be gleaned from the relation type

[jnoordsij]

Is there some canonical way to do this? Last time I dived into this I was told there to be no trivial way to do such a thing; see phpstan/phpstan#8814.

[canvural]

Adding a new generic type to a class should've been considered a BC break from type system POV. But I guess Laravel doesn't care about that.