Skip to content
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

Generic parent methods erroneously treated as private #1952

Closed
treed-eaglemetal opened this issue May 20, 2024 · 0 comments · Fixed by #1953
Closed

Generic parent methods erroneously treated as private #1952

treed-eaglemetal opened this issue May 20, 2024 · 0 comments · Fixed by #1953
Labels
bug Something isn't working

Comments

@treed-eaglemetal
Copy link

  • Larastan Version: 2.9.6
  • Laravel Version: 11.7.0

The Surface Problem

I've created an example project to replicate this issue. In short, I'm faced with the following error:

Call to private method orderBy() of parent class App\Builders\UserBuilder.

My UserBuilder class extends Illuminate\Database\Eloquent\Builder, where the orderBy method is sourced from the Illuminate\Database\Query\Builder class via mixin. Anyone familiar with Laravel will know that the orderBy method is not private.

The code in question being blamed is also seemingly harmless:

public function index(Request $request): View
{
    $users = PortalUser::query()
        ->whereType($request->type)
        ->orderBy('name') // <-- Blame
        ->get();

    return view('users.index', compact('users'));
}

Without knowing the setup behind the scenes to cause this, the error looks like a fluke. This is why I took my working problem, and created the example project to ensure the issue isn't related to my specific setup.

The Deeper Problem

This happens when generics are involved in a particular manner. To simplify the example project even further, here are the docblocks for the involved classes:

Larastan Eloquent Stub:

/**
 * @template TModelClass of Model
 * @property-read static $orWhere
 */
class Builder

Example Parent Builder:

/** @extends Builder<User> */
class UserBuilder extends Builder

Example Child Builder:

class PortalUserBuilder extends UserBuilder

Note that the child builder has no generics. The parent builder references generics, but is not itself generic.

When this specific combination is in play, any method call to the underlying Database Query Builder instance is erroneously treated as private.

The Likely Culprit

After digging around through Larastan's source code, I came across this:

// Generic type is not specified
if ($modelType === null) {
    if (! $classReflection->isGeneric() && $classReflection->getParentClass()?->isGeneric()) {
        $modelType = $classReflection->getParentClass()->getActiveTemplateTypeMap()->getType('TModelClass');
    }
}

If I'm reading this right, when the class itself is not generic, and the parent itself isn't generic either, the $modelType stays null. However, had this code checked the grandparent, it would have found Illuminate\Database\Eloquent\Model (as the grandparent is the Eloquent Builder class).

I'm not saying that this code exactly is to blame, but this reads like Larastan expects custom eloquent builders to be a direct descendant of a generic eloquent builder. In the example scenario I've laid out, that isn't true, and I start getting nonsensical output (such as public methods being treated as private).

The Workaround

I can definitely work around this bug by passing the generics through the parent builder, such that the child builder must reference the generics. This works, but has limitations.

In my actual scenario at work, the parent builder is defined within a package, and is therefore not open for modification. When my hands are tied in this manner, I'm left with either grandiose modifications of a package, or ignoring the private method error.

I would prefer that Larastan understand this sort of situation. While niche, it does happen.

@treed-eaglemetal treed-eaglemetal added the bug Something isn't working label May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant