From e6ead4f7d5703d8313600bd85b24e1caa36e1470 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Fri, 13 Nov 2026 18:51:59 +0600 Subject: [PATCH 1/3] fix(laravel): preserve custom builders through local scopes Keep the custom builder and concrete model when a fluent local scope returns Eloquent Builder. Prefer the actual scope signature over mixed-in method annotations while preserving real builder methods and macros. Recover callback model types from custom builder returns using the exact class name, and preserve explicit return types for other results. Closes #571 --- .../variable/closure_resolution.rs | 29 +++++++++- .../laravel/builder_injection.rs | 15 ++++- src/virtual_members/laravel/model_provider.rs | 9 ++- src/virtual_members/laravel/scopes.rs | 55 +++++++++--------- src/virtual_members/laravel/scopes_tests.rs | 57 +++++++++++++++++++ src/virtual_members/laravel/tests.rs | 53 +++++++++++++++++ 6 files changed, 182 insertions(+), 36 deletions(-) diff --git a/src/type_engine/variable/closure_resolution.rs b/src/type_engine/variable/closure_resolution.rs index eb165bb16..d27267463 100644 --- a/src/type_engine/variable/closure_resolution.rs +++ b/src/type_engine/variable/closure_resolution.rs @@ -880,14 +880,15 @@ fn extract_generic_args_from_methods(class: &ClassInfo, class_fqn: &str) -> Opti None } -/// Extract the model type from a resolved `Builder` class by -/// scanning its method return types for `Builder` and returning `X`. +/// Recover the model from a resolved builder's fluent return types, +/// including scopes returning the custom builder itself. pub(in crate::type_engine) fn extract_model_from_builder(builder: &ClassInfo) -> Option { + let builder_fqn = builder.fqn(); for method in &builder.methods { if let Some(ref ret) = method.return_type && let TypeKind::Generic(g) = ret.kind() && !g.args.is_empty() - && (g.name == ELOQUENT_BUILDER_FQN || g.name == "Builder") + && (g.name == ELOQUENT_BUILDER_FQN || g.name == "Builder" || g.name == builder_fqn) { // Skip unsubstituted template params like "TModel". if !g.args[0].is_empty() && !g.args[0].is_named("TModel") { @@ -897,3 +898,25 @@ pub(in crate::type_engine) fn extract_model_from_builder(builder: &ClassInfo) -> } None } + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_fixtures::{make_class, make_method}; + + #[test] + fn recover_model_from_custom_builder_uses_its_exact_class() { + let mut builder = make_class("App\\Builders\\Query"); + for (name, ret) in [ + ("other", "Other\\Builders\\Query"), + ("unbound", "App\\Builders\\Query"), + ("scoped", "App\\Builders\\Query"), + ] { + builder.methods.push(Arc::new(make_method(name, Some(ret)))); + } + assert_eq!( + extract_model_from_builder(&builder), + Some(PhpType::named(atom("App\\Models\\Post"))), + ); + } +} diff --git a/src/virtual_members/laravel/builder_injection.rs b/src/virtual_members/laravel/builder_injection.rs index 5d8bc1dbe..0a5b3bf09 100644 --- a/src/virtual_members/laravel/builder_injection.rs +++ b/src/virtual_members/laravel/builder_injection.rs @@ -279,11 +279,20 @@ fn inject_scopes_and_model_methods( // 1. Inject scope methods. let scope_methods = build_scope_methods_for_builder(model_arg, class_loader); for method in scope_methods { - let already_exists = result + let existing = result .methods .iter() - .any(|m| m.name == method.name && m.is_static == method.is_static); - if !already_exists { + .position(|m| m.name.eq_ignore_ascii_case(&method.name)); + if let Some(index) = existing { + // Builder::__call invokes the model's scope ahead of any + // @method or @mixin description of that scope. Real builder + // methods and registered macros still win at runtime. + let previous = &result.methods[index]; + if previous.is_virtual && !previous.is_macro { + result.methods.make_mut()[index] = + forward_builder_return(&method, forwarded, class_loader); + } + } else { result .methods .push(forward_builder_return(&method, forwarded, class_loader)); diff --git a/src/virtual_members/laravel/model_provider.rs b/src/virtual_members/laravel/model_provider.rs index a461510aa..32117d6f5 100644 --- a/src/virtual_members/laravel/model_provider.rs +++ b/src/virtual_members/laravel/model_provider.rs @@ -27,7 +27,7 @@ use super::relationships::{ RelationshipKind, build_property_type, classify_relationship_typed, count_property_name, extract_pivot_accessor_typed, extract_related_type_typed, }; -use super::scopes::{build_scope_methods, is_scope_method}; +use super::scopes::{build_scope_methods, is_scope_method, rebind_scope_builder_return}; use super::table_names::model_schema_table; use super::where_property::{build_where_property_methods_with_properties, lowercase_method_names}; use super::{ @@ -463,6 +463,7 @@ impl<'a> ModelMembers<'a> { /// relationship method loads. fn add_method_members(&mut self) { let class = self.class; + let mut builder_type = None; for method in &class.methods { if self.is_base_model_method(method) { continue; @@ -473,7 +474,11 @@ impl<'a> ModelMembers<'a> { // A method that is both `#[Scope]`-attributed and named // `scopeX` keeps the attribute's name as-is; the prefix is // only stripped for convention-based scope methods. - let [instance_method, static_method] = build_scope_methods(method); + let builder = builder_type + .get_or_insert_with(|| super::model_builder_type(class, self.class_loader)); + let [mut instance_method, mut static_method] = build_scope_methods(method); + rebind_scope_builder_return(&mut instance_method, builder); + rebind_scope_builder_return(&mut static_method, builder); self.methods.push(Arc::new(instance_method)); self.methods.push(Arc::new(static_method)); continue; diff --git a/src/virtual_members/laravel/scopes.rs b/src/virtual_members/laravel/scopes.rs index 830cc18b1..c95a7492e 100644 --- a/src/virtual_members/laravel/scopes.rs +++ b/src/virtual_members/laravel/scopes.rs @@ -12,7 +12,6 @@ use std::sync::Arc; use crate::php_type::{PhpType, TypeKind}; use crate::types::{MethodExtras, MethodInfo}; -use crate::util::short_name; use super::ELOQUENT_BUILDER_FQN; use super::helpers::extends_eloquent_model; @@ -94,6 +93,29 @@ pub(super) fn scope_return_type(method: &MethodInfo) -> PhpType { } } +/// Keep a fluent scope on the model's builder, while preserving a return +/// that explicitly queries a different model or names another result type. +pub(super) fn rebind_scope_builder_return(method: &mut MethodInfo, builder: &PhpType) { + let Some(ret) = method.return_type.as_ref() else { + return; + }; + let should_rebind = match ret.kind() { + TypeKind::Named(name) => Fqn::new(name).as_str() == ELOQUENT_BUILDER_FQN, + TypeKind::Generic(g) => { + Fqn::new(&g.name).as_str() == ELOQUENT_BUILDER_FQN + && builder.base_name() != Some(ELOQUENT_BUILDER_FQN) + && g.args.first().is_some_and(|model| { + model.is_self_ref() + || matches!(builder.kind(), TypeKind::Generic(b) if b.args.first() == Some(model)) + }) + } + _ => false, + }; + if should_rebind { + method.return_type = Some(builder.clone()); + } +} + /// Build virtual methods for a scope method. /// /// Returns two `MethodInfo` values: one static and one instance. Both @@ -189,6 +211,7 @@ pub fn build_scope_methods_for_builder( let subs = super::self_ref_subs(model_type.clone()); let mut methods = Vec::new(); + let mut builder_type = None; // The rewrite below depends only on the model, so the produced // copies are interned and shared by every Builder and relation @@ -203,6 +226,8 @@ pub fn build_scope_methods_for_builder( if !is_scope_method(method) { continue; } + let builder = builder_type + .get_or_insert_with(|| super::model_builder_type(&model_class, class_loader)); let transformed = crate::virtual_members::intern_transformed_method(method, fp, || { // Build an instance method (scopes are called as instance @@ -235,21 +260,8 @@ pub fn build_scope_methods_for_builder( if let Some(ref mut ret) = m.return_type { *ret = ret.substitute(&subs); - - // When a scope method declares a bare `Builder` return type - // (without generic args), the chain loses track of the - // concrete model. Subsequent calls on the returned Builder - // would not find model-specific scope methods because - // `type_hint_to_classes_typed` only injects scopes when - // generic args are present. Wrap bare Builder return types - // as `Builder` to preserve the chain. - if is_bare_builder_type(ret) { - *ret = PhpType::generic( - ELOQUENT_BUILDER_FQN, - vec![PhpType::named(atom(model_name))], - ); - } } + rebind_scope_builder_return(&mut m, builder); m }); @@ -259,19 +271,6 @@ pub fn build_scope_methods_for_builder( methods } -/// Check whether a `PhpType` is a bare Eloquent Builder reference -/// without generic arguments. -/// -/// Matches both the FQN (`Illuminate\Database\Eloquent\Builder`) and -/// the short name (`Builder`) since scope methods in user code -/// typically use the imported short name. -fn is_bare_builder_type(ty: &PhpType) -> bool { - match ty.kind() { - TypeKind::Named(name) => name == ELOQUENT_BUILDER_FQN || short_name(name) == "Builder", - _ => false, - } -} - #[cfg(test)] #[path = "scopes_tests.rs"] mod tests; diff --git a/src/virtual_members/laravel/scopes_tests.rs b/src/virtual_members/laravel/scopes_tests.rs index 9004dbfc2..3d562785c 100644 --- a/src/virtual_members/laravel/scopes_tests.rs +++ b/src/virtual_members/laravel/scopes_tests.rs @@ -154,6 +154,63 @@ fn scope_return_type_custom() { ); } +#[test] +fn rebind_scope_builder_return_preserves_explicit_results() { + let custom = PhpType::parse("App\\Builders\\PostBuilder"); + let base = PhpType::parse("Illuminate\\Database\\Eloquent\\Builder"); + for (ret, builder, expected) in [ + (None, &custom, None), + (Some("int"), &custom, Some("int")), + (Some("int|string"), &custom, Some("int|string")), + (Some("App\\Builder"), &custom, Some("App\\Builder")), + (Some("Builder"), &custom, Some("Builder")), + ( + Some("Illuminate\\Database\\Eloquent\\Builder"), + &custom, + Some("App\\Builders\\PostBuilder"), + ), + ( + Some("\\Illuminate\\Database\\Eloquent\\Builder"), + &custom, + Some("App\\Builders\\PostBuilder"), + ), + ( + Some("Illuminate\\Database\\Eloquent\\Builder"), + &base, + Some("Illuminate\\Database\\Eloquent\\Builder"), + ), + ( + Some("Illuminate\\Database\\Eloquent\\Builder"), + &custom, + Some("App\\Builders\\PostBuilder"), + ), + ( + Some("Illuminate\\Database\\Eloquent\\Builder"), + &custom, + Some("App\\Builders\\PostBuilder"), + ), + ( + Some("Illuminate\\Database\\Eloquent\\Builder"), + &custom, + Some("Illuminate\\Database\\Eloquent\\Builder"), + ), + ( + Some("App\\Builders\\OtherBuilder"), + &custom, + Some("App\\Builders\\OtherBuilder"), + ), + ( + Some("Illuminate\\Database\\Eloquent\\Builder"), + &base, + Some("Illuminate\\Database\\Eloquent\\Builder"), + ), + ] { + let mut method = make_method("scopePublished", ret); + rebind_scope_builder_return(&mut method, builder); + assert_eq!(method.return_type, expected.map(PhpType::parse), "{ret:?}"); + } +} + // ── build_scope_methods (convention) ───────────────────────────────── #[test] diff --git a/src/virtual_members/laravel/tests.rs b/src/virtual_members/laravel/tests.rs index 0f624fff1..c14b8fd36 100644 --- a/src/virtual_members/laravel/tests.rs +++ b/src/virtual_members/laravel/tests.rs @@ -12,6 +12,59 @@ use std::sync::Arc; // ── applies_to ────────────────────────────────────────────────────── +#[test] +fn builder_scope_injection_respects_declared_methods_and_macros() { + let mut model = make_class("App\\Models\\Post"); + model.parent_class = Some(atom(ELOQUENT_MODEL_FQN)); + model.methods.push(Arc::new(make_method( + "scopePublished", + Some(ELOQUENT_BUILDER_FQN), + ))); + let loader = |fqn: Fqn<'_>| match fqn.as_str() { + "App\\Models\\Post" => Some(Arc::new(model.clone())), + ELOQUENT_MODEL_FQN => Some(Arc::new(make_class(ELOQUENT_MODEL_FQN))), + _ => None, + }; + for (is_virtual, is_macro, is_static, expected) in [ + (false, false, false, "int"), + (true, true, false, "int"), + ( + true, + false, + false, + "Illuminate\\Database\\Eloquent\\Builder", + ), + ( + true, + false, + true, + "Illuminate\\Database\\Eloquent\\Builder", + ), + ] { + let mut builder = make_class(ELOQUENT_BUILDER_FQN); + let raw = builder.clone(); + let mut method = make_method("PUBLISHED", Some("int")); + method.is_virtual = is_virtual; + method.is_macro = is_macro; + method.is_static = is_static; + builder.methods.push(Arc::new(method)); + super::builder_injection::try_inject_builder_scopes( + &mut builder, + &raw, + ELOQUENT_BUILDER_FQN, + &[PhpType::named(model.fqn())], + &loader, + ); + let instance = builder + .methods + .iter() + .find(|m| !m.is_static && m.name.eq_ignore_ascii_case("published")) + .unwrap(); + assert_eq!(instance.return_type_str().as_deref(), Some(expected)); + assert!(!instance.is_static); + } +} + #[test] fn model_builder_type_survives_missing_framework_and_custom_stubs() { let mut model = make_class("App\\Models\\Product"); From ef39dc924e6acdd536b9569fb8e1f27b81abf546 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Fri, 13 Nov 2026 18:52:09 +0600 Subject: [PATCH 2/3] test(laravel): cover custom builder scope chains Verify hover types, completion, model generics, and return diagnostics across ide-helper annotations and builders with or without templates. Cover inherited scopes, attribute scopes, builder overrides, and explicit non-builder returns from a caller in a different namespace. Refs #571 --- tests/integration/laravel_custom_builder.rs | 184 ++++++++++++++++++++ 1 file changed, 184 insertions(+) diff --git a/tests/integration/laravel_custom_builder.rs b/tests/integration/laravel_custom_builder.rs index 22c8cb1d2..eb07c53b7 100644 --- a/tests/integration/laravel_custom_builder.rs +++ b/tests/integration/laravel_custom_builder.rs @@ -2,6 +2,190 @@ use crate::common::create_psr4_workspace; use crate::common::{completion_response, goto_definition_at, open_php}; use tower_lsp_server::gen_lsp_types::*; +#[tokio::test] +async fn test_custom_builder_scopes_preserve_query_type() { + use crate::common::{ + assigned_type, complete_labels_at, hover_on, messages_with_code, position_after, + }; + + for (model_tag, builder_mixin, extends_tag) in [ + (true, true, "@template-extends"), + (false, true, "@template-extends"), + (true, false, "@template-extends"), + (true, true, "@extends"), + (false, false, ""), + ] { + let builder = format!( + r#"") + }, + if builder_mixin { "@mixin Post" } else { "" }, + ); + let model = format!( + r#" */ + protected function scopeGeneric(Builder $query): Builder {{ return $query; }} + protected function scopeCounted(Builder $query): int {{ return 1; }} + protected function scopeOverridden(Builder $query): Builder {{ return $query; }} +}} +class ChildPost extends Post {{}} +"#, + if model_tag { + "@method static PostBuilder|Post published()" + } else { + "" + }, + ); + let (backend, dir) = create_psr4_workspace( + r#"{"autoload":{"psr-4":{"App\\":"app/","Illuminate\\":"vendor/illuminate/"}}}"#, + &[ + ( + "vendor/illuminate/Database/Eloquent/Model.php", + " */ + public function scoped(): PostBuilder { + $plain = Post::query(); + $published = Post::query()->published(); + $recent = Post::query()->recent(); + $featured = Post::query()->featured(); + $generic = Post::query()->generic(); + $inherited = Post::query()->inherited(); + $child = ChildPost::query()->published(); + $static = Post::published(); + $instance = (new Post())->published(); + $chained = Post::query()->published()->recent()->featured()->popular(); + $model = Post::query()->published()->firstOrFail(); + $plainModel = Post::query()->firstOrFail(); + $count = Post::query()->counted(); + $override = Post::query()->overridden(); + return Post::query()->published(); + } + public function wrong(): string { + return Post::query()->published(); + } +} +"#; + open_php(&backend, &uri, content).await; + for variable in [ + "$plain", + "$published", + "$recent", + "$featured", + "$generic", + "$static", + "$instance", + "$chained", + "$inherited", + ] { + assert_eq!( + assigned_type(&backend, uri.as_str(), content, variable), + "PostBuilder", + "{variable}, model tag: {model_tag}, mixin: {builder_mixin}, {extends_tag}", + ); + } + assert_eq!( + assigned_type(&backend, uri.as_str(), content, "$child"), + "PostBuilder", + ); + for (variable, expected) in [ + ("$plainModel", "Post"), + ("$model", "Post"), + ("$count", "int"), + ("$override", "string"), + ] { + assert_eq!( + assigned_type(&backend, uri.as_str(), content, variable), + expected + ); + } + let hover = hover_on(&backend, &uri, content, "$published =").await; + assert!(!hover.contains("PostBuilderpop"); + let labels = complete_labels_at( + &backend, + &uri, + completion, + position.line, + position.character, + ) + .await; + assert!( + labels.iter().any(|label| label.starts_with("popular")), + "{labels:?}" + ); + } +} + #[tokio::test] async fn test_custom_eloquent_builder_attribute() { let (backend, dir) = create_psr4_workspace( From fdbdc538163c5fcbe9a2022208acc07c2aeeb2a6 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Fri, 13 Nov 2026 18:52:19 +0600 Subject: [PATCH 3/3] docs(laravel): demonstrate scoped custom builders Add convention and attribute scope examples with ide-helper docblocks, runtime assertions for builder identity and model retention, and an unreleased changelog entry. Refs #571 --- docs/CHANGELOG.md | 1 + examples/laravel/app/Demo.php | 9 +++++++++ examples/laravel/app/Models/Baker.php | 14 ++++++++++++++ examples/laravel/app/Models/BakerBuilder.php | 1 + examples/laravel/assertions.php | 14 ++++++++++++++ 5 files changed, 39 insertions(+) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index d1165bcd1..7ff826124 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -521,6 +521,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 #### Laravel +- **Scopes keep custom Eloquent builders.** Chaining a local scope preserves the custom builder and its model type, including with ide-helper docblocks, so its methods remain available and valid returns are no longer reported as type mismatches. Contributed by @shuvroroy. - **Route groups in `bootstrap/app.php`.** Routes registered from the `then:` or `using:` callback of `withRouting()`, including route files grouped there under a name prefix, are now known by their full names and are no longer reported as unknown routes. - **Imported `#[Table]` and `#[Connection]` attributes.** A model's table and connection attributes are now read when the attribute is imported with `use`, not only when written fully qualified, in files open in the editor and in `analyze`. Contributed by @shuvroroy. - **Nullable columns stay nullable through a cast.** A model attribute cast to `boolean`, `integer`, a date, an enum or another cast that hands `null` back untouched is now nullable when its column is, and so is a nullable `updated_at` or other timestamp column. A null check on a nullable date column, the soft-delete column included, leaves the date class with all its members. diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index 4b048d853..53bac34d8 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -15,6 +15,7 @@ use App\Http\Requests\StoreBakeryRequest; use App\Http\Requests\UpdateBakeryRequest; use App\Models\Baker; +use App\Models\BakerBuilder; use App\Models\Bakery; use App\Models\BakeryOrder; use App\Models\BlogAuthor; @@ -186,6 +187,14 @@ public function classCasts(CastSample $sample): void // ── Eloquent Query Builder ────────────────────────────────────────────── + /** @return BakerBuilder */ + public function scopedBakers(): BakerBuilder + { + // Try: hover employed() and available(), then complete after either. + // Both scopes keep BakerBuilder, including its active() method. + return Baker::query()->employed()->available()->active(); + } + public function eloquentQuery(): void { // Builder-as-static forwarding diff --git a/examples/laravel/app/Models/Baker.php b/examples/laravel/app/Models/Baker.php index 8408f1930..b365a6335 100644 --- a/examples/laravel/app/Models/Baker.php +++ b/examples/laravel/app/Models/Baker.php @@ -3,10 +3,24 @@ namespace App\Models; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\Builder; +use Illuminate\Database\Eloquent\Attributes\Scope; use Illuminate\Database\Eloquent\Attributes\UseEloquentBuilder; +/** @method static BakerBuilder|Baker employed() */ #[UseEloquentBuilder(BakerBuilder::class)] class Baker extends Model { public function getName(): string { return ''; } + + protected function scopeEmployed(Builder $query): Builder + { + return $query->whereNotNull('hired_at'); + } + + #[Scope] + protected function available(Builder $query): void + { + $query->where('available', true); + } } diff --git a/examples/laravel/app/Models/BakerBuilder.php b/examples/laravel/app/Models/BakerBuilder.php index dfc967a03..e1c16eeb9 100644 --- a/examples/laravel/app/Models/BakerBuilder.php +++ b/examples/laravel/app/Models/BakerBuilder.php @@ -7,6 +7,7 @@ /** * @template TModel of \App\Models\Baker * @extends Builder + * @mixin Baker */ class BakerBuilder extends Builder { diff --git a/examples/laravel/assertions.php b/examples/laravel/assertions.php index 2c0f7ea04..81ba52d52 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -376,6 +376,20 @@ function assertMethodReturnType(string $class, string $method, string $expected) 'BakerBuilder holds the Baker model it queries', $result->getModel() instanceof \App\Models\Baker ); +check( + 'Convention and attribute scopes preserve BakerBuilder and its model', + $result->employed()->available()->active() === $result + && $result->getModel() instanceof \App\Models\Baker +); +check( + 'Static scope forwarding preserves BakerBuilder', + \App\Models\Baker::employed()->available()->active() instanceof \App\Models\BakerBuilder +); +check( + 'The scoped builder demo returns BakerBuilder', + ($scopedBakers = (new \App\Demo())->scopedBakers()) instanceof \App\Models\BakerBuilder + && $scopedBakers->getModel() instanceof \App\Models\Baker +); check( 'Inherited where/orderBy calls preserve the LoafBuilder instance',