Skip to content

Scopes/forwarded Builder methods on abstract Model base classes are not resolved #901

@alies-dev

Description

@alies-dev

Summary

When a call site is typed as an abstract Eloquent Model base class, the plugin doesn't resolve scope methods (scopeFoofoo()), trait-declared Builder methods, or Query\Builder forwarded methods. ModelRegistrationHandler::afterCodebasePopulated() skips classes whose $storage->abstract is true, so ModelMethodHandler::doesMethodExist() / getReturnTypeForForwardedMethod() / getMethodParams() are never registered for the abstract class. Concrete subclasses are unaffected.

This causes UndefinedMagicMethod on any code that calls scopes/forwarded methods through an abstract-typed reference, regardless of whether the call site is static or instance.

Reproduction

Found in BookStackApp/BookStack (commit 82ef735, benchmark snapshot v4.10.0).

Definition site

app/Entities/Models/Entity.php:55

abstract class Entity extends Model implements ...
{
    /**
     * Get the entities that are visible to the current user.
     */
    public function scopeVisible(Builder $query): Builder
    {
        return app()->make(PermissionApplicator::class)->restrictEntityQuery($query);
    }
}

API boundary returning the abstract type

app/Entities/EntityProvider.php:55

public function get(string $type): Entity { ... }

Call site

app/Entities/Tools/SiblingFetcher.php:26

$entity = (new EntityProvider())->get($entityType)->visible()->findOrFail($entityId);

Psalm output:

ERROR (UndefinedMagicMethod): Magic method BookStack\Entities\Models\Entity::visible does not exist

The same pattern fails for Entity::visible() (static) and (new EntityProvider())->get(\$type)::visible() because the abstract class never gets its closures registered.

Root cause

src/Handlers/Eloquent/ModelRegistrationHandler.php:58-60

foreach (\$codebase->classlike_storage_provider::getAll() as \$storage) {
    if (\$storage->abstract) {
        continue;
    }
    ...
}

The skip predates the unified ModelMethodHandler and was reasonable when only concrete-model state (table, casts, fillable) was registered. Now that scope/forward resolution is also wired per-class in this loop, the skip excludes abstract bases from the resolver chain entirely.

Other registrations affected by the same skip (less critical, but worth confirming during the fix):

  • getMethodParams for forwarded calls on the abstract type
  • isMethodVisible (visibility check for __callStatic-forwarded methods)
  • Custom-builder method/param providers when the abstract declares newEloquentBuilder()

Expected behaviour

  • Entity::visible() resolves to Builder<Entity> (or Builder<static>).
  • \$entityInstance->visible() resolves the same way (Laravel's Model::__call forwards to \$this->newQuery()).
  • EntityProvider::get(\$type)->visible()->findOrFail() types as Entity.

This matches the existing behaviour for concrete Model subclasses with scope* methods (covered by closed #498).

Why not just "don't type returns as the abstract base"

The abstract base is the legitimate API contract in patterns like:

  • Polymorphic registries returning the common base (BookStack EntityProvider)
  • Factory methods returning the abstract type
  • Relation::getRelated() on polymorphic relationships
  • Repositories that operate on the abstract base

These are common in larger Laravel codebases; pushing every consumer to narrow at the call site is impractical.

Proposed fix

Remove or scope the \$storage->abstract early-exit in ModelRegistrationHandler::afterCodebasePopulated():

  1. Don't skip — process abstract classes the same way as concrete ones for the resolver registrations (scope method existence, forwarded-call return types, param providers, custom builder discovery).
  2. Skip only the per-instance work that genuinely needs a concrete class: schema reflection via new \$class(), getTable(), getCasts(), getKeyName() (these would fail for abstract classes). Guard those specific reads behind !\$storage->abstract rather than skipping the whole iteration.
  3. Keep the class_exists() force-load (line 82) — abstract classes still need to be loadable for ReflectionClass to find scope methods.

Rough sketch:

foreach (\$codebase->classlike_storage_provider::getAll() as \$storage) {
    if (!isset(\$storage->parent_classes[\$modelFqcn])) {
        continue;
    }
    if (\$storage->stmt_location !== null
        && AnonymousClassNameDetector::isSynthetic(\$storage->name, \$storage->stmt_location->file_path)
    ) {
        continue;
    }
    // ... force-load class ...

    // Always register scope/forwarded-method closures (works for abstract bases too)
    self::registerMethodResolvers(\$storage->name);

    if (\$storage->abstract) {
        continue; // Skip metadata work that requires instantiation
    }

    self::populateModelMetadata(\$storage->name);
}

Tests to add

tests/Type/tests/Builder/AbstractModelScopeTest.phpt covering:

  • scopeFoo() declared on abstract base, called on abstract-typed instance and static class reference
  • scopeFoo() declared on abstract base, called on concrete subclass instance/static (regression)
  • scopeFoo() declared on concrete subclass, abstract base reference falls back cleanly (no false positive)
  • Method param resolution for both legacy scope* and #[Scope] attribute forms
  • Builder<AbstractBase> chain returning correct generic

tests/Application/ regression: add an abstract model + provider returning the abstract type + scope call, mirroring the BookStack pattern.

Impact estimate

Spot-check across the v4.10.0 benchmark corpus:

  • bookstack: 1 instance (the one above; visible() is reused dozens of times in real BookStack but the rest go through subclasses or query() so they resolve)
  • Pattern is common enough to land in other audits: polymorphic-base repositories, Spatie\Permission Role/Permission abstract patterns, single-table-inheritance bases.

Related

Labels: eloquent, priority:medium

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions