Weebly / phpstan-laravel

Laravel plugins for PHPStan
BSD 2-Clause "Simplified" License
51 stars 22 forks source link

Not so good to have Builder method in sub class of Model #18

Closed kamicloud closed 5 years ago

kamicloud commented 6 years ago

In BuilderMethodExtension::hasMethod, if a class is sub class of Model, when calling Builder method, hasMethod will return true. But will cause following code be determined as a non-error code.

             $query = Topic::where('id', 1);
             $id = $query->id;

The code will throw "Undefined property: Illuminate\Database\Eloquent\Builder::$id"

It is caused by function in Builder, return static::xxxx(), $query was determined as an instance of Topic.

I think it is better to force return type Builder when calling where/orderBy/groupBy and etc. And implements DynamicMethodReturnTypeExtension, DynamicStaticMethodReturnTypeExtension for find/get/first and etc.

I did that in https://github.com/Ttdnts/phpstan-laravel/blob/newBuilder/src/BuilderMethodExtension.php and https://github.com/Ttdnts/phpstan-laravel/blob/newBuilder/src/Types/ModelFindReturnType.php

Result: 25 Access to an undefined property Illuminate\Database\Eloquent\Builder::$id.

kamicloud commented 6 years ago

Maybe add a method 'isInstanceMethod' in \PHPStan\Reflection\Php\PhpMethodReflection in phpstan if return false, can not be called by $instance->notInstanceMethod(); so Model has staticMethod where/orderBy.. allowed: Topic::where('id', 1); Topic::orderBy('id', 'desc'); disallowed: $topic = new Topic(); $topic->where('id', 1);