Larastan: Static method on model has return type signature replaced incorrectly when containing a collection

Created on 1 Jan 2021  路  6Comments  路  Source: nunomaduro/larastan

  • Larastan Version: v0.6.11
  • --level used: 6

Description

Defining a static method on a Model that contains a reference to an Eloquent Collection in its return type signature results in a completely different return type being parsed.

Laravel code where the issue was found

I have a legacy method that has a slightly awkward return type that can provide either a collection of a specific model, or just a single model if only one is available:

use Illuminate\Database\Eloquent\Collection as DatabaseCollection;

class User extends Model
{
    /**
     * @return DatabaseCollection<Order>|Order
     */
    public static function testFunctionWithCollectionInReturnType()
    {
        return new DatabaseCollection([]);
    }
}

$orders = User::testFunctionWithCollectionInReturnType();
foreach ($orders as $order) {
    $order->some_property_belonging_to_orders;
}

It seems that the return type is parsed as DatabaseCollection<User> which loses the union type and the intended type.
As such an access to an undefined property is raised.

I'm not familiar with the internals of phpstan/larastan or I would also work on a PR to fix, but if someone can point me in the right direction, I am happy to take a stab at it. After a little debugging, it seems the return type signature is replaced here:
https://github.com/nunomaduro/larastan/blob/a036948cdcb09bc374f8be77db4c1262374cb297/src/ReturnTypes/ModelExtension.php#L89-L93

bug

All 6 comments

Hello,

This looks like a bug. Will take a look.

Hi,

Sorry for the delay. I tried to reproduce this, but it works correctly for me.

Added this to User model:

/**
 * @return Collection<Post>|Post
 */
public function collectionOrModel() // tried also with static method
{
    if (random_int(0, 1) === 0) {
        return new Collection([new Post()]);
    }

    return new Post();
}

And in another class I have this test method:

public function testMethodReturningCollectionAndModelInUnion(User $user)
{
    $collectionOrModel = $user->collectionOrModel();

    \PHPStan\dumpType($collectionOrModel);
}

And the result is Dumped type: App\Post|Illuminate\Database\Eloquent\Collection<App\Post>

This seems correct to me.

In your original example can you add PHPStan\dumpType($orders) just before the foreach and tell me the result?

Hi, sorry for the late response.

In your original example can you add PHPStan\dumpType($orders) just before the foreach and tell me the result?

Thanks for this, did not know it existed. Very handy. In my original project:

Dumped type: Illuminate\Database\Eloquent\Collection<App\User>

That's with my original return type of:

EloquentCollection<Order>|Order

I've also got a minimal repro setup to demonstrate the issue I'm experiencing. You can see in the action result the failure + dumped type. I set this up quickly but I think it should accurately reflect the issue.

The methods must be static to trigger this extension:
https://github.com/nunomaduro/larastan/blob/ca11c1e598b7a11ff81b6a2c110acea61b96f6ca/src/ReturnTypes/ModelExtension.php#L70-L74

Just curious if there is any issue with https://github.com/nunomaduro/larastan/pull/751 that is preventing it getting merged? Is there another way to resolve this bug?

@Josh-G No, there isn't.
Please resolve the conflict and it has green lights 馃崗

Ah sorry, hadn't noticed there were conflicts

Was this page helpful?
0 / 5 - 0 ratings