Support custom eloquent collections

[timacdonald]

Fixes: #436

This PR brings support for custom eloquent collections.

BuilderHelper::determineCollectionType

Now that this PR is done, I'm not entirely sure determineCollectionType should be on the builder helper. It was suggested this could be its home, however maybe it should be extracted to a Collection helper as it isn't really just about the builder? Maybe it doesn't matter.

See method

Fixed find...() null return type

If any find...() methods receive an iterable, they will never return null. They always proxy to findMany which in all cases returns a collection.

See here and here

Fixed fromQuery return type

Builder::fromQuery was included in some existing type checks, but as that method always returns a collection I removed it.

See removal

Doesn't handle factory helper

Due to the dynamic nature of the factory helper method and the builder, I was unable to get the correct return type for factory calls. Not sure if this needs to be included in this PR or if we can follow up with another PR to add this functionality. Here is some examples of what we'd have to handle...

// should return custom class collection...
factory(Account::class, 1)->create();
factory(Account::class, 'name', 1)->create();
factory(Account::class)->times(1)->create();
factory(Account::class)->times(null)->times(1)->create();


// should return a single instance...
factory(Account::class)->create();
factory(Account::class, 'name')->create();
factory(Account::class)->times(null)->create();
factory(Account::class)->times(1)->times(null)->create();

See factory helper

Doesn't handle MorphTo::getResultsByType

This method accepts a string that could be either a class or a morph map key. Without booting the framework and retrieving the class from the Morph Map there is not way (that I'm aware) to determine the class, and therefore the collection type.

It is a protected method, and I'd guess this method isn't really used outside the framework anyway, however I've got no data to back that up.

Relation::morphMap([
    'videos' => 'App\Video',
]);

$morphTo->getResultsByType('videos');

Doesn't handle instance proxying by design

All methods that are accessible via static calls on the model are also available via instance calls. Looks like Larastan doesn't currently support that for other methods, so I didn't add support for this feature, but I could if needed.

$instance->find($id);
$instance->hydrate($records);

See method

[szepeviktor]

Please un-draft this PR when you are ready!

[timacdonald]

Will do!

[canvural]

Hi, thanks for taking a stab at this! I added some comments, for how to make it in an easier way.

Generally, it should be similar to how custom Eloquent builder stuff is implemented. Feel free to take look at the implementation of that. And please feel free to ask any questions you might have about the implementation, Larastan or PHPStan.

[timacdonald]

Thanks so much for all this feedback @canvural - much appreciated getting pointed in the right direction. Making some more progress on this today.

[szepeviktor]

👍 👍

[szepeviktor]

@timacdonald You are trying very hard.
Thank you.

[timacdonald]

Haha, that I am. This is a whole new world and I needed a few goes at it to get my head around it all and in the right frame of mind. Now I at least know how it all (kinda) comes together. Hoping to have it wrapped up next weekend 😀

[canvural]

Thank you very much for this! Looks great!

however maybe it should be extracted to a Collection helper as it isn't really just about the builder? Maybe it doesn't matter.

Yeah, that might make more sense maybe. But we don't have any other logic related to collections right now, that can also be extracted to a helper. So, right now leaving it in BuilderHelper is fine.

If any find...() methods receive an iterable, they will never return null. They always proxy to findMany which in all cases returns a collection.

Yeah, good catch! I really don't remember why I did it like that 😅

Due to the dynamic nature of the factory helper method and the builder, I was unable to get the correct return type for factory calls. Not sure if this needs to be included in this PR or if we can follow up with another PR to add this functionality. Here is some examples of what we'd have to handle...

Yeah, we can do a separate PR for this. I'm not sure if we can support times method calls, but we can definitely do something for factory(User::class, 5)->create() calls.

[szepeviktor]

@timacdonald Success! 🍏

[timacdonald]

Thanks @canvural and @szepeviktor! Glad to have been able to help out on the project. While my head is still in the headspace, I'd love to get the factory helper stuff done.

Any pointers on where to start with supporting factory(User::class, 5)->create()?

All my current work has just relied on what was passed into the final method, however this would require looking back and seeing what was called before the create method.

[mr-feek]

@timacdonald you can solve model factories very simply via templates. Here's an implementation i did over at psalm https://github.com/mr-feek/psalm-plugin-laravel/pull/1/files