Larastan: PHPStan 0.12.82 compatibility

[canvural]

In PHPStan v0.12.82 release there were changes related to how static keyword is resolved. This caused some issues in Larastan. Need to investigate and fix.

[szepeviktor]

Was it this PR? https://github.com/phpstan/phpstan-src/pull/474

[canvural]

Was it this PR? phpstan/phpstan-src#474

Probably.

[shaffe-fr]

In my project, most of the errors introduced by PHPStan 0.12.82 can be summed up to the following snippet:

$this->userOrNull = $office->user()->active()->first();

• Model scopes Builder parameter is not ignored
  Method Illuminate\Database\Query\Builder::active() invoked with 0 parameters, at least 1 required.
• Model type is lost on relations Builder: Property \MyClass::$userOrNull (App\User|null) does not accept Illuminate\Database\Eloquent\Model|null.

Hope it helps.

[canvural]

@shaffe-fr Thank you for the information!

I also changed the master branch so we can see which tests are failing. Here is the failing tests from the actions.

[canvural]

I spent a little bit of time on this. But couldn't come up with a good solution. So if anyone else is interested, we can collaborate on a solution!

[szepeviktor]

Should I dare to mention Ondřej Mirtes? What if he will points out bad design in Larastan??

[szepeviktor]

First one being

Method Tests\\Features\\Methods\\ModelExtension::testLockForUpdate() should return
Illuminate\\Database\\Eloquent\\Builder<App\\User> but returns
Illuminate\\Database\\Eloquent\\Builder<Illuminate\\Database\\Eloquent\\Model>.

in

https://github.com/nunomaduro/larastan/blob/f74befd87b7278285cbf6f1597509cbfaca46bf8/tests/Features/Methods/ModelExtension.php#L284-L288

[ondrejmirtes]

Don't hesitate to reach out to me as I'm probably the only person that can help you here :) it would be great if you could isolate what Larastan extensions started behaving differently between PHPStan 0.12.81 and 0.12.82 then we can move on to a solution.

[ondrejmirtes]

One side-effect of 0.12.82 changes is that generics + MethodsClassReflectionExtension/PropertiesClassReflectionExtension behave a bit differently. If you introduce new methods based on a template type variable (like @template T), the extension isn't going to work unless you introduce the object (or more specific) bound - @template T of object.

But of course the problem can be something completely different. For example if you have some custom types that override getProperty or getMethod, it now looks differently: https://github.com/phpstan/phpstan-src/blob/ed1736c045a29da4673ccbbbce7e9305841b00f9/src/Type/ObjectType.php#L512-L551

[canvural]

I didn't have much time to really dive into it. But from a short look previously, the problem appears to be like this:

• In Builder we have template like this
• Then in EloquentBuilderForwardsCallsExtension when we get the TModelClass from the active template type map sometimes it's a child model (\App\User for example) and sometimes it's the \Illuminate\Database\Eloquent\Model (the template bound)

And when it's the latter, we get the errors like should return Illuminate\\Database\\Eloquent\\Builder<App\\User> but returns Illuminate\\Database\\Eloquent\\Builder<Illuminate\\Database\\Eloquent\\Model>.

So maybe the solution can be as simple as checking if TModelClass is not Illuminate\Database\Eloquent\Model after this line? Didn't try this yet, so don't know if it can cause other issues.

[szepeviktor]

So maybe the solution can be as simple as checking if TModelClass is not Illuminate\Database\Eloquent\Model after this line

Go for it!

[ondrejmirtes]

So maybe the solution can be as simple as checking if TModelClass is not Illuminate\Database\Eloquent\Model after this line?

Yeah, try to return null in that case.