diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 30d99b3cf..6522b1131 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -33,11 +33,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Eloquent query examples and inference coverage.** The Laravel playground demonstrates custom builders surviving query chains and callbacks following dotted relationships. An audit against the PHPStan Laravel extensions now guards these behaviours with editor and runtime assertions; the remaining inference gaps are tracked as focused follow-up work. Contributed by @shuvroroy. - **Extract interface is offered only to editors that can create files.** The action writes the new interface to a file of its own, which an editor has to say it accepts (the `create` resource operation) before a server may send it. Editors that do not are no longer shown an action they cannot apply. - **Updated the bundled mago toolchain to 1.47.5.** The parser, docblock parser, formatter, and supporting crates are refreshed to the latest upstream release. Contributed by @nguyentranchung. ### 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 retain 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 retains the model’s custom builder and its model type through subsequent calls. Contributed by @shuvroroy. - **Hover, completion, go-to-definition, signature help, and inlay hints parse the document once per request.** The type engine reads the syntax tree from several places while resolving an expression, and only diagnostics and code actions were sharing one parse between them; every other request re-parsed the whole file once per resolution step, which on a large file made a hover noticeably slower than the diagnostics for the same line. - **A Blade template deleted or renamed on disk no longer keeps its lowered PHP in memory.** The template's generated PHP and source map were only released when the editor closed the file, so a template removed by a rename or a branch switch stayed resident for the rest of the session and kept being visited by every Blade refresh pass. - **A method's unnamed `@param` tags are now matched by position, and its `@param` descriptions now show up in hover.** Both already worked for a standalone function's docblock; a method's own merge was a separate, older implementation that never grew the positional fallback (common in phpstorm-stubs-style docs, e.g. `@param callable(TValue, TKey): bool` with no `$callback`) and never copied the description across at all. diff --git a/docs/todo.md b/docs/todo.md index bbafd6b3b..cb0251127 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -161,7 +161,8 @@ unlikely to move the needle for most users. | L32 | [Config-backed named-resource strings](todo/laravel.md#l32-config-backed-named-resource-strings) (log channels, cache stores, guards, connections, rate limiters) | 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 | -| L54 | [Audit custom-builder and relation-closure inference against the PHPStan extensions](todo/laravel.md#l54-audit-custom-builder-and-relation-closure-inference-against-the-phpstan-extensions) | Medium | Medium-High | +| L58 | [Infer morph constraint callbacks from candidate models](todo/laravel.md#l58-infer-morph-constraint-callbacks-from-candidate-models) | Medium | Medium-High | +| L59 | [Infer both callback receivers for withWhereHas](todo/laravel.md#l59-infer-both-callback-receivers-for-withwherehas) | Medium | Medium-High | | L31 | [String-key rename, highlight, and semantic tokens](todo/laravel.md#l31-string-key-rename-highlight-and-semantic-tokens) | Low-Medium | Medium | | L42 | [Morph alias completion in array positions](todo/laravel.md#l42-morph-alias-completion-in-array-positions) | Low-Medium | Medium | | L3 | `$dates` array (deprecated) | Low-Medium | Medium | diff --git a/docs/todo/laravel.md b/docs/todo/laravel.md index 1bb754d25..daa0912c0 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -247,35 +247,56 @@ 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. -#### L54. Audit custom-builder and relation-closure inference against the PHPStan extensions +#### L58. Infer morph constraint callbacks from candidate models **Impact: Medium · Complexity: Medium-High** -Two areas where the PHPStan Laravel extensions have moved past what we -mirror, and where we have machinery that has not been checked against -them: - -- **A custom builder surviving the chain.** We read - `newEloquentBuilder()` (`virtual_members/laravel/model_extraction.rs`) - and inject the builder, but it is not established that - `Team::query()->where(…)->orderBy(…)` stays on `TeamBuilder` rather than - degrading to `Builder` at the first inherited call, nor that - static calls on the model and instance calls on the builder agree about - what comes back. -- **Relation-constraint closure parameters.** We type closures for the - `whereHas` family (`type_engine/variable/closure_resolution.rs`, - `forward_walk/callable_inference.rs`). Unverified: dotted relation paths - (`whereHas('stocks.warehouse', …)` should type the closure for - `Warehouse`'s builder, resolving each segment against the model the - previous one named), the `*Morph` variants' union of candidate builders - plus their `$type` parameter, `withWhereHas` receiving both a builder - and the relation, and closures in non-leading argument positions - (`has('stocks', '>=', 1, 'and', fn ($q) => …)`). - -**Where to change:** Write the assertion cases first — the existing -`tests/integration/completion_laravel.rs` conventions cover both areas — -and file what actually fails. Splitting this into concrete items once the -gaps are known is preferable to a broad rewrite of either subsystem. +The `whereHasMorph` family does not participate in relation-query +inference. A literal model class or an array of candidates yields +`Builder` in the assertion fixture, losing both the related +models and their custom builders. The second `$type` callback parameter +already resolves to `string` from Laravel's declaration and must stay +typed when the first parameter is refined. + +**Reproducer:** The `morphs()` assertions in +`tests/phpstan_nsrt/laravel-builder-relations.php` cover all six +callback-taking existence methods, single and multiple candidates, +reordered named arguments, wildcard and empty-list fallbacks, and a +declared `MorphTo` target. The desired builder types remain in the +`SKIP` assertions; the `$type` assertions run normally. + +**Where to change:** Extend shared callable inference using bound +`relation`, `types`, and `callback` arguments. Build a union of candidate +builders, preserving custom builders. For unresolved/wildcard candidates, +use the relation's declared target, falling back to `Model` when that is +all the declaration knows. Also cover class-string variables and mixed +known/unknown candidate arrays, as +[Larastan's callback extension](https://github.com/larastan/larastan/blob/c328727e6103c1147d1c64cc96b3aedfda26bc20/src/ClosureTypes/RelationshipQueryCallbackExtension.php) +does. Do not boot the application or query the database for wildcard +discovery. Check the `whereMorphRelation` shortcuts when sharing the +callback logic. + +#### L59. Infer both callback receivers for withWhereHas + +**Impact: Medium · Complexity: Medium-High** + +`withWhereHas('stocks', …)` calls the constraint with `Builder` for +the existence query and `HasMany` for eager loading. We infer +only the builder. Dotted paths likewise lose the final relation type, and +the `stocks:id` column-selection form fails relation lookup entirely, +falling back to unbound `Builder|Relation`. + +**Reproducer:** The `eagerRelations()` assertions in +`tests/phpstan_nsrt/laravel-builder-relations.php`. The Laravel runtime +assertions also verify both invocations without executing eager-load SQL. + +**Where to change:** Extend the shared relation-chain resolver to retain +the terminal instantiated relation and its declaring model, then use the +builder/relation union for this callback. Strip the column suffix where +Laravel accepts it, preserve custom builders, and bind named arguments. +The callback's ordinary fluent methods must preserve that union. Cover +explicit `Builder|Relation` hints and the `withWhereRelation` counterpart; +ordinary `whereHas` must continue to receive just the related builder. #### L45. `*_count` properties are offered on every relationship diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index 59ed52562..879ecb288 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -27,6 +27,7 @@ use Database\Factories\BlogAuthorFactory; use Database\Factories\EditorialFactory; use Illuminate\Contracts\Filesystem\Filesystem; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Http\Client\Factory as HttpFactory; use Illuminate\Http\Client\PendingRequest; @@ -180,6 +181,14 @@ public function eloquentQuery(): void Loaf::query()->stale()->get(); // → Collection Baker::query()->active()->firstOrFail()->getName(); // → Baker + // Inherited methods and forwarded query methods keep the custom builder. + Loaf::query()->where('crust', 'sourdough')->orderBy('id')->stale(); // → LoafBuilder + Loaf::where('crust', 'sourdough')->orderBy('id')->stale(); // → LoafBuilder + Baker::query()->whereIn('id', [1])->lockForUpdate()->active(); // → BakerBuilder + (new Loaf())->newQuery()->orderBy('id')->stale(); // → LoafBuilder + (new Baker())->newQueryWithoutScopes()->active(); // → BakerBuilder + (new Baker())->newModelQuery()->firstOrFail()->getName(); // → Baker + // Paginators carry the model element type through foreach foreach (BlogAuthor::where('active', 1)->paginate() as $author) { $author->profile->getBio(); // → BlogAuthor @@ -464,10 +473,23 @@ public function eloquentClosure(): void $query->where('published', true); // resolves to Builder }); - // Dot-notation relation chain - BlogPost::whereHas('author', function ($q) { - $q->where('active', true); // resolves to Builder + // Each dotted segment is resolved on the preceding related model. + BlogPost::whereHas('author.posts', function ($q) { + $q->where('published', true); // resolves to Builder + }); + + // has() takes its callback in the fifth argument; arrow functions + // 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 }); + Bakery::query()->whereHas('headBaker', fn ($q) => $q->active()); // → BakerBuilder } diff --git a/examples/laravel/assertions.php b/examples/laravel/assertions.php index 9e5178ff1..6a6808858 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -196,6 +196,96 @@ function assertMethodReturnType(string $class, string $method, string $expected) $result->getModel() instanceof \App\Models\Baker ); +check( + 'Inherited where/orderBy calls preserve the LoafBuilder instance', + ($query = \App\Models\Loaf::query())->where('crust', 'sourdough')->orderBy('id')->stale() === $query +); +check( + 'Static where/orderBy forwarding returns LoafBuilder', + \App\Models\Loaf::where('crust', 'sourdough')->orderBy('id')->stale() instanceof \App\Models\LoafBuilder +); +check( + 'Query-builder forwarding preserves BakerBuilder and its model', + ($query = \App\Models\Baker::query())->whereIn('id', [1])->lockForUpdate()->active() === $query + && $query->getModel() instanceof \App\Models\Baker +); + +foreach (['newQuery', 'newModelQuery', 'newQueryWithoutScopes'] as $factory) { + $loafQuery = (new \App\Models\Loaf())->$factory(); + $bakerQuery = (new \App\Models\Baker())->$factory(); + check( + "Model::$factory preserves custom builders and their models", + $loafQuery instanceof \App\Models\LoafBuilder + && $loafQuery->stale()->getModel() instanceof \App\Models\Loaf + && $bakerQuery instanceof \App\Models\BakerBuilder + && $bakerQuery->active()->getModel() instanceof \App\Models\Baker + ); +} + +$callbackModels = []; +\App\Models\BlogPost::whereHas('author.posts', function ($query) use (&$callbackModels) { + $callbackModels[] = get_class($query->getModel()); + $query->where('published', true); +}); +check( + 'Dotted whereHas callback queries the last related model', + $callbackModels === [\App\Models\BlogPost::class] +); + +$relatedBuilders = []; +\App\Models\Bakery::whereHas('baguettes', function (\Illuminate\Database\Eloquent\Builder $query) use (&$relatedBuilders) { + $relatedBuilders[] = get_class($query->stale()); +}); +\App\Models\Bakery::query()->whereHas('headBaker', function ($query) use (&$relatedBuilders) { + $relatedBuilders[] = get_class($query->active()); +}); +check( + 'Relation constraints select each related model custom builder', + $relatedBuilders === [\App\Models\LoafBuilder::class, \App\Models\BakerBuilder::class] +); + +$callbackModels = []; +\App\Models\BlogAuthor::has('posts.author', '>=', 1, 'and', function ($query) use (&$callbackModels) { + $callbackModels[] = get_class($query->active()->getModel()); +}); +check( + 'The fifth has argument receives the final related model builder and scopes', + $callbackModels === [\App\Models\BlogAuthor::class] +); +check( + 'An arrow callback in the fifth has argument preserves the outer query', + ($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) { + $callbackClasses[] = get_class($related); +}); +$query->getEagerLoads()['posts']((new \App\Models\BlogAuthor())->posts()); +check( + 'withWhereHas supplies a builder for existence and a relation for eager loading', + $callbackClasses === [\Illuminate\Database\Eloquent\Builder::class, \Illuminate\Database\Eloquent\Relations\HasMany::class] +); + +$morphCallbacks = []; +\App\Models\Review::whereHasMorph('reviewable', [\App\Models\BlogPost::class, \App\Models\Loaf::class], function ($related, $type) use (&$morphCallbacks) { + $morphCallbacks[] = [get_class($related), get_class($related->getModel()), $type]; +}); +check( + 'Morph callbacks receive each candidate builder, its model, and the class-string type', + $morphCallbacks === [ + [\Illuminate\Database\Eloquent\Builder::class, \App\Models\BlogPost::class, \App\Models\BlogPost::class], + [\App\Models\LoafBuilder::class, \App\Models\Loaf::class, \App\Models\Loaf::class], + ] +); + // Model::fresh() on instance (non-existing model returns null) $result = $bakery->fresh(); check( diff --git a/src/type_engine/variable/closure_resolution.rs b/src/type_engine/variable/closure_resolution.rs index ac2b606c6..af09d907a 100644 --- a/src/type_engine/variable/closure_resolution.rs +++ b/src/type_engine/variable/closure_resolution.rs @@ -22,7 +22,8 @@ use crate::atom::{atom, bytes_to_str}; use crate::php_type::{PhpType, TypeKind}; use crate::type_engine::resolver::ResolutionCtx; use crate::virtual_members::laravel::{ - ELOQUENT_BUILDER_FQN, RELATION_QUERY_METHODS, extends_eloquent_model, resolve_relation_chain, + ELOQUENT_BUILDER_FQN, RELATION_QUERY_METHODS, extends_eloquent_model, model_builder_type, + resolve_relation_chain, }; thread_local! { @@ -729,7 +730,7 @@ fn resolve_closure_this_type( /// Check whether the inferred callable-signature type is a more specific /// version of the explicit type hint. /// -/// Returns `true` in two situations: +/// Returns `true` when generic or element information refines the hint: /// /// * The explicit hint is a bare class name (e.g. `Collection`) and the /// inferred type is the same class with generic arguments (e.g. @@ -742,7 +743,13 @@ fn resolve_closure_this_type( /// `list`, `User[]`). A closure passed to `array_map()` over /// `list` may declare its parameter as plain /// `array`; the call site still knows the shape. -fn inferred_type_is_more_specific(explicit_hint: &PhpType, inferred: &PhpType) -> bool { +/// * The inferred generic class is a subclass of the explicit bare class, +/// such as a custom Eloquent builder passed to a `Builder` parameter. +fn inferred_type_is_more_specific( + explicit_hint: &PhpType, + inferred: &PhpType, + class_loader: &dyn Fn(&str) -> Option>, +) -> bool { // The explicit hint must be a bare name (no generic args). let explicit_base = match explicit_hint.kind() { TypeKind::Named(name) => name.as_str(), @@ -767,6 +774,13 @@ fn inferred_type_is_more_specific(explicit_hint: &PhpType, inferred: &PhpType) - let inferred_short = crate::util::short_name(inferred_base); explicit_short.eq_ignore_ascii_case(inferred_short) + || class_loader(explicit_base).is_some_and(|explicit| { + crate::class_lookup::is_subtype_of_typed( + inferred, + &PhpType::named(explicit.fqn()), + class_loader, + ) + }) } /// Whether an array type carries element information worth keeping over a @@ -794,24 +808,24 @@ 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 -/// `Builder` as the closure parameter type. +/// 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 /// method, receiver is not a model, relation chain cannot be resolved), /// in which case the caller falls through to normal callable param /// inference. fn try_relation_query_override( - receiver_classes: &[Arc], + 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; } @@ -819,15 +833,13 @@ fn try_relation_query_override( // Determine the base model from the receiver. The receiver may be // the model itself (static call: `Brand::whereHas(...)`) or a // `Builder` instance. - let model = find_model_from_receivers(receiver_classes, class_loader)?; + let model = find_model_from_receivers(receivers, class_loader)?; // Walk the dot-separated relation chain to find the final related model. let related_fqn = resolve_relation_chain(&model, relation_name, class_loader, None)?; - let builder_type = PhpType::generic( - ELOQUENT_BUILDER_FQN, - vec![PhpType::named(atom(&related_fqn))], - ); + let related = class_loader(&related_fqn)?; + let builder_type = model_builder_type(&related, class_loader); Some(vec![builder_type]) } @@ -925,30 +937,46 @@ fn extract_generic_args_from_methods(class: &ClassInfo, class_fqn: &str) -> Opti None } -/// Given a list of receiver classes, find the underlying Eloquent model: +/// Given resolved receivers, find the underlying Eloquent model: /// the receiver itself when it is a model class, or the model extracted /// from a `Builder` receiver. fn find_model_from_receivers( - receiver_classes: &[Arc], + receivers: &[ResolvedType], class_loader: &dyn Fn(&str) -> Option>, ) -> Option> { - for cls in receiver_classes { - // Direct model class. + for receiver in receivers { + let Some(cls) = receiver.class_info.as_ref() else { + continue; + }; if extends_eloquent_model(cls, class_loader) { return Some(Arc::clone(cls)); } - - // Builder — extract the model name from the Builder's - // method return types. After generic substitution, methods like - // `where()` return `Builder`, so we can extract `Brand` - // from any method's return type that is `Builder`. - let cls_fqn = cls.fqn(); - if (cls.name == "Builder" || cls_fqn == ELOQUENT_BUILDER_FQN) - && let Some(model_type) = extract_model_from_builder(cls) - && let Some(model_cls) = model_type.base_name().and_then(class_loader) - && extends_eloquent_model(&model_cls, class_loader) + if cls.fqn() != ELOQUENT_BUILDER_FQN + && !crate::virtual_members::laravel::helpers::extends_eloquent_builder( + cls, + class_loader, + ) + { + continue; + } + // Read the receiver's retained generic context before recovering it + // from declarations; custom builders need not declare TModel at all. + let model_type = match receiver.type_string.kind() { + TypeKind::Generic(g) => g.args.first().cloned(), + _ => None, + } + .or_else(|| { + cls.get_method("getModel") + .and_then(|method| method.return_type.clone()) + }) + .or_else(|| extract_model_from_builder(cls)); + if let Some(model) = model_type + .as_ref() + .and_then(PhpType::base_name) + .and_then(class_loader) + && extends_eloquent_model(&model, class_loader) { - return Some(model_cls); + return Some(model); } } None @@ -979,12 +1007,12 @@ fn extract_model_from_builder(builder: &ClassInfo) -> Option { // can perform callable parameter inference without duplicating the logic. pub(in crate::type_engine) fn try_relation_query_override_pub( - receiver_classes: &[Arc], + 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(receiver_classes, 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( @@ -997,6 +1025,7 @@ pub(in crate::type_engine) fn build_receiver_self_type_pub( pub(in crate::type_engine) fn inferred_type_is_more_specific_pub( explicit_hint: &PhpType, inferred: &PhpType, + class_loader: &dyn Fn(&str) -> Option>, ) -> bool { - inferred_type_is_more_specific(explicit_hint, inferred) + inferred_type_is_more_specific(explicit_hint, inferred, class_loader) } diff --git a/src/type_engine/variable/forward_walk/callable_inference.rs b/src/type_engine/variable/forward_walk/callable_inference.rs index 014b9cf6a..c3431d773 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( - &receiver_classes, - 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; } @@ -336,7 +330,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 { @@ -353,14 +346,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 let Some(override_params) = - super::super::closure_resolution::try_relation_query_override_pub( - &[Arc::new(cls.clone())], - method_name, - first_arg_text, - ctx.class_loader, - ) - { + if let Some(override_params) = infer_relation_callback_params( + &[ResolvedType::from_class(cls.clone())], + method_name, + arg_idx, + argument_list, + ctx, + ) { return override_params; } @@ -527,29 +519,66 @@ pub(crate) fn extract_callable_params_at_fw( vec![] } -/// 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 @@ -609,14 +638,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, ) @@ -632,14 +658,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, ) @@ -654,14 +677,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 a3074a22f..6ff29d84d 100644 --- a/src/type_engine/variable/forward_walk/diagnostic_walk.rs +++ b/src/type_engine/variable/forward_walk/diagnostic_walk.rs @@ -389,7 +389,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, outer_scope, ctx, |arg_idx| { if let Some(ref name) = method_name { infer_callable_params_from_receiver_fw( @@ -397,7 +396,6 @@ pub(crate) fn walk_closures_in_call<'b>( name, arg_idx, &mc.argument_list, - first_arg.as_deref(), outer_scope, ctx, ) @@ -415,7 +413,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, outer_scope, ctx, |arg_idx| { if let Some(ref name) = method_name { infer_callable_params_from_receiver_fw( @@ -423,7 +420,6 @@ pub(crate) fn walk_closures_in_call<'b>( name, arg_idx, &mc.argument_list, - first_arg.as_deref(), outer_scope, ctx, ) @@ -440,7 +436,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, outer_scope, ctx, |arg_idx| { if let Some(ref name) = method_name { infer_callable_params_from_static_receiver_fw( @@ -448,7 +443,6 @@ pub(crate) fn walk_closures_in_call<'b>( name, arg_idx, &sc.argument_list, - first_arg.as_deref(), outer_scope, ctx, ) @@ -663,7 +657,11 @@ pub(crate) fn seed_closure_params( let use_inferred_over_explicit = if let Some(ref eff) = effective_type && let Some(inferred) = inferred_for_idx { - super::super::closure_resolution::inferred_type_is_more_specific_pub(eff, inferred) + super::super::closure_resolution::inferred_type_is_more_specific_pub( + eff, + inferred, + ctx.class_loader, + ) } else { false }; diff --git a/src/virtual_members/laravel/builder.rs b/src/virtual_members/laravel/builder.rs index e64b4ee1b..3ead213d9 100644 --- a/src/virtual_members/laravel/builder.rs +++ b/src/virtual_members/laravel/builder.rs @@ -16,53 +16,65 @@ use std::sync::Arc; use crate::inheritance::apply_substitution_to_conditional; -use crate::php_type::PhpType; +use crate::php_type::{PhpType, TypeKind}; use crate::types::{ClassInfo, MAX_INHERITANCE_DEPTH, MethodInfo, Visibility}; use crate::virtual_members::ResolvedClassCache; use super::ELOQUENT_BUILDER_FQN; -/// Build static virtual methods by forwarding Eloquent Builder's public -/// instance methods onto the model class. See the module docs for the -/// return-type mapping. -pub(super) fn build_builder_forwarded_methods( - class: &ClassInfo, +/// Select a model's builder using the same inherited configuration as +/// static forwarding. Missing custom classes fall back to Eloquent's builder. +fn model_builder_class( + model: &ClassInfo, class_loader: &dyn Fn(&str) -> Option>, - cache: Option<&ResolvedClassCache>, -) -> Vec> { - // Walk the parent chain to find a custom builder definition. - // Laravel's #[UseEloquentBuilder] and HasBuilder are effectively inherited. - let mut requested_builder_fqn = ELOQUENT_BUILDER_FQN.to_string(); - let mut current = Some(class.clone()); +) -> Option> { + let mut current = crate::inheritance::ClassRef::Borrowed(model); for _ in 0..MAX_INHERITANCE_DEPTH { - let Some(curr) = current else { break }; - if let Some(name) = curr + if let Some(name) = current .laravel() - .and_then(|l| l.custom_builder.as_ref()) - .and_then(|b| b.base_name()) + .and_then(|metadata| metadata.custom_builder.as_ref()) + .and_then(PhpType::base_name) { - requested_builder_fqn = name.to_string(); - break; + return class_loader(name).or_else(|| class_loader(ELOQUENT_BUILDER_FQN)); } - current = curr + let Some(parent) = current .parent_class .as_ref() - .and_then(|p| class_loader(p)) - .map(Arc::unwrap_or_clone); + .and_then(|name| class_loader(name)) + else { + break; + }; + current = crate::inheritance::ClassRef::Owned(parent); } + class_loader(ELOQUENT_BUILDER_FQN) +} - // Load the Eloquent Builder class (or custom builder). - let (builder_class, builder_fqn) = match class_loader(&requested_builder_fqn) { - Some(c) => (c, requested_builder_fqn), - // Fallback to standard builder if custom builder fails to load. - None if requested_builder_fqn != ELOQUENT_BUILDER_FQN => { - match class_loader(ELOQUENT_BUILDER_FQN) { - Some(c) => (c, ELOQUENT_BUILDER_FQN.to_string()), - None => return Vec::new(), - } - } - None => return Vec::new(), +/// The concrete query type constructed for a model, including custom builders +/// that do not declare their own template parameter. +pub(crate) fn model_builder_type( + model: &ClassInfo, + class_loader: &dyn Fn(&str) -> Option>, +) -> PhpType { + let builder = model_builder_class(model, class_loader); + let name = builder.as_ref().map_or_else( + || crate::atom::atom(ELOQUENT_BUILDER_FQN), + |builder| builder.fqn(), + ); + PhpType::generic_atom(name, vec![PhpType::named(model.fqn())]) +} + +/// Build static virtual methods by forwarding Eloquent Builder's public +/// instance methods onto the model class. See the module docs for the +/// return-type mapping. +pub(super) fn build_builder_forwarded_methods( + class: &ClassInfo, + class_loader: &dyn Fn(&str) -> Option>, + cache: Option<&ResolvedClassCache>, +) -> Vec> { + let Some(builder_class) = model_builder_class(class, class_loader) else { + return Vec::new(); }; + let builder_fqn = builder_class.fqn(); // Fully resolve Builder (own + traits + parents + virtual members // including @mixin Query\Builder). This is safe because Builder @@ -82,7 +94,7 @@ pub(super) fn build_builder_forwarded_methods( // Build a substitution map: TModel → concrete model class name, // and static/$this/self → Builder. - let builder_self_type = PhpType::generic(&builder_fqn, vec![PhpType::named(class.fqn())]); + let builder_self_type = PhpType::generic(builder_fqn, vec![PhpType::named(class.fqn())]); let mut subs = super::self_ref_subs(builder_self_type.clone()); insert_builder_template_substitutions( &mut subs, @@ -163,14 +175,42 @@ pub(super) fn build_builder_forwarded_methods( methods.push(forwarded); } - // ── query() / newQuery() / newModelQuery() ────────────────────── - // When a model has a custom builder, User::query() should return - // UserBuilder instead of the default Builder. - for name in ["query", "newQuery", "newModelQuery"] { - methods.push(Arc::new(MethodInfo { - is_static: true, - ..MethodInfo::virtual_method_typed(name, Some(&builder_self_type)) - })); + let query_fp = crate::virtual_members::TransformFingerprint::new( + Some(&subs), + Some("model-query-factory"), + 0, + ); + for name in [ + "query", + "newQuery", + "newModelQuery", + "newQueryWithoutScopes", + ] { + if let Some(original) = class.get_method_arc(name) { + // A user override returning a different builder owns its type. + // Refine only the framework's base-builder declaration, keeping + // its parameters, visibility and instance/static distinction. + if let Some(ret) = original.return_type.as_ref() + && (ret.base_name() != Some(ELOQUENT_BUILDER_FQN) + || matches!(ret.kind(), TypeKind::Generic(g) if g.args.first().is_some_and(|model| !model.is_self_ref()))) + { + continue; + } + methods.push(crate::virtual_members::intern_transformed_method( + &original, + query_fp, + || { + let mut method = (*original).clone(); + method.return_type = Some(builder_self_type.clone()); + method + }, + )); + } else { + methods.push(Arc::new(MethodInfo { + is_static: name == "query", + ..MethodInfo::virtual_method_typed(name, Some(&builder_self_type)) + })); + } } methods diff --git a/src/virtual_members/laravel/builder_tests.rs b/src/virtual_members/laravel/builder_tests.rs index 4b54295a0..96f22f4ea 100644 --- a/src/virtual_members/laravel/builder_tests.rs +++ b/src/virtual_members/laravel/builder_tests.rs @@ -16,6 +16,49 @@ fn make_builder(methods: Vec) -> ClassInfo { // ── build_builder_forwarded_methods ───────────────────────────── +#[test] +fn model_query_factories_preserve_declaration_metadata() { + let builder = Arc::new(make_builder(vec![])); + let mut model = make_class("App\\Team"); + let mut original = make_method_with_params( + "newQueryWithoutScopes", + Some(ELOQUENT_BUILDER_FQN), + vec![make_param("$connection", Some("string"), false)], + ); + original.is_static = false; + original.is_virtual = false; + original.name_offset = 42; + original.native_return_type = original.return_type.clone(); + model.methods.push(Arc::new(original)); + let loader = |name: &str| (name == ELOQUENT_BUILDER_FQN).then(|| Arc::clone(&builder)); + let methods = build_builder_forwarded_methods(&model, &loader, None); + let factory = methods + .iter() + .find(|m| m.name == "newQueryWithoutScopes") + .unwrap(); + assert!(!factory.is_static); + assert!(!factory.is_virtual); + assert_eq!(factory.name_offset, 42); + assert_eq!(factory.parameters[0].name, "$connection"); + assert_eq!( + factory.native_return_type, + Some(PhpType::named(atom(ELOQUENT_BUILDER_FQN))) + ); + assert_eq!( + factory.return_type, + Some(PhpType::generic( + ELOQUENT_BUILDER_FQN, + vec![PhpType::named(atom("App\\Team"))] + )) + ); + for name in ["query", "newQuery", "newModelQuery"] { + assert_eq!( + methods.iter().find(|m| m.name == name).unwrap().is_static, + name == "query" + ); + } +} + #[test] fn builder_forwarding_returns_empty_when_builder_not_found() { let class = make_class("App\\Models\\User"); @@ -39,8 +82,8 @@ fn builder_forwarding_converts_instance_to_static() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - // 1 real + 3 synthesized (query, newQuery, newModelQuery) - assert_eq!(result.len(), 4); + // 1 real + 4 synthesized query factories + assert_eq!(result.len(), 5); assert!(result[0].is_static, "Forwarded method should be static"); assert_eq!(result[0].name, "where"); } @@ -59,7 +102,7 @@ fn builder_forwarding_maps_static_to_builder_self_type() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 4); + assert_eq!(result.len(), 5); assert_eq!( result[0].return_type_str().as_deref(), Some("Illuminate\\Database\\Eloquent\\Builder"), @@ -81,7 +124,7 @@ fn builder_forwarding_maps_this_to_builder_self_type() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 4); + assert_eq!(result.len(), 5); assert_eq!( result[0].return_type_str().as_deref(), Some("Illuminate\\Database\\Eloquent\\Builder"), @@ -103,7 +146,7 @@ fn builder_forwarding_maps_self_to_builder_self_type() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 4); + assert_eq!(result.len(), 5); assert_eq!( result[0].return_type_str().as_deref(), Some("Illuminate\\Database\\Eloquent\\Builder"), @@ -125,7 +168,7 @@ fn builder_forwarding_maps_tmodel_to_concrete_class() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 4); + assert_eq!(result.len(), 5); assert_eq!( result[0].return_type_str().as_deref(), Some("App\\Models\\User|null"), @@ -213,7 +256,7 @@ fn builder_forwarding_maps_generic_collection_return() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 4); + assert_eq!(result.len(), 5); assert_eq!( result[0].return_type_str().as_deref(), Some("Illuminate\\Database\\Eloquent\\Collection"), @@ -235,7 +278,7 @@ fn builder_forwarding_maps_static_in_union() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 4); + assert_eq!(result.len(), 5); assert_eq!( result[0].return_type_str().as_deref(), Some("Illuminate\\Database\\Eloquent\\Builder|null"), @@ -263,7 +306,7 @@ fn builder_forwarding_skips_magic_methods() { let result = build_builder_forwarded_methods(&user, &loader, None); assert_eq!( result.len(), - 4, + 5, "Only non-magic methods should be forwarded" ); assert_eq!(result[0].name, "where"); @@ -287,7 +330,7 @@ fn builder_forwarding_skips_non_public_methods() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 4, "Only public methods should be forwarded"); + assert_eq!(result.len(), 5, "Only public methods should be forwarded"); assert_eq!(result[0].name, "where"); } @@ -314,7 +357,7 @@ fn builder_forwarding_skips_methods_already_on_model() { let result = build_builder_forwarded_methods(&user, &loader, None); assert_eq!( result.len(), - 4, + 5, "Should skip 'myMethod' because the model already has it as static" ); assert_eq!(result[0].name, "where"); @@ -342,7 +385,7 @@ fn builder_forwarding_does_not_skip_instance_method_with_same_name() { let result = build_builder_forwarded_methods(&user, &loader, None); assert_eq!( result.len(), - 4, + 5, "Static forwarded method should be added even when an instance method with the same name exists" ); assert!(result[0].is_static); @@ -366,7 +409,7 @@ fn builder_forwarding_maps_parameter_types() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 4); + assert_eq!(result.len(), 5); assert_eq!( result[0].parameters[0].type_hint_str().as_deref(), Some("App\\Models\\User"), @@ -398,7 +441,7 @@ fn builder_forwarding_preserves_method_metadata() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 4); + assert_eq!(result.len(), 5); assert!( result[0].deprecation_message.is_some(), "Deprecated flag should be preserved" @@ -430,13 +473,13 @@ fn builder_forwarding_multiple_methods() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 7); + assert_eq!(result.len(), 8); let names: Vec<&str> = result.iter().map(|m| m.name.as_str()).collect(); assert!(names.contains(&"where")); assert!(names.contains(&"orderBy")); assert!(names.contains(&"get")); assert!(names.contains(&"first")); - assert!(result.iter().all(|m| m.is_static)); + assert!(result[..4].iter().all(|m| m.is_static)); } #[test] @@ -507,7 +550,7 @@ fn builder_forwarding_with_no_return_type() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 4); + assert_eq!(result.len(), 5); assert!( result[0].return_type.is_none(), "None return type should stay None" @@ -531,7 +574,7 @@ fn builder_forwarding_preserves_non_template_return_types() { }; let result = build_builder_forwarded_methods(&user, &loader, None); - assert_eq!(result.len(), 5); + assert_eq!(result.len(), 6); assert_eq!(result[0].return_type_str().as_deref(), Some("string")); assert_eq!(result[1].return_type_str().as_deref(), Some("bool")); } diff --git a/src/virtual_members/laravel/mod.rs b/src/virtual_members/laravel/mod.rs index 39deac10b..8e85d473e 100644 --- a/src/virtual_members/laravel/mod.rs +++ b/src/virtual_members/laravel/mod.rs @@ -232,6 +232,7 @@ use std::collections::HashMap; use std::sync::Arc; use builder::build_builder_forwarded_methods; +pub(crate) use builder::model_builder_type; use casts::cast_type_to_php_type; pub use facade::LaravelFacadeProvider; pub use factory::LaravelFactoryProvider; diff --git a/src/virtual_members/mod.rs b/src/virtual_members/mod.rs index 6b0f69314..fa33280c8 100644 --- a/src/virtual_members/mod.rs +++ b/src/virtual_members/mod.rs @@ -157,7 +157,8 @@ pub trait VirtualMemberProvider { /// first `$query` parameter stripped, which is what callers actually see. /// The `query` / `newQuery` / `newModelQuery` methods are also replaced, /// so a model with a custom builder returns that builder rather than the -/// base one from the framework declaration. +/// base one from the framework declaration. The Laravel provider also +/// refines the real `newQueryWithoutScopes` declaration in place. /// /// Properties are deduplicated by name. When a property with the same /// name already exists, the **more specific** type wins regardless of @@ -199,6 +200,7 @@ pub fn merge_virtual_members(class: &mut ClassInfo, virtual_members: VirtualMemb if let Some(&idx) = method_index.get(&key) { if class.methods[idx].has_scope_attribute || matches!(method.name.as_str(), "query" | "newQuery" | "newModelQuery") + || (method.name == "newQueryWithoutScopes" && !method.is_virtual) { // Replace the original with the synthesized virtual method. // For scope attributes, the original is an implementation detail. diff --git a/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index 2848f2746..9ab0f15a9 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -213,6 +213,19 @@ class Builder { public function exists(): bool { return false; } /** @return string */ public function toSql(): string { return ''; } + /** + * @template TRelatedModel of \\Illuminate\\Database\\Eloquent\\Model + * @param \\Illuminate\\Database\\Eloquent\\Relations\\Relation|string $relation + * @param (\\Closure(\\Illuminate\\Database\\Eloquent\\Builder): mixed)|null $callback + * @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 @@ -2463,6 +2476,83 @@ class User extends Model { ); } +/// Exercise the custom builder after inherited and query-builder mixin calls, +/// through both the model's static forwarding and an instance query. +#[tokio::test] +async fn test_builder_audit_custom_chain_completion() { + for generics in [ + "", + "/** @template TModel of Model\n * @extends Builder */", + ] { + let builder = format!( + "where('active', true)->orderBy('id')", + "Team::where('active', true)->orderBy('id')", + "Team::active()->whereIn('id', [1])->lockForUpdate()", + "(new Team())->newQuery()->where('active', true)->orderBy('id')", + "(new Team())->newModelQuery()->orderBy('id')", + "(new Team())->newQueryWithoutScopes()->active()", + ] { + let content = format!( + "active();\n$query->" + ); + let position = crate::common::position_after(&content, "\n$query->active();\n$query->"); + let items = complete_at( + &backend, + &dir, + "audit.php", + &content, + position.line, + position.character, + ) + .await; + let methods = method_names(&items); + for expected in ["active", "where", "orderBy", "whereIn", "firstOrFail"] { + assert!( + methods.contains(&expected), + "{generics}: {chain} lost {expected}: {methods:?}" + ); + } + + let content = + format!("firstOrFail();\n$model->"); + let position = crate::common::position_after(&content, "$model->"); + let items = complete_at( + &backend, + &dir, + "audit.php", + &content, + position.line, + position.character, + ) + .await; + assert!( + method_names(&items).contains(&"teamName"), + "{chain} lost its model" + ); + } + } +} + #[tokio::test] async fn test_builder_lock_for_update_via_intermediate_variable() { let user_php = "\ @@ -14733,6 +14823,120 @@ class User extends Model { // ─── whereHas / whereDoesntHave closure param from relation chain ─────────── +/// Completion and the diagnostics walk must agree on the final dotted segment, +/// including arrow functions and callbacks after the comparison arguments. +#[tokio::test] +async fn test_builder_audit_relation_callback_consumers() { + let model = r#" */ + public function stocks(): HasMany {} + public function scopeTeamOnly(Builder $query): void {} +}"#; + let stock = r#" */ + public function warehouse(): BelongsTo {} + public function scopeStockOnly(Builder $query): void {} +}"#; + let warehouse = r#" + */ +class WarehouseBuilder extends \Illuminate\Database\Eloquent\Builder { + /** @return $this */ + public function warehouseCustom() { return $this; } +}"#, + ), + ]); + for call in [ + "Team::whereHas('stocks.warehouse', function ($q) { BODY });", + "Team::whereHas('stocks.warehouse', fn ($q) => BODY);", + "Team::has('stocks.warehouse', '>=', 1, 'and', function ($q) { BODY });", + "Team::has('stocks.warehouse', '>=', 1, 'and', fn ($q) => BODY);", + "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 = "")); + let position = crate::common::position_after(&content, "$q->"); + let items = complete_at( + &backend, + &dir, + "audit.php", + &content, + position.line, + position.character, + ) + .await; + let methods = method_names(&items); + assert!(methods.contains(&"warehouseOnly"), "{call}: {methods:?}"); + assert!(methods.contains(&"warehouseCustom"), "{call}: {methods:?}"); + assert!( + !methods.contains(&"stockOnly"), + "{call}: intermediate model leaked" + ); + assert!(!methods.contains(&"teamOnly"), "{call}: outer model leaked"); + + let body = if call.contains("fn (") { + "$q->warehouseCustom()->warehouseOnly()->firstOrFail()->warehouseName()" + } else { + "$q->warehouseCustom()->warehouseOnly()->firstOrFail()->warehouseName();" + }; + let content = format!("{header}{}", call.replace("BODY", body)); + let uri = Url::from_file_path(dir.path().join("audit.php")).unwrap(); + let diagnostics = crate::common::unknown_member_diagnostics_with_scope_cache( + &backend, + uri.as_str(), + &content, + ); + assert!(diagnostics.is_empty(), "{call}: {diagnostics:?}"); + + let content = content.replace("warehouseName()", "missingWarehouseMethod()"); + let diagnostics = crate::common::unknown_member_diagnostics_with_scope_cache( + &backend, + uri.as_str(), + &content, + ); + assert_eq!(diagnostics.len(), 1, "{call}: {diagnostics:?}"); + assert!(diagnostics[0].message.contains("missingWarehouseMethod")); + } +} + #[tokio::test] async fn test_where_has_closure_resolves_to_related_model() { // Brand::whereHas('orders', function ($q) { $q-> }) diff --git a/tests/phpstan_nsrt/laravel-builder-relations.php b/tests/phpstan_nsrt/laravel-builder-relations.php new file mode 100644 index 000000000..2dfe76cea --- /dev/null +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -0,0 +1,426 @@ + */ + public static function query() {} + /** @return Builder */ + public function newQuery() {} + /** @return Builder */ + public function newQueryWithoutScopes() {} + /** @return Builder */ + public function newModelQuery() {} + } + + /** + * @template TModel of Model + * @mixin \Illuminate\Database\Query\Builder + */ + class Builder { + use \Illuminate\Support\Traits\ForwardsCalls; + + /** @return $this */ + public function where($column, $operator = null, $value = null) { return $this; } + /** @return TModel */ + public function firstOrFail() {} + /** @return TModel|null */ + public function first() {} + /** @return TModel */ + public function getModel() {} + public function exists(): bool { return false; } + + /** + * @template TRelatedModel of Model + * @param Relations\Relation|string $relation + * @param (\Closure(Builder): mixed)|null $callback + * @return $this + */ + public function has($relation, $operator = '>=', $count = 1, $boolean = 'and', ?\Closure $callback = null) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\Relation|string $relation + * @param (\Closure(Builder): mixed)|null $callback + * @return $this + */ + public function doesntHave($relation, $boolean = 'and', ?\Closure $callback = null) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\Relation|string $relation + * @param (\Closure(Builder): mixed)|null $callback + * @return $this + */ + public function whereHas($relation, ?\Closure $callback = null, $operator = '>=', $count = 1) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\Relation|string $relation + * @param (\Closure(Builder): mixed)|null $callback + * @return $this + */ + public function orWhereHas($relation, ?\Closure $callback = null, $operator = '>=', $count = 1) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\Relation|string $relation + * @param (\Closure(Builder): mixed)|null $callback + * @return $this + */ + public function whereDoesntHave($relation, ?\Closure $callback = null) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\Relation|string $relation + * @param (\Closure(Builder): mixed)|null $callback + * @return $this + */ + public function orWhereDoesntHave($relation, ?\Closure $callback = null) { return $this; } + + /** + * @param string $relation + * @param (\Closure(Builder<*>|Relations\Relation<*, *, *>): mixed)|null $callback + * @return $this + */ + public function withWhereHas($relation, ?\Closure $callback = null, $operator = '>=', $count = 1) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\MorphTo|string $relation + * @param string|array $types + * @param (\Closure(Builder, string): mixed)|null $callback + * @return $this + */ + public function whereHasMorph($relation, $types, ?\Closure $callback = null, $operator = '>=', $count = 1) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\MorphTo|string $relation + * @param string|array $types + * @param (\Closure(Builder, string): mixed)|null $callback + * @return $this + */ + public function orWhereHasMorph($relation, $types, ?\Closure $callback = null, $operator = '>=', $count = 1) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\MorphTo|string $relation + * @param string|array $types + * @param (\Closure(Builder, string): mixed)|null $callback + * @return $this + */ + public function hasMorph($relation, $types, $operator = '>=', $count = 1, $boolean = 'and', ?\Closure $callback = null) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\MorphTo|string $relation + * @param string|array $types + * @param (\Closure(Builder, string): mixed)|null $callback + * @return $this + */ + public function doesntHaveMorph($relation, $types, $boolean = 'and', ?\Closure $callback = null) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\MorphTo|string $relation + * @param string|array $types + * @param (\Closure(Builder, string): mixed)|null $callback + * @return $this + */ + public function whereDoesntHaveMorph($relation, $types, ?\Closure $callback = null) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\MorphTo|string $relation + * @param string|array $types + * @param (\Closure(Builder, string): mixed)|null $callback + * @return $this + */ + public function orWhereDoesntHaveMorph($relation, $types, ?\Closure $callback = null) { return $this; } + } +} + +namespace Illuminate\Database\Eloquent\Relations { + /** + * @template TRelatedModel of \Illuminate\Database\Eloquent\Model + * @template TDeclaringModel of \Illuminate\Database\Eloquent\Model + * @template TResult + * @mixin \Illuminate\Database\Eloquent\Builder + */ + class Relation { + use \Illuminate\Support\Traits\ForwardsCalls; + } + /** + * @template TRelatedModel of \Illuminate\Database\Eloquent\Model + * @template TDeclaringModel of \Illuminate\Database\Eloquent\Model + * @extends Relation + */ + class HasMany extends Relation {} + /** + * @template TRelatedModel of \Illuminate\Database\Eloquent\Model + * @template TDeclaringModel of \Illuminate\Database\Eloquent\Model + * @extends Relation + */ + class BelongsTo extends Relation {} + /** + * @template TRelatedModel of \Illuminate\Database\Eloquent\Model + * @template TDeclaringModel of \Illuminate\Database\Eloquent\Model + * @extends Relation + */ + class MorphTo extends Relation {} +} + +namespace BuilderRelationAudit { + use Illuminate\Database\Eloquent\Builder; + use Illuminate\Database\Eloquent\Builder as Query; + use Illuminate\Database\Eloquent\Model; + use Illuminate\Database\Eloquent\Relations\BelongsTo; + use Illuminate\Database\Eloquent\Relations\HasMany; + use Illuminate\Database\Eloquent\Relations\MorphTo; + use function PHPStan\Testing\assertType; + + /** + * @template TModel of Model + * @extends Builder + */ + class TeamBuilder extends Builder { + /** @return $this */ + public function active() { return $this; } + } + class Team extends Model { + /** @return TeamBuilder */ + public function newEloquentBuilder($query): TeamBuilder { return new TeamBuilder(); } + /** @return HasMany */ + public function stocks(): HasMany {} + public function teamName(): string { return ''; } + } + class Stock extends Model { + /** @return BelongsTo */ + public function warehouse(): BelongsTo {} + /** @return BelongsTo */ + public function team(): BelongsTo {} + /** @return BelongsTo */ + public function plainTeam(): BelongsTo {} + } + class Warehouse extends Model { + public function warehouseName(): string { return ''; } + } + class PlainTeamBuilder extends Builder { + /** @return $this */ + public function active() { return $this; } + } + class PlainTeam extends Model { + public function newEloquentBuilder($query): PlainTeamBuilder { return new PlainTeamBuilder(); } + public function teamName(): string { return ''; } + } + class ChildTeam extends Team {} + class OverrideTeam extends Team { + public function newQuery(): PlainTeamBuilder { return new PlainTeamBuilder(); } + } + class OtherModelQuery extends Team { + /** @return Builder */ + public function newQuery(): Builder {} + } + class Comment extends Model { + /** @return MorphTo */ + public function commentable(): MorphTo {} + } + class TeamComment extends Model { + /** @return MorphTo */ + public function commentable(): MorphTo {} + } + + function builders(Team $team, PlainTeam $plain): void { + $builder = Team::query(); + assertType('BuilderRelationAudit\TeamBuilder', $builder->where('active', true)->orderBy('id')); + assertType('BuilderRelationAudit\Team', $builder->where('active', true)->getModel()); + assertType('BuilderRelationAudit\TeamBuilder', Team::query()); + assertType('BuilderRelationAudit\TeamBuilder', Team::where('active', true)->orderBy('id')); + assertType('BuilderRelationAudit\TeamBuilder', Team::query()->where('active', true)->orderBy('id')->active()); + assertType('BuilderRelationAudit\TeamBuilder', Team::active()->whereIn('id', [1])->lockForUpdate()); + assertType('BuilderRelationAudit\TeamBuilder', $team->newQuery()->orderBy('id')); + assertType('BuilderRelationAudit\Team', Team::where('id', 1)->firstOrFail()); + assertType('BuilderRelationAudit\TeamBuilder', $team->newQueryWithoutScopes()->active()); + assertType('BuilderRelationAudit\Team', $team->newModelQuery()->firstOrFail()); + assertType('BuilderRelationAudit\PlainTeam', $plain->newQueryWithoutScopes()->firstOrFail()); + assertType('BuilderRelationAudit\PlainTeamBuilder', $plain->newModelQuery()->active()); + assertType('BuilderRelationAudit\Team|null', Team::query()->active()->first()); + assertType('bool', Team::query()->where('id', 1)->exists()); + assertType('int', Team::query()->where('id', 1)->count()); + // PHPantom retains the model argument even on a non-generic custom builder. + assertType('BuilderRelationAudit\PlainTeamBuilder', PlainTeam::query()->where('active', true)->orderBy('id')->active()); + assertType('BuilderRelationAudit\PlainTeamBuilder', PlainTeam::where('active', true)->orderBy('id')); + assertType('BuilderRelationAudit\PlainTeamBuilder', $plain->newQuery()->orderBy('id')); + assertType('BuilderRelationAudit\PlainTeam', PlainTeam::query()->active()->firstOrFail()); + } + + function inheritedFactories(ChildTeam $child, OverrideTeam $override, Warehouse $ordinary, OtherModelQuery $other): void { + assertType('BuilderRelationAudit\TeamBuilder', $child->newQuery()->active()); + assertType('BuilderRelationAudit\PlainTeamBuilder', $override->newQuery()); + assertType('Illuminate\Database\Eloquent\Builder', $ordinary->newQuery()); + assertType('Illuminate\Database\Eloquent\Builder', $other->newQuery()); + } + + class UnrelatedQuery { + /** @param \Closure(Warehouse): mixed $callback */ + public function whereHas(string $relation, \Closure $callback): void {} + } + + function unrelated(UnrelatedQuery $query): void { + $query->whereHas('stocks.warehouse', function ($model) { + assertType('BuilderRelationAudit\Warehouse', $model); + }); + } + + + 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); + }); + Team::whereHas('stocks.warehouse', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + assertType('BuilderRelationAudit\Warehouse', $query->firstOrFail()); + }); + Stock::query()->whereHas('warehouse', function (Builder $query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + Team::query()->where('active', true)->whereHas('stocks.warehouse', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + Team::has('stocks.warehouse', '>=', 1, 'and', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + Team::doesntHave('stocks', 'and', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + Team::orWhereHas('stocks.warehouse', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + Team::whereDoesntHave('stocks', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + Team::orWhereDoesntHave('stocks', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + Team::has(relation: 'stocks.warehouse', callback: function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + Team::has(callback: function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }, relation: 'stocks.warehouse'); + Stock::whereHas('team', function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Stock::whereHas('team', function (Builder $query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Stock::query()->whereHas('team', function (Query $query) { + assertType('BuilderRelationAudit\TeamBuilder', $query->active()); + }); + Stock::whereHas('plainTeam', function (Builder $query) { + assertType('BuilderRelationAudit\PlainTeamBuilder', $query); + }); + Stock::whereHas('team', function (Warehouse $query) { + assertType('BuilderRelationAudit\Warehouse', $query); + }); + } + + function eagerRelations(): void { + Team::withWhereHas('stocks', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder|Illuminate\Database\Eloquent\Relations\HasMany', $query); // SKIP: eager callback needs the builder/relation union + }); + Team::withWhereHas('stocks.warehouse', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder|Illuminate\Database\Eloquent\Relations\BelongsTo', $query); // SKIP: eager callback needs the builder/relation union + }); + Team::withWhereHas('stocks:id', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder|Illuminate\Database\Eloquent\Relations\HasMany', $query); // SKIP: eager callback needs the builder/relation union + }); + assertType('BuilderRelationAudit\TeamBuilder', Team::withWhereHas('stocks', null)); + } + + function morphs(): void { + Comment::whereHasMorph('commentable', Warehouse::class, function ($query, $type) { + assertType('Illuminate\Database\Eloquent\Builder', $query); // SKIP: morph candidate builders are not inferred + assertType('string', $type); + }); + Comment::whereHasMorph('commentable', [Team::class, Warehouse::class], function ($query, $type) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder', $query); // SKIP: morph candidate builders are not inferred + assertType('string', $type); + }); + Comment::query()->orWhereHasMorph('commentable', Team::class, function ($query, $type) { + assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('string', $type); + }); + Comment::hasMorph('commentable', Team::class, '>=', 1, 'and', function ($query, $type) { + assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('string', $type); + }); + Comment::doesntHaveMorph('commentable', Team::class, 'and', function ($query, $type) { + assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('string', $type); + }); + Comment::whereDoesntHaveMorph('commentable', Team::class, function ($query, $type) { + assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('string', $type); + }); + Comment::orWhereDoesntHaveMorph('commentable', Team::class, function ($query, $type) { + assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('string', $type); + }); + Comment::whereHasMorph(callback: function ($query, $type) { + assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('string', $type); + }, relation: 'commentable', types: Team::class); + Comment::whereHasMorph('commentable', '*', function ($query, $type) { + assertType('Illuminate\Database\Eloquent\Builder', $query); // SKIP: morph candidate builders are not inferred + assertType('string', $type); + }); + Comment::whereHasMorph('commentable', [], function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); // SKIP: morph candidate builders are not inferred + }); + TeamComment::whereHasMorph('commentable', '*', function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + }); + assertType('Illuminate\Database\Eloquent\Builder', Comment::whereHasMorph('commentable', [Team::class], null)); + } +}