Eloquent model timestamps are treated as Carbon\Carbon|null

[magnetas]

• Larastan Version: 2.5.1
• --level used: 3
• Pull request with failing test:

Description

Larastan treats/resolves eloquent model timestamps being of Carbon\Carbon|null type instead of Illuminate\Support\Carbon.

Prepared a simple case in a fresh Laravel project. Basically my arrow function says it'll return an Illuminate\Support\Carbon instance and that's the eloquent casted created_at value. Get the following error:

Anonymous function should return Illuminate\Support\Carbon but returns Carbon\Carbon|null

If I check the actual type (i.e. dd($user->created_at)) it in fact is Illuminate\Support\Carbon.

Any clue how this can be sorted out?
Thank you!

Laravel code where the issue was found

namespace App;

use App\Models\User;
use Illuminate\Support\Carbon;

class SomeClass
{
    public function someMethod(): void
    {
        $users = User::get();

        $timestamps = $users->map(fn (User $user): Carbon => $user->created_at);
    }
}

[szepeviktor]

Hellooo @magnetas! 👋🏻
Did you know Carbon has built-in PHPStan extensions?

includes:
    - vendor/nesbot/carbon/extension.neon

[szepeviktor]

...as for this specific case: try returning a non-nullable value.

[magnetas]

Hellooo @magnetas! 👋🏻 Did you know Carbon has built-in PHPStan extensions?

Hi @szepeviktor! First of all thank you for such a prompt reaction!

I wasn't aware of this, but unfortunately it doesn't help with my case - appended it to the includes list and keep getting the same errors.

[szepeviktor]

Maybe we have a 🐞 bug?
https://github.com/nunomaduro/larastan/blob/6327eb91b4256615b0193eb62403d91fc6f2eb98/tests/Type/data/model-properties.php#L60

[magnetas]

...as for this specific case: try returning a non-nullable value.

well...the thing is eloquent model timestamp fields are nullable (i.e. before persisting them they are null. Also DB columns behind them are nullable). Tried hinting my return as ?Carbon (nullable support one) - no luck.

[magnetas]

Maybe we have a 🐞 bug?

https://github.com/nunomaduro/larastan/blob/6327eb91b4256615b0193eb62403d91fc6f2eb98/tests/Type/data/model-properties.php#L60

Yes, this seems to be related. If this assertion passes it'd mean that created_at is nullable Carbon\Carbon instance. However it isn't since Laravel has the wrapper (I guess mostly for mocking).

[magnetas]

@szepeviktor could this be the problem?

https://github.com/nunomaduro/larastan/blob/6327eb91b4256615b0193eb62403d91fc6f2eb98/src/Properties/ModelCastHelper.php#L154-L156

If $dateClass is illuminate carbon then types are unified (illuminate and nestbot carbon). I get a ObjectType instance that has both className and cachedDescription set to "Carbon\Carbon" string.

[szepeviktor]

You can debug it in your function: \PHPStan\dumpType($user->created_at);

[magnetas]

Technically Date::now() is used to set the timestamp values.
And we also have this:

https://github.com/nunomaduro/larastan/blob/6327eb91b4256615b0193eb62403d91fc6f2eb98/tests/Type/data/date-extension.php#L24

which passes as well...

[magnetas]

You can debug it in your function: \PHPStan\dumpType($user->created_at);

Neat!

This is what I get:

 ------ ---------------------------------
  Line   SomeClass.php
 ------ ---------------------------------
  14     Dumped type: Carbon\Carbon|null
 ------ ---------------------------------

[szepeviktor]

@canvural Could you help? I'm lost.

[erikgaal]

@magnetas is right here. The union of \Illuminate\Support\Carbon and \Carbon\Carbon is simplified to \Carbon\Carbon, because one extends the other. When I refactored this code, the union was already there. However, thinking about this now again I'm doubting whether that is the correct behaviour?

https://github.com/nunomaduro/larastan/blob/3e96560fa98c927460e1d0a308a598bc25aaca09/src/Properties/ModelPropertyExtension.php#L151-L153

[magnetas]

Just commented these three lines out in my example project, cleared phpstan cache and ran analysis again - all green.

https://github.com/nunomaduro/larastan/blob/11bdd132565d37e808e4ac985a580b492c2d8887/src/Properties/ModelCastHelper.php#L154-L156

Also dumpType now returns proper value - Illuminate\Support\Carbon|null

Not sure why the union was needed there in the first place. I think what this part of the cast helper does is it determines what type is returned when casting datetimes/timestamps and the union should not be needed.

However it may be needed when defining writable type (since \Carbon\Carbon or even string can be used to set the value.

https://github.com/nunomaduro/larastan/blob/11bdd132565d37e808e4ac985a580b492c2d8887/src/Properties/ModelPropertyExtension.php#L146

Might be missing some context here too. If that's the case then maybe the property extension should do the checks and decide whether additional union is needed...

[magnetas]

Just tried applying the same (commenting out those three lines with union) to actual Laravel 10 project.

I get 18 such errors across the codebase using phpstan level 3.

Skipping the union meant no more errors.

Now I could prepare a PR if this is REALLY how it should behave, but again - I might be missing something