Skip to content

fix(database): use static instead of self in IsDatabaseModel - #2274

Merged
brendt merged 1 commit into
tempestphp:3.xfrom
osbre:fix/database-model-static-return-types
Sep 7, 2026
Merged

fix(database): use static instead of self in IsDatabaseModel#2274
brendt merged 1 commit into
tempestphp:3.xfrom
osbre:fix/database-model-static-return-types

Conversation

@osbre

@osbre osbre commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Problem

IsDatabaseModel trait methods using self caused two issues:

  • Runtime & PHPStan: self inside a trait resolves to the class that uses the trait, not the class the method was called on. For example, calling Admin::create() (where Admin extends User and User uses the trait) built queries against User, and PHPStan incorrectly typed the return as User.
  • Mago: Mago resolved self inside a trait to the trait itself (IsDatabaseModel), forcing consumers to manually narrow types with instanceof.

Fix

  • Switched return types to static and routed calls through static::queryBuilder() / query(static::class).
  • Inherited models now query themselves, and static analysers correctly infer the concrete model.
  • Added 5 @phpstan-ignore-next-line annotations (matching the one pre-existing in resolve()).

Why the PHPStan Ignores Are Needed

QueryBuilder returns TModel|object<TModel>, but object is not generic in PHPStan - object<TModel> degrades to bare object, which doesn't satisfy static. That is what forces the suppression today.

The correct annotation (@template TModel of object with @param class-string<TModel>|TModel) resolves cleanly. But query() also accepts raw table names (e.g., query('users')), and adding |string to the union collapses PHPStan's template inference (upstream issue phpstan#12985). A permanent fix requires binding TModel from an instance rather than a class-string (similar to Eloquent), which is out of scope for this PR.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Benchmark Results

Comparison of fix/database-model-static-return-types against 3.x (cf035264805564eb789f2741734e481aa3696b4d).

Open to see the benchmark results
Benchmark Set Mem. Peak Time Variability
DiscoveryScanBench(benchFullDiscoveryScan) - 25.857mb 0.00% 19.535ms +10.69% ±0.48% -65.05%

Generated by phpbench against commit 5ce9c94

@aidan-casey

Copy link
Copy Markdown
Member

This honestly should have been static to begin with. This looks good to me, but I'll let @brendt and @innocenzi weigh in.

Thanks!

@brendt

brendt commented Sep 7, 2026

Copy link
Copy Markdown
Member

It wasn't static because PhpStorm had issues with traits + generics + static in the past. However, I just checked and it seems to work now with static, so let's merge!

@brendt
brendt merged commit dfd921f into tempestphp:3.x Sep 7, 2026
79 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants