From 0d7ce6265f584a279939d6eedb59cda437b9da98 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Sun, 20 Sep 2026 23:41:43 +0600 Subject: [PATCH 1/2] fix(laravel): bind named relation constraint arguments --- docs/CHANGELOG.md | 1 + docs/todo.md | 1 - docs/todo/laravel.md | 21 ---- examples/laravel/app/Demo.php | 3 + examples/laravel/assertions.php | 6 + .../variable/closure_resolution.rs | 10 +- .../forward_walk/callable_inference.rs | 115 +++++++++++------- .../variable/forward_walk/diagnostic_walk.rs | 6 - tests/integration/completion_laravel.rs | 12 ++ .../laravel-builder-relations.php | 17 ++- 10 files changed, 111 insertions(+), 81 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index fd19c3f16..09795fd02 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Named relation callbacks keep their model types.** Completion and diagnostics now resolve relationship constraints when named arguments are reordered or optional arguments are omitted. Contributed by @shuvroroy. - **Relation callbacks keep custom builders.** Constraints on related models now offer their custom builder methods, including through custom-builder query chains and bare `Builder` parameter hints. Contributed by @shuvroroy. - **Model instance queries keep custom builders.** Starting a query with `newQuery()`, `newModelQuery()`, or `newQueryWithoutScopes()` now keeps the model's custom builder and its model type through later calls. Contributed by @shuvroroy. - **`filter_var()` and `filter_input()` return types.** The filter now decides the result, so `filter_var($v, FILTER_VALIDATE_INT, ['options' => ['min_range' => 1]])` is `int<1, max>|false` instead of `mixed`, and `FILTER_NULL_ON_FAILURE` turns the `false` into `null`. diff --git a/docs/todo.md b/docs/todo.md index 5df73eea5..4eb524d76 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -140,7 +140,6 @@ unlikely to move the needle for most users. | L57 | [Laravel queue names](todo/laravel.md#l57-laravel-queue-names) | Low-Medium | Medium | | L59 | [Typed controller middleware names](todo/laravel.md#l59-typed-controller-middleware-names) | Low-Medium | Medium | | L53 | [Collection key types from the column for `keyBy` / `groupBy` / `pluck`](todo/laravel.md#l53-collection-key-types-from-the-column-for-keyby-groupby-pluck) | Medium | Medium | -| L62 | [Bind named relation callback arguments before inference](todo/laravel.md#l62-bind-named-relation-callback-arguments-before-inference) | Medium | Medium | | L49 | [Unguarded Eloquent mass assignment diagnostic](todo/laravel.md#l49-unguarded-eloquent-mass-assignment-diagnostic) | Medium | Medium | | L17 | [Additional string contexts without booting](todo/laravel.md#l17-additional-string-contexts-without-booting) (middleware, assets, validation, Inertia) | Medium | Medium-High | | L63 | [Infer morph constraint callbacks from candidate models](todo/laravel.md#l63-infer-morph-constraint-callbacks-from-candidate-models) | Medium | Medium-High | diff --git a/docs/todo/laravel.md b/docs/todo/laravel.md index 8955b179f..b1c5e806f 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -249,27 +249,6 @@ the collection return-type patches for the argument forms. The property lookup already exists for `model-property`; the work is threading a resolved key type through the generic substitution. -#### L62. Bind named relation callback arguments before inference - -**Impact: Medium · Complexity: Medium** - -`Team::has(relation: 'stocks.warehouse', callback: …)` types the callback -correctly, but putting `callback:` first loses the relation and falls back -to `Builder` in the assertion fixture. Positional callbacks in the -fifth argument already work; the missing step is binding named arguments, -not assuming the first supplied expression names the relation. - -**Reproducer:** The reordered `has(callback: …, relation: …)` assertion -in `relations()` in `tests/phpstan_nsrt/laravel-builder-relations.php`. - -**Where to change:** Relation overrides in -`type_engine/variable/forward_walk/callable_inference.rs` currently receive -the first literal argument from `extract_first_arg_string_fw`. Use the -shared argument binder to locate both the relation and the actual callback -parameter, consistently in the cursor and diagnostics walks. Cover static -and instance calls, arrow functions, omitted defaults, and closures in an -unrelated argument so the override does not attach to the wrong closure. - #### L63. Infer morph constraint callbacks from candidate models **Impact: Medium · Complexity: Medium-High** diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index 3b8db0ee5..6a08d8b75 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -569,6 +569,9 @@ public function eloquentClosure(): void // receive the final related model's builder too. BlogAuthor::has('posts.author', '>=', 1, 'and', fn ($q) => $q->active()); // → Builder + // Named arguments may put the callback before the relation. + BlogAuthor::has(callback: fn ($q) => $q->active(), relation: 'posts.author'); // → Builder + // The related model chooses the builder, including with a bare hint. Bakery::whereHas('baguettes', function (Builder $q) { $q->stale(); // → LoafBuilder diff --git a/examples/laravel/assertions.php b/examples/laravel/assertions.php index d9a8e9c5d..92ca598d3 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -334,6 +334,12 @@ function assertMethodReturnType(string $class, string $method, string $expected) ($query = \App\Models\BlogAuthor::query())->has('posts.author', '>=', 1, 'and', fn ($related) => $related->active()) === $query ); +$namedCallbackModels = []; +\App\Models\BlogAuthor::query()->has(callback: function ($query) use (&$namedCallbackModels) { + $namedCallbackModels[] = get_class($query->active()->getModel()); +}, relation: 'posts.author'); +check('Reordered named relation arguments preserve the callback model', $namedCallbackModels === [\App\Models\BlogAuthor::class]); + // Check both callback invocations without executing the eager-load SQL. $callbackClasses = []; $query = \App\Models\BlogAuthor::withWhereHas('posts', function ($related) use (&$callbackClasses) { diff --git a/src/type_engine/variable/closure_resolution.rs b/src/type_engine/variable/closure_resolution.rs index d3f1ba52e..372748530 100644 --- a/src/type_engine/variable/closure_resolution.rs +++ b/src/type_engine/variable/closure_resolution.rs @@ -883,7 +883,7 @@ fn describes_elements(ty: &PhpType, allow_iterable: bool) -> bool { /// Check whether `method_name` is a relation-query method (e.g. /// `whereHas`, `orWhereHas`, `whereDoesntHave`, etc.) and the receiver /// is an Eloquent model or `Builder`. If so, resolve the -/// relation chain from the first argument string and return +/// relation chain from the bound relation string and return /// the final related model's builder as the closure parameter type. /// /// Returns `None` when the override does not apply (not a relation-query @@ -893,14 +893,14 @@ fn describes_elements(ty: &PhpType, allow_iterable: bool) -> bool { fn try_relation_query_override( receivers: &[ResolvedType], method_name: &str, - first_arg_text: Option<&str>, + relation_name: Option<&str>, class_loader: &dyn Fn(&str) -> Option>, ) -> Option> { if !RELATION_QUERY_METHODS.contains(&method_name) { return None; } - let relation_name = first_arg_text?; + let relation_name = relation_name?; if relation_name.is_empty() { return None; } @@ -1088,10 +1088,10 @@ fn extract_model_from_builder(builder: &ClassInfo) -> Option { pub(in crate::type_engine) fn try_relation_query_override_pub( receivers: &[ResolvedType], method_name: &str, - first_arg_text: Option<&str>, + relation_name: Option<&str>, class_loader: &dyn Fn(&str) -> Option>, ) -> Option> { - try_relation_query_override(receivers, method_name, first_arg_text, class_loader) + try_relation_query_override(receivers, method_name, relation_name, class_loader) } pub(in crate::type_engine) fn build_receiver_self_type_pub( diff --git a/src/type_engine/variable/forward_walk/callable_inference.rs b/src/type_engine/variable/forward_walk/callable_inference.rs index 341cc15b8..f60975fb4 100644 --- a/src/type_engine/variable/forward_walk/callable_inference.rs +++ b/src/type_engine/variable/forward_walk/callable_inference.rs @@ -3,8 +3,6 @@ use std::collections::HashMap; use std::sync::Arc; use mago_span::HasSpan; -use mago_syntax::cst::argument::Argument; -use mago_syntax::cst::sequence::TokenSeparatedSequence; use crate::atom::{atom, bytes_to_str}; use crate::php_type::{PhpType, TypeKind}; @@ -85,7 +83,6 @@ pub(crate) fn infer_callable_params_from_receiver_fw( method_name: &str, arg_idx: usize, argument_list: &ArgumentList<'_>, - first_arg_text: Option<&str>, scope: &ScopeState, ctx: &ForwardWalkCtx<'_>, ) -> Vec { @@ -109,12 +106,9 @@ pub(crate) fn infer_callable_params_from_receiver_fw( // For relation-query methods (whereHas, etc.), override the closure // parameter type with Builder. - if let Some(override_params) = super::super::closure_resolution::try_relation_query_override_pub( - &resolved_types, - method_name, - first_arg_text, - ctx.class_loader, - ) { + if let Some(override_params) = + infer_relation_callback_params(&resolved_types, method_name, arg_idx, argument_list, ctx) + { return override_params; } @@ -380,7 +374,6 @@ pub(crate) fn infer_callable_params_from_static_receiver_fw( method_name: &str, arg_idx: usize, argument_list: &ArgumentList<'_>, - first_arg_text: Option<&str>, scope: &ScopeState, ctx: &ForwardWalkCtx<'_>, ) -> Vec { @@ -398,13 +391,13 @@ pub(crate) fn infer_callable_params_from_static_receiver_fw( if let Some(ref cls) = owner { // For relation-query methods, override with Builder. if crate::virtual_members::laravel::RELATION_QUERY_METHODS.contains(&method_name) - && let Some(override_params) = - super::super::closure_resolution::try_relation_query_override_pub( - &[ResolvedType::from_class(cls.clone())], - method_name, - first_arg_text, - ctx.class_loader, - ) + && let Some(override_params) = infer_relation_callback_params( + &[ResolvedType::from_class(cls.clone())], + method_name, + arg_idx, + argument_list, + ctx, + ) { return override_params; } @@ -598,29 +591,66 @@ pub(crate) fn extract_callable_params_at_fw( .unwrap_or_default() } -/// Extract the text of the first positional argument, stripping quotes. -pub(crate) fn extract_first_arg_string_fw( - arguments: &TokenSeparatedSequence<'_, Argument<'_>>, - content: &str, -) -> Option { - let first = arguments.iter().next()?; - let expr = match first { - Argument::Positional(pos) => pos.value, - Argument::Named(named) => named.value, - }; - let span = expr.span(); - let start = span.start.offset as usize; - let end = span.end.offset as usize; - let raw = content.get(start..end)?.trim(); - - if raw.len() >= 2 - && ((raw.starts_with('\'') && raw.ends_with('\'')) - || (raw.starts_with('"') && raw.ends_with('"'))) - { - Some(raw[1..raw.len() - 1].to_string()) +/// Refine only the closure bound to the relation constraint parameter. +/// Binding against the installed declaration handles reordered named arguments +/// and callbacks after optional parameters without duplicating PHP's call rules. +fn infer_relation_callback_params( + receivers: &[ResolvedType], + method_name: &str, + arg_idx: usize, + argument_list: &ArgumentList<'_>, + ctx: &ForwardWalkCtx<'_>, +) -> Option> { + if !crate::virtual_members::laravel::RELATION_QUERY_METHODS.contains(&method_name) { + return None; + } + let callback_name = if method_name == "whereRelation" { + "column" } else { - None + "callback" + }; + let argument = argument_list.arguments.iter().nth(arg_idx)?.value(); + for receiver in receivers { + let Some(class) = receiver.class_info.as_ref() else { + continue; + }; + let method = class.get_method_arc(method_name).or_else(|| { + crate::virtual_members::resolve_class_fully_maybe_cached( + class, + ctx.class_loader, + ctx.resolved_class_cache, + ) + .get_method_arc(method_name) + }); + let Some(method) = method else { continue }; + let param_index = |name| { + method + .parameters + .iter() + .position(|p| p.name.trim_start_matches('$') == name) + }; + let (Some(relation_idx), Some(callback_idx)) = + (param_index("relation"), param_index(callback_name)) + else { + continue; + }; + let bound = crate::call_args::bind_args_to_params(&method.parameters, argument_list); + if bound[callback_idx].is_none_or(|callback| callback.span() != argument.span()) { + continue; + } + let Expression::Literal(mago_syntax::cst::literal::Literal::String(relation)) = + bound[relation_idx]? + else { + return None; + }; + return super::super::closure_resolution::try_relation_query_override_pub( + receivers, + method_name, + relation.value.map(bytes_to_str), + ctx.class_loader, + ); } + None } /// Seed `$this` in the scope with the enclosing class's type. Callers @@ -680,14 +710,11 @@ pub(crate) fn infer_callable_params_for_call( }; if let Some(ref name) = method_name { let obj_span = mc.object.span(); - let first_arg = - extract_first_arg_string_fw(&mc.argument_list.arguments, ctx.content); infer_callable_params_from_receiver_fw( (obj_span.start.offset, obj_span.end.offset), name, arg_idx, &mc.argument_list, - first_arg.as_deref(), scope, ctx, ) @@ -703,14 +730,11 @@ pub(crate) fn infer_callable_params_for_call( }; if let Some(ref name) = method_name { let obj_span = mc.object.span(); - let first_arg = - extract_first_arg_string_fw(&mc.argument_list.arguments, ctx.content); infer_callable_params_from_receiver_fw( (obj_span.start.offset, obj_span.end.offset), name, arg_idx, &mc.argument_list, - first_arg.as_deref(), scope, ctx, ) @@ -725,14 +749,11 @@ pub(crate) fn infer_callable_params_for_call( None }; if let Some(ref name) = method_name { - let first_arg = - extract_first_arg_string_fw(&sc.argument_list.arguments, ctx.content); infer_callable_params_from_static_receiver_fw( sc.class, name, arg_idx, &sc.argument_list, - first_arg.as_deref(), scope, ctx, ) diff --git a/src/type_engine/variable/forward_walk/diagnostic_walk.rs b/src/type_engine/variable/forward_walk/diagnostic_walk.rs index efa49d35d..31446b6b8 100644 --- a/src/type_engine/variable/forward_walk/diagnostic_walk.rs +++ b/src/type_engine/variable/forward_walk/diagnostic_walk.rs @@ -468,7 +468,6 @@ pub(crate) fn walk_closures_in_call<'b>( None }; let obj_span = mc.object.span(); - let first_arg = extract_first_arg_string_fw(&mc.argument_list.arguments, ctx.content); walk_closures_in_call_args( &mc.argument_list.arguments, Some(call), @@ -481,7 +480,6 @@ pub(crate) fn walk_closures_in_call<'b>( name, arg_idx, &mc.argument_list, - first_arg.as_deref(), outer_scope, ctx, ) @@ -500,7 +498,6 @@ pub(crate) fn walk_closures_in_call<'b>( None }; let obj_span = mc.object.span(); - let first_arg = extract_first_arg_string_fw(&mc.argument_list.arguments, ctx.content); walk_closures_in_call_args( &mc.argument_list.arguments, Some(call), @@ -513,7 +510,6 @@ pub(crate) fn walk_closures_in_call<'b>( name, arg_idx, &mc.argument_list, - first_arg.as_deref(), outer_scope, ctx, ) @@ -531,7 +527,6 @@ pub(crate) fn walk_closures_in_call<'b>( } else { None }; - let first_arg = extract_first_arg_string_fw(&sc.argument_list.arguments, ctx.content); walk_closures_in_call_args( &sc.argument_list.arguments, Some(call), @@ -544,7 +539,6 @@ pub(crate) fn walk_closures_in_call<'b>( name, arg_idx, &sc.argument_list, - first_arg.as_deref(), outer_scope, ctx, ) diff --git a/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index d57d8b6d9..d6e4300d2 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -222,6 +222,12 @@ class Builder { * @return $this */ public function has($relation, $operator = '>=', $count = 1, $boolean = 'and', ?\\Closure $callback = null) { return $this; } + /** + * @param \\Closure(\\Illuminate\\Database\\Eloquent\\Builder): mixed $column + * @return $this + */ + public function whereRelation($relation, $column, $operator = null, $value = null) { return $this; } + /** * @param string $relation * @param (\\Closure(\\Illuminate\\Database\\Eloquent\\Builder): mixed)|null $callback @@ -15373,6 +15379,12 @@ class WarehouseBuilder extends \Illuminate\Database\Eloquent\Builder { "Stock::query()->whereHas('warehouse', function ($q) { BODY });", "Team::query()->where('id', 1)->whereHas('stocks.warehouse', function ($q) { BODY });", "Team::whereHas('stocks.warehouse', function (Builder $q) { BODY });", + "Team::has(callback: function ($q) { BODY }, relation: 'stocks.warehouse');", + "Team::has(callback: fn ($q) => BODY, relation: 'stocks.warehouse');", + "Team::query()->has(callback: fn ($q) => BODY, relation: 'stocks.warehouse');", + "Team::query()?->whereHas(callback: function ($q) { BODY }, relation: 'stocks.warehouse');", + "Stock::whereHas(callback: function (Builder $q) { BODY }, relation: 'warehouse');", + "Team::whereRelation(column: fn ($q) => BODY, relation: 'stocks.warehouse');", ] { let header = "")); diff --git a/tests/phpstan_nsrt/laravel-builder-relations.php b/tests/phpstan_nsrt/laravel-builder-relations.php index cc9c7f04d..de895137b 100644 --- a/tests/phpstan_nsrt/laravel-builder-relations.php +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -298,6 +298,21 @@ function unrelated(UnrelatedQuery $query): void { }); } + + class CustomArguments extends Team { + /** + * @param \Closure(Warehouse): void $operator + * @param \Closure(Builder): void $callback + */ + public static function has($relation, $operator = null, $count = 1, $boolean = 'and', ?\Closure $callback = null) {} + } + + function unrelatedArgument(): void { + CustomArguments::has('stocks', operator: function ($value) { + assertType('BuilderRelationAudit\Warehouse', $value); + }); + } + function relations(): void { Team::whereHas('stocks', function ($query) { assertType('Illuminate\Database\Eloquent\Builder', $query); @@ -331,7 +346,7 @@ function relations(): void { assertType('Illuminate\Database\Eloquent\Builder', $query); }); Team::has(callback: function ($query) { - assertType('Illuminate\Database\Eloquent\Builder', $query); // SKIP: relation argument must currently come first + assertType('Illuminate\Database\Eloquent\Builder', $query); }, relation: 'stocks.warehouse'); Stock::whereHas('team', function ($query) { assertType('BuilderRelationAudit\TeamBuilder', $query); From 0cc67b935d27bde3d7a8e4fab971960c567d51f4 Mon Sep 17 00:00:00 2001 From: Anders Jenbo Date: Mon, 12 Oct 2026 01:14:29 +0200 Subject: [PATCH 2/2] test(laravel): tidy named relation argument fixture Drop a doubled blank line before the CustomArguments class in the builder-relations assertion fixture. Co-Authored-By: Claude Opus 5.5 --- tests/phpstan_nsrt/laravel-builder-relations.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/phpstan_nsrt/laravel-builder-relations.php b/tests/phpstan_nsrt/laravel-builder-relations.php index de895137b..b716a1a26 100644 --- a/tests/phpstan_nsrt/laravel-builder-relations.php +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -298,7 +298,6 @@ function unrelated(UnrelatedQuery $query): void { }); } - class CustomArguments extends Team { /** * @param \Closure(Warehouse): void $operator