-
-
Notifications
You must be signed in to change notification settings - Fork 401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Method calls on custom query builder to public parent methods are wrongly reported as private #1289
Comments
🧪 Hello Matthias! 👋🏻 |
First, there is no union type. I assume you mean intersection type. However, this is not the problem which this issue is about. The problem is the first reported error by Larastan as shown in the issue description. |
Yes. I never use the name, only |
I think Larastan does not support this much abstraction. |
Try going back to this simplicity, this is supported. |
This is exactly as in the the example. The template: use Illuminate\Database\Eloquent\Builder;
/**
* @template TModelClass of \Illuminate\Database\Eloquent\Model
* @extends Builder<TModelClass>
*/
class CustomBuilder extends Builder
{
} My example: use App\Models\Contracts\Node;
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
use Illuminate\Database\Eloquent\Model;
/**
* @template TNodeModel of Model&Node
* @phpstan-extends EloquentBuilder<TNodeModel>
*/
class NodeBuilder extends EloquentBuilder
{
} Except for difference in names it is exactly the same. My gut feeling tells me that the problem is related to the trait. The template directly defines the methods like
This is unfortunately impossible, because there is a reason for |
For a moment please try to move the trait to everywhere it is used. |
That is not going to happen. Rather we migrate the whole project away from Laravel to Symfony. Not only due to this problem at hand, but because we had a long history of fuck-ups due to Laravel's stupid architecture. (Yes, I am aware that Laravel and Larastan are two different projects.) In the past I used Symfony for other project which had a much more complex DB model than the current Laravel project and I never ever encountered any of the many problem we already had with Laravel despite the fact that the current project is much simpler. (I don't want to know what would have happened, if we had tried to realize the other projects in Laravel.) So before I start duplicating code and create a maintainability hell for the future, I rather take the effort to migrate the whole project. |
AFAIK Laravel filled the gap between existing childish "frameworks" and Symfony. |
BTW, I wonder why uses The line dates back to commit af4094a when the whole If this is indeed the cause for the bug, then it does not solely affect this line but all other lines in |
I updated my example at https://github.com/nagmat84/larastan-example. It is now even more trivial and does not use any interface or any intersection type. But the errors are still there. IMHO this needs to be fixed, because it is a very typical pattern that models are augmented via traits. |
See the results online: https://github.com/nagmat84/larastan-example/actions/runs/2578269170 |
Hi, I cannot reproduce About the other errors;
|
It is intentional and has nothing to do with the errors you got. |
Sorry, for that. I pushed an even smaller MWE after @szepeviktor suggested in his post that the intersection types might be the problem (even though he wrongly called them union types). In doing the original error changed from "call to private method" into "call to undefined method". I followed your advices (see below) and also reverted the example repo to the full example (note that the default branch has changed).
Great. This did not only solve the problem "call to undefined method" for the branch I don't understand why one must not rename the template parameter, but I can live with that restriction. IMHO, there should be a bold, eye-catching warning in the documentation that one must not rename inherited template parameters.
Yes, this is the only error which also remains for the branch |
Fixed in #1299 |
I can confirm that #1299 fixed it, see here https://github.com/nagmat84/larastan-example/runs/7161581815?check_suite_focus=true. I will now use what I have learned (i.e. that you must not rename the template parameter!) and apply it to our actual code. May be there are still other problems, but that another issue then. Is there any chance that #1299 gets back-ported to Larastan 1.x. Our project is still stuck at Laravel 8 due to various reasons. |
Unfortunately, the problem is back again in #1285 after I have added a test class for custom relations. |
--level
used: 7Description
If one creates a trait to equip a model with a custom query builder, then methods calls on the custom query builder which are defined in
Illuminate\Database\Query\Builder
are wrongly reported asprivate
.Laravel code where the issue was found
I prepared the following GITHub repository with a minimal Laravel application: https://github.com/nagmat84/larastan-example. Step to reproduce the problem:
composer install
composer phpstan
You will get the following output
However the method is not private.
The text was updated successfully, but these errors were encountered: