diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 30d99b3cf..7c212be95 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -33,11 +33,13 @@ 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 +- **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..dc1600f1d 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -158,10 +158,13 @@ unlikely to move the needle for most users. | L46 | [`->can()` on a user model the receiver does not name](todo/laravel.md#l46-can-on-a-user-model-the-receiver-does-not-name) | Medium-High | Medium-High | | L30 | [Eloquent attribute-array key completion](todo/laravel.md#l30-eloquent-attribute-array-key-completion) | 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 | +| L57 | [Bind named relation callback arguments before inference](todo/laravel.md#l57-bind-named-relation-callback-arguments-before-inference) | Medium | Medium | | 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 | +| L56 | [Preserve custom builders in relation callbacks](todo/laravel.md#l56-preserve-custom-builders-in-relation-callbacks) | 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..91b4c85e5 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -247,35 +247,103 @@ 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 +#### L56. Preserve custom builders in relation callbacks **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. +Two sides of the relation override discard custom-builder context: + +- `Team::query()->whereHas('stocks.warehouse', …)` fails to identify the + receiver's model when `Team` uses `TeamBuilder`. The callback in the + assertion fixture becomes `Builder` instead of + `Builder`. +- `Stock::whereHas('team', …)` resolves the related model but always builds + `Builder`, losing `TeamBuilder` and its custom methods. An explicit + bare `Builder` parameter has the same problem. + +**Reproducer:** The custom-builder callback assertions in `relations()` +in `tests/phpstan_nsrt/laravel-builder-relations.php`. Expected types follow +[Larastan's relation callback assertions](https://github.com/larastan/larastan/blob/c328727e6103c1147d1c64cc96b3aedfda26bc20/tests/Type/data/relationship-query-callbacks.php). + +**Where to change:** `find_model_from_receivers` and +`try_relation_query_override` in +`type_engine/variable/closure_resolution.rs`, using the receiver's resolved +generic context and the existing custom-builder metadata/selection. Keep +this in the shared callable inference path so completion, hover, and +diagnostics agree. Refining a bare `Builder` hint to its custom subclass +must preserve the inferred generic arguments too. + +#### L57. 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 produces +`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. + +#### L58. Infer morph constraint callbacks from candidate models + +**Impact: Medium · Complexity: Medium-High** + +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..d93bca548 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -180,6 +180,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 +472,14 @@ 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 } diff --git a/examples/laravel/assertions.php b/examples/laravel/assertions.php index 9e5178ff1..a6e06ea60 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -196,6 +196,78 @@ 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] +); + +$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 +); + +// 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/virtual_members/laravel/builder.rs b/src/virtual_members/laravel/builder.rs index e64b4ee1b..4fd0f9e1b 100644 --- a/src/virtual_members/laravel/builder.rs +++ b/src/virtual_members/laravel/builder.rs @@ -16,7 +16,7 @@ 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; @@ -163,14 +163,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/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..fc643073f 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -213,6 +213,13 @@ 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 string $relation * @param (\\Closure(\\Illuminate\\Database\\Eloquent\\Builder): mixed)|null $callback @@ -2463,6 +2470,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 +14817,93 @@ 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#" 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::whereHas('stocks.warehouse', function (Builder $q) { BODY });", + ] { + 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(&"stockOnly"), + "{call}: intermediate model leaked" + ); + assert!(!methods.contains(&"teamOnly"), "{call}: outer model leaked"); + + let body = if call.contains("fn (") { + "$q->warehouseOnly()->firstOrFail()->warehouseName()" + } else { + "$q->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..c3d90e692 --- /dev/null +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -0,0 +1,399 @@ + */ + 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\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 {} + } + 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); + }); + } + + 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); // SKIP: custom-builder receiver is not recognized + }); + 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); // SKIP: relation argument must currently come first + }, relation: 'stocks.warehouse'); + Stock::whereHas('team', function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: related custom builder is replaced with the base builder + }); + Stock::whereHas('team', function (Builder $query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: related custom builder is replaced with the base builder + }); + } + + 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)); + } +}