Skip to content

Commit 812aa4f

Browse files
committed
A custom Eloquent builder keeps the model it was built for
1 parent 36e8f35 commit 812aa4f

8 files changed

Lines changed: 245 additions & 43 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
108108
### Fixed
109109

110110
- **A Laravel project that requires Larastan gets PHPStan diagnostics too.** PHPStan auto-detection looked only for a direct `phpstan/phpstan` dependency in `composer.json`, so a project that requires `larastan/larastan` and lets it pull `phpstan/phpstan` in transitively never had `vendor/bin/phpstan` recognised, even though the binary was right there. A Laravel project is now recognised through a direct `larastan/larastan` dependency instead: plain PHPStan does not understand Eloquent magic, facades, or container bindings, so a Laravel project that depends on `phpstan/phpstan` directly but has not installed Larastan is still left alone rather than run through an analyser that would misread its own framework.
111+
- **A custom Eloquent builder keeps the model it was built for.** `SiteCertificate::query()->whereKey($id)->firstOrFail()` resolved to the base `Model`, or reported `subject type 'TModel' could not be resolved`, whenever the model routed its queries through a custom builder. PHP has no generics, so almost nobody writes `@template`/`@extends` on a builder subclass: `class SiteCertificateBuilder extends Builder {}` is the whole class, and the model was lost at whichever method on the chain returns it. The model the query was started from now travels through the builder to the end of the chain, so the result of `firstOrFail()`, `first()`, `get()`, and the rest completes, hovers, and is checked as the concrete model, whether the builder is declared with generics or without. A builder specialised this way also keeps everything the ordinary resolution gives it, including the query-builder methods it reaches through `@mixin`. Closes #362.
111112
- **`array_filter()` reports the type the filter leaves behind.** A callback that tests the value it is handed proves something about every entry that survives, but the result kept whatever element type went in, so `array_filter($values, fn ($v) => $v !== null)` still looked like it could hold `null` and returning it from a function declared `int[]` was reported as a type error. The surviving values are now narrowed the way the body of an `if` narrows the variable it guards, whether the test is written as an `is_…()` call, a comparison against `null`, an `instanceof` check, or a callable string such as `'is_int'`. The keys were already narrowed this way in the modes that hand the callback a key, and both halves narrow together under `ARRAY_FILTER_USE_BOTH`. Closes #376.
112113
- **A deprecation warning belongs to the variable it is written on.** The deprecated-usage check typed its subject from a cache keyed by variable name and class, so two methods of the same class that reuse a parameter name shared one entry and whichever type was bound first won. A Laravel controller with a `Request $request` method above a `PendingRequest $request` method reported `Illuminate\Http\Request::get is deprecated` on the HTTP client call, where `get()` is the ordinary way to make a request, and renaming either parameter made the warning vanish. Every subject is now typed in the scope it is written in, so each method, closure, and `instanceof` branch gets its own answer, and a genuine deprecation is still reported no matter which order the methods appear in.
113114
- **A generic argument left out takes the default its `@template` declares.** `@template TAsync of bool = false` says that a use of the class without a generic argument means `false`, but the parameter was widened to its upper bound instead, so a conditional return keyed on it always picked the else branch. Laravel's HTTP client is where this shows up: `PendingRequest` declares synchronous mode as its default, so an ordinary `Http::get()` looked like it returned a promise rather than a `Response`, and `json()` along with the rest of the response API appeared to be missing on it. A plain request now resolves to `Response` whether it is made through `PendingRequest`, the client factory, or the `Http` facade, and `async()` still resolves to `PromiseInterface` through all three. Contributed by @shuvroroy (#377).

examples/laravel/app/Demo.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
use App\Mail\OrderShipped;
1515
use App\Http\Requests\StoreBakeryRequest;
1616
use App\Http\Requests\UpdateBakeryRequest;
17+
use App\Models\Baker;
1718
use App\Models\Bakery;
1819
use App\Models\BlogAuthor;
1920
use App\Models\BlogPost;
2021
use App\Models\Customer;
22+
use App\Models\Loaf;
2123
use App\Models\PostCollection;
2224
use App\Models\Review;
2325
use App\Models\ReviewCollection;
@@ -155,6 +157,14 @@ public function eloquentQuery(): void
155157
BlogAuthor::where('active', 1)->when(true, fn($q) => $q)->get();
156158
BlogAuthor::where('active', 1)->unless(false, fn($q) => $q)->first();
157159

160+
// Custom builders keep the model they were built for, whether or
161+
// not the builder class declares generics. LoafBuilder declares
162+
// none (see app/Models/LoafBuilder.php), BakerBuilder does.
163+
Loaf::query()->stale()->where('crust', 'sourdough')->firstOrFail()->getWeight();
164+
Loaf::query()->whereKey(1)->first()->getWeight(); // → Loaf|null
165+
Loaf::query()->stale()->get(); // → Collection<Loaf>
166+
Baker::query()->active()->firstOrFail()->getName(); // → Baker
167+
158168
// Paginators carry the model element type through foreach
159169
foreach (BlogAuthor::where('active', 1)->paginate() as $author) {
160170
$author->profile->getBio(); // → BlogAuthor

examples/laravel/app/Models/Loaf.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22

33
namespace App\Models;
44

5+
use Illuminate\Database\Eloquent\Attributes\UseEloquentBuilder;
56
use Illuminate\Database\Eloquent\Model;
67

8+
#[UseEloquentBuilder(LoafBuilder::class)]
79
class Loaf extends Model
810
{
911
public function getWeight(): int { return 0; }

examples/laravel/assertions.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,34 @@ function assertMethodReturnType(string $class, string $method, string $expected)
146146
$result instanceof \Illuminate\Database\Eloquent\Builder
147147
);
148148

149+
// #[UseEloquentBuilder] hands the query off to the custom builder, and the
150+
// builder keeps a reference to the model it was built for — which is why the
151+
// analyzer can name the concrete model at the end of the chain even though
152+
// LoafBuilder declares no `@template` of its own.
153+
$result = \App\Models\Loaf::query();
154+
check(
155+
'Loaf::query() returns LoafBuilder (#[UseEloquentBuilder])',
156+
$result instanceof \App\Models\LoafBuilder
157+
);
158+
check(
159+
'LoafBuilder::stale() keeps the chain on LoafBuilder',
160+
$result->stale() instanceof \App\Models\LoafBuilder
161+
);
162+
check(
163+
'LoafBuilder holds the Loaf model it queries',
164+
$result->getModel() instanceof \App\Models\Loaf
165+
);
166+
167+
$result = \App\Models\Baker::query();
168+
check(
169+
'Baker::query() returns BakerBuilder (#[UseEloquentBuilder])',
170+
$result instanceof \App\Models\BakerBuilder
171+
);
172+
check(
173+
'BakerBuilder holds the Baker model it queries',
174+
$result->getModel() instanceof \App\Models\Baker
175+
);
176+
149177
// Model::fresh() on instance (non-existing model returns null)
150178
$result = $bakery->fresh();
151179
check(

src/inheritance/mod.rs

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -566,37 +566,67 @@ pub(crate) fn resolve_class_with_inheritance(
566566
merged
567567
}
568568

569-
/// Whether `class` declares no `@template` of its own but fixes exactly
570-
/// one ancestor's generics via `@extends`, with an arg count matching
571-
/// `new_arg_count` — the shape [`rebind_extends_only_generics`] needs.
572-
pub(crate) fn is_extends_only_generic_rebindable(class: &ClassInfo, new_arg_count: usize) -> bool {
573-
class.template_params.is_empty()
574-
&& matches!(class.extends_generics.as_slice(), [(_, args)] if args.len() == new_arg_count)
569+
/// Whether `class` declares no `@template` of its own but still takes
570+
/// `new_arg_count` type arguments through its parent — the shape
571+
/// [`rebind_extends_only_generics`] needs.
572+
pub(crate) fn is_extends_only_generic_rebindable(
573+
class: &ClassInfo,
574+
new_arg_count: usize,
575+
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
576+
) -> bool {
577+
rebindable_parent(class, new_arg_count, class_loader).is_some()
578+
}
579+
580+
/// The parent whose `@extends` binding [`rebind_extends_only_generics`]
581+
/// would override, for a class that declares no `@template` of its own.
582+
///
583+
/// Two shapes qualify: the class fixes exactly one ancestor's generics
584+
/// via `@extends`, or it names no generics at all and simply extends a
585+
/// generic parent. The latter is how nearly every custom Eloquent
586+
/// builder is written (`class UserBuilder extends Builder {}`): PHP has
587+
/// no generics, so the subclass silently stands in for `Builder<TModel>`
588+
/// and a caller that knows the model (`UserBuilder<User>`) has to be
589+
/// able to bind it.
590+
fn rebindable_parent(
591+
class: &ClassInfo,
592+
new_arg_count: usize,
593+
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
594+
) -> Option<Atom> {
595+
if !class.template_params.is_empty() {
596+
return None;
597+
}
598+
match class.extends_generics.as_slice() {
599+
[(parent, args)] if args.len() == new_arg_count => Some(*parent),
600+
[] => {
601+
let parent_name = class.parent_class.as_deref()?;
602+
let parent = class_loader(parent_name)?;
603+
(parent.template_params.len() == new_arg_count).then(|| atom(parent_name))
604+
}
605+
_ => None,
606+
}
575607
}
576608

577-
/// Re-derive `class` with its single `@extends` generic binding replaced
578-
/// by `new_args`, baking the override into every inherited member the
579-
/// same way the original binding was baked.
609+
/// Re-derive `class` with its `@extends` generic binding replaced by
610+
/// `new_args`, returning the overridden raw class alongside the class
611+
/// re-merged through the parent chain under that binding.
580612
///
581613
/// A `static<TNewKey, TValue>` return type rebind (Laravel's
582614
/// `Collection::keyBy()`/`groupBy()`/`mapWithKeys()` and friends) names
583615
/// the calling class, but a concrete collection subclass that only fixes
584616
/// its key/value types through `@extends` (`final class Sub extends
585617
/// Collection {}` with `@extends Collection<int, Item>`) has no
586618
/// `@template` of its own for [`apply_generic_args`] to substitute
587-
/// against. Overriding the `@extends` binding and re-running the parent
619+
/// against. Neither has a custom Eloquent builder, which usually names
620+
/// no generics at all. Overriding the binding and re-running the parent
588621
/// chain merge reproduces the same baking the original binding went
589622
/// through, just with the rebind's args in place of the old ones.
590623
///
591624
/// Returns `None` when `class` is not shaped like [`is_extends_only_generic_rebindable`]
592625
/// describes.
593626
///
594-
/// Only base inheritance merge runs (traits + parent chain) — no virtual
595-
/// member providers, interface merging, or Laravel patches, the same
596-
/// trade-off [`crate::virtual_members::resolve_class_base_cached`] makes.
597-
/// This covers a rebind through real declared methods; a macro registered
598-
/// on the base collection class after the rebind will not be visible on
599-
/// the result (see `docs/todo/laravel.md`).
627+
/// The caller is responsible for layering virtual members, interface
628+
/// members, and framework patches back on — that is what the returned
629+
/// raw class is for.
600630
///
601631
/// `class` may be a raw (unmerged) or already fully-resolved `ClassInfo`
602632
/// — `extends_generics` passes through inheritance merge unchanged either
@@ -610,18 +640,14 @@ pub(crate) fn rebind_extends_only_generics(
610640
class: &ClassInfo,
611641
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
612642
new_args: &[PhpType],
613-
) -> Option<ClassInfo> {
614-
let [(parent_name, parent_args)] = class.extends_generics.as_slice() else {
615-
return None;
616-
};
617-
if parent_args.len() != new_args.len() {
618-
return None;
619-
}
643+
) -> Option<(ClassInfo, ClassInfo)> {
644+
let parent_name = rebindable_parent(class, new_args.len(), class_loader)?;
620645
let fqn = class.fqn();
621646
let raw = class_loader(fqn.as_str()).filter(|raw| raw.fqn().eq_ignore_ascii_case(fqn.as_str()));
622647
let mut overridden = raw.as_deref().unwrap_or(class).clone();
623-
overridden.extends_generics = vec![(*parent_name, new_args.to_vec())];
624-
Some(resolve_class_with_inheritance(&overridden, class_loader))
648+
overridden.extends_generics = vec![(parent_name, new_args.to_vec())];
649+
let merged = resolve_class_with_inheritance(&overridden, class_loader);
650+
Some((overridden, merged))
625651
}
626652

627653
/// Look up a method's return type through the inheritance chain.

src/type_engine/types/resolution.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -418,14 +418,16 @@ fn resolve_named_type(
418418

419419
// Apply generic substitution if the type hint carried generic
420420
// arguments and the class has template parameters of its own
421-
// to substitute, or (a `static<TNewKey, TValue>` rebind on a
422-
// concrete collection subclass) fixes a single ancestor's
423-
// generics via `@extends` instead.
421+
// to substitute, or inherits them from a generic parent it
422+
// never bound (a `static<TNewKey, TValue>` rebind on a
423+
// concrete collection subclass, a custom Eloquent builder
424+
// specialised to its model).
424425
if !generic_args.is_empty()
425426
&& (!cls.template_params.is_empty()
426427
|| crate::inheritance::is_extends_only_generic_rebindable(
427428
&cls,
428429
generic_args.len(),
430+
class_loader,
429431
))
430432
{
431433
let generic_arg_strings: Vec<String> =

src/virtual_members/resolve.rs

Lines changed: 58 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -377,9 +377,35 @@ pub fn resolve_class_fully_with_generics(
377377

378378
let mut result = if !base.template_params.is_empty() {
379379
Arc::new(crate::inheritance::apply_generic_args(&base, generic_args))
380-
} else if let Some(rebound) =
380+
} else if let Some((overridden, mut rebound)) =
381381
crate::inheritance::rebind_extends_only_generics(class, class_loader, generic_args)
382382
{
383+
// The rebind restarts from base inheritance, so everything the
384+
// full resolution layers on top (virtual members, `@mixin`
385+
// expansion, interface members, framework patches) has to run
386+
// again over the re-merged class. `mark_in_flight` keeps a
387+
// provider that resolves this same specialisation from
388+
// recursing back into the rebind; on re-entry the caller gets
389+
// the base-inheritance-only result instead.
390+
let temp_cache;
391+
let stage_cache: &ResolvedClassCache = match cache {
392+
Some(c) => c,
393+
None => match super::cache::active_resolved_class_cache() {
394+
Some(active) => active,
395+
None => {
396+
temp_cache = super::cache::new_resolved_class_cache();
397+
&temp_cache
398+
}
399+
},
400+
};
401+
if stage_cache.write().mark_in_flight(fqn) {
402+
let _in_flight = InFlightGuard {
403+
cache: stage_cache,
404+
fqn,
405+
};
406+
apply_post_merge_stages(&mut rebound, &overridden, class_loader, stage_cache);
407+
}
408+
rebound.rebuild_method_index();
383409
Arc::new(rebound)
384410
} else {
385411
base
@@ -543,6 +569,34 @@ fn resolve_class_fully_inner(
543569

544570
// ── Uncached resolution ─────────────────────────────────────────
545571
let mut merged = resolve_class_with_inheritance(effective_class, class_loader);
572+
apply_post_merge_stages(&mut merged, effective_class, class_loader, cache);
573+
574+
// ── Cache store ─────────────────────────────────────────────────
575+
merged.rebuild_method_index();
576+
let result = Arc::new(merged);
577+
cache.write().insert(cache_key, Arc::clone(&result));
578+
579+
result
580+
}
581+
582+
/// Layer virtual members, interface members, and framework patches onto
583+
/// an inheritance-merged class.
584+
///
585+
/// Split out of [`resolve_class_fully_inner`] so that a class re-merged
586+
/// under an overridden `@extends` binding (see
587+
/// [`rebind_extends_only_generics`](crate::inheritance::rebind_extends_only_generics))
588+
/// goes through exactly the same stages instead of stopping at base
589+
/// inheritance. `effective_class` is the un-merged class the stages
590+
/// read structural information from (interfaces, `@implements`
591+
/// generics, the parent chain); `merged` is the result of running the
592+
/// inheritance merge over it.
593+
fn apply_post_merge_stages(
594+
merged: &mut ClassInfo,
595+
effective_class: &ClassInfo,
596+
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
597+
cache: &ResolvedClassCache,
598+
) {
599+
let fqn = effective_class.fqn();
546600

547601
// Whether Laravel-specific resolution should run. A throwaway
548602
// cache defaults to `true`, so behaviour is unchanged for
@@ -582,7 +636,7 @@ fn resolve_class_fully_inner(
582636

583637
let providers = default_providers(is_laravel);
584638
if !providers.is_empty() {
585-
apply_virtual_members(&mut merged, class_loader, &providers, Some(cache));
639+
apply_virtual_members(merged, class_loader, &providers, Some(cache));
586640
}
587641

588642
// ── Interface member merging ────────────────────────────────────
@@ -758,11 +812,7 @@ fn resolve_class_fully_inner(
758812
// right input for the substituted case too.
759813
let resolved_iface = resolve_class_fully_inner(&iface, class_loader, Some(cache));
760814

761-
merge_interface_members_into(
762-
&mut merged,
763-
ClassInfo::clone(&resolved_iface),
764-
&iface_subs,
765-
);
815+
merge_interface_members_into(merged, ClassInfo::clone(&resolved_iface), &iface_subs);
766816
}
767817
}
768818

@@ -790,15 +840,8 @@ fn resolve_class_fully_inner(
790840
// `laravel/patches.rs` for the full patch inventory. Skipped for
791841
// non-Laravel projects.
792842
if is_laravel {
793-
apply_laravel_patches(&mut merged, &fqn);
843+
apply_laravel_patches(merged, &fqn);
794844
}
795-
796-
// ── Cache store ─────────────────────────────────────────────────
797-
merged.rebuild_method_index();
798-
let result = Arc::new(merged);
799-
cache.write().insert(cache_key, Arc::clone(&result));
800-
801-
result
802845
}
803846

804847
/// Merge resolved interface members into a class, applying `@implements`

0 commit comments

Comments
 (0)