From aa607af403e634867cc8de29941f9acfd2d0d9c0 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Sun, 20 Sep 2026 23:20:47 +0600 Subject: [PATCH 1/7] test(laravel): audit custom builders and relation callbacks --- docs/CHANGELOG.md | 1 + docs/todo.md | 6 +- docs/todo/laravel.md | 140 +++++-- examples/laravel/app/Demo.php | 15 +- examples/laravel/assertions.php | 60 +++ tests/integration/completion_laravel.rs | 168 ++++++++ .../laravel-builder-relations.php | 376 ++++++++++++++++++ 7 files changed, 736 insertions(+), 30 deletions(-) create mode 100644 tests/phpstan_nsrt/laravel-builder-relations.php diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 30d99b3cf..7cf43fe9c 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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. diff --git a/docs/todo.md b/docs/todo.md index bbafd6b3b..1c179878a 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -158,10 +158,14 @@ 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 | +| L55 | [Preserve custom builders through model instance query factories](todo/laravel.md#l55-preserve-custom-builders-through-model-instance-query-factories) | 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..a19f45463 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -247,35 +247,123 @@ 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 +#### L55. Preserve custom builders through model instance query factories + +**Impact: Medium · Complexity: Medium** + +`Team::query()` and `Team::where(…)->orderBy(…)` preserve a custom +`TeamBuilder`, but `$team->newQuery()->orderBy(…)` returns `Builder` +instead. This loses custom methods even though Laravel constructs the +same builder in both cases. Both generic and non-generic custom builders +are affected. + +**Reproducer:** The `builders()` assertions marked `SKIP` in +`tests/phpstan_nsrt/laravel-builder-relations.php`. Compare +[Larastan's custom builder assertions](https://github.com/larastan/larastan/blob/c328727e6103c1147d1c64cc96b3aedfda26bc20/tests/Type/data/custom-eloquent-builder.php). + +**Where to change:** Reuse the model's custom-builder selection for the +instance query-factory return in the shared type engine. Check +`newQueryWithoutScopes()` and `newModelQuery()` alongside `newQuery()`. +Keep the inferred model argument for a builder without its own templates; +removing that context would break terminal methods such as `firstOrFail()`. + +#### L56. Preserve custom builders in relation callbacks + +**Impact: Medium · Complexity: Medium-High** + +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** -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. +`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..f5826193a 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -180,6 +180,11 @@ 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 + // Paginators carry the model element type through foreach foreach (BlogAuthor::where('active', 1)->paginate() as $author) { $author->profile->getBio(); // → BlogAuthor @@ -464,10 +469,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..91b927a34 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -196,6 +196,66 @@ 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 +); + +$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/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index 2848f2746..fad968ca7 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,80 @@ 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()", + ] { + 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 +14814,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..cd0cc1733 --- /dev/null +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -0,0 +1,376 @@ + */ + public static function query() {} + /** @return Builder */ + public function newQuery() {} + } + + /** + * @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 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')); // SKIP: newQuery loses the custom builder + assertType('BuilderRelationAudit\Team', Team::where('id', 1)->firstOrFail()); + 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')); // SKIP: newQuery loses the custom builder + assertType('BuilderRelationAudit\PlainTeam', PlainTeam::query()->active()->firstOrFail()); + } + + 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)); + } +} From 7d608a7ee7bae4a9f1c33163d486f2860c225e46 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Sun, 20 Sep 2026 23:28:30 +0600 Subject: [PATCH 2/7] fix(laravel): preserve custom builders in model query factories --- docs/CHANGELOG.md | 1 + docs/todo.md | 1 - docs/todo/laravel.md | 20 ----- examples/laravel/app/Demo.php | 3 + examples/laravel/assertions.php | 12 +++ src/virtual_members/laravel/builder.rs | 46 ++++++++--- src/virtual_members/laravel/builder_tests.rs | 79 ++++++++++++++----- src/virtual_members/mod.rs | 4 +- tests/integration/completion_laravel.rs | 3 + .../laravel-builder-relations.php | 27 ++++++- 10 files changed, 145 insertions(+), 51 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7cf43fe9c..7c212be95 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -39,6 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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 1c179878a..dc1600f1d 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -158,7 +158,6 @@ 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 | -| L55 | [Preserve custom builders through model instance query factories](todo/laravel.md#l55-preserve-custom-builders-through-model-instance-query-factories) | 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 | diff --git a/docs/todo/laravel.md b/docs/todo/laravel.md index a19f45463..91b4c85e5 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -247,26 +247,6 @@ the collection return-type patches for the argument forms. The property lookup already exists for `model-property`; the work is threading a resolved key type through the generic substitution. -#### L55. Preserve custom builders through model instance query factories - -**Impact: Medium · Complexity: Medium** - -`Team::query()` and `Team::where(…)->orderBy(…)` preserve a custom -`TeamBuilder`, but `$team->newQuery()->orderBy(…)` returns `Builder` -instead. This loses custom methods even though Laravel constructs the -same builder in both cases. Both generic and non-generic custom builders -are affected. - -**Reproducer:** The `builders()` assertions marked `SKIP` in -`tests/phpstan_nsrt/laravel-builder-relations.php`. Compare -[Larastan's custom builder assertions](https://github.com/larastan/larastan/blob/c328727e6103c1147d1c64cc96b3aedfda26bc20/tests/Type/data/custom-eloquent-builder.php). - -**Where to change:** Reuse the model's custom-builder selection for the -instance query-factory return in the shared type engine. Check -`newQueryWithoutScopes()` and `newModelQuery()` alongside `newQuery()`. -Keep the inferred model argument for a builder without its own templates; -removing that context would break terminal methods such as `firstOrFail()`. - #### L56. Preserve custom builders in relation callbacks **Impact: Medium · Complexity: Medium-High** diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index f5826193a..d93bca548 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -184,6 +184,9 @@ public function eloquentQuery(): void 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) { diff --git a/examples/laravel/assertions.php b/examples/laravel/assertions.php index 91b927a34..a6e06ea60 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -210,6 +210,18 @@ function assertMethodReturnType(string $class, string $method, string $expected) && $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()); 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 fad968ca7..fc643073f 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -2502,6 +2502,9 @@ class Team extends Model { "Team::query()->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->" diff --git a/tests/phpstan_nsrt/laravel-builder-relations.php b/tests/phpstan_nsrt/laravel-builder-relations.php index cd0cc1733..c3d90e692 100644 --- a/tests/phpstan_nsrt/laravel-builder-relations.php +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -32,6 +32,10 @@ class Model { public static function query() {} /** @return Builder */ public function newQuery() {} + /** @return Builder */ + public function newQueryWithoutScopes() {} + /** @return Builder */ + public function newModelQuery() {} } /** @@ -232,6 +236,14 @@ 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 {} @@ -249,18 +261,29 @@ function builders(Team $team, PlainTeam $plain): void { 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')); // SKIP: newQuery loses the custom builder + 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')); // SKIP: newQuery loses the custom builder + 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 {} From e419ac0f9a9287a9dacbcc9d29ef5c23f07d7be6 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Sun, 20 Sep 2026 23:34:53 +0600 Subject: [PATCH 3/7] fix(laravel): retain custom builders in relation callbacks --- docs/CHANGELOG.md | 1 + docs/todo.md | 1 - docs/todo/laravel.md | 26 ------ examples/laravel/app/Demo.php | 7 ++ examples/laravel/assertions.php | 12 +++ .../variable/closure_resolution.rs | 85 +++++++++++++------ .../forward_walk/callable_inference.rs | 4 +- .../variable/forward_walk/diagnostic_walk.rs | 6 +- src/virtual_members/laravel/builder.rs | 76 ++++++++++------- src/virtual_members/laravel/mod.rs | 1 + tests/integration/completion_laravel.rs | 25 +++++- .../laravel-builder-relations.php | 18 +++- 12 files changed, 167 insertions(+), 95 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7c212be95..087190f48 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -39,6 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **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. diff --git a/docs/todo.md b/docs/todo.md index dc1600f1d..177d0cbb6 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -162,7 +162,6 @@ 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 | -| 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 | diff --git a/docs/todo/laravel.md b/docs/todo/laravel.md index 91b4c85e5..a941a5af1 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -247,32 +247,6 @@ the collection return-type patches for the argument forms. The property lookup already exists for `model-property`; the work is threading a resolved key type through the generic substitution. -#### L56. Preserve custom builders in relation callbacks - -**Impact: Medium · Complexity: Medium-High** - -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** diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index d93bca548..98047453b 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; @@ -480,6 +481,12 @@ public function eloquentClosure(): void // 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 + + // 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 a6e06ea60..63e64a236 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -232,6 +232,18 @@ function assertMethodReturnType(string $class, string $method, string $expected) $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()); diff --git a/src/type_engine/variable/closure_resolution.rs b/src/type_engine/variable/closure_resolution.rs index ac2b606c6..4d6c3a133 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 @@ -795,14 +809,14 @@ fn describes_elements(ty: &PhpType, allow_iterable: bool) -> bool { /// `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. +/// 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>, class_loader: &dyn Fn(&str) -> Option>, @@ -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>, 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, first_arg_text, 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..b215d2b14 100644 --- a/src/type_engine/variable/forward_walk/callable_inference.rs +++ b/src/type_engine/variable/forward_walk/callable_inference.rs @@ -110,7 +110,7 @@ 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, + &resolved_types, method_name, first_arg_text, ctx.class_loader, @@ -355,7 +355,7 @@ pub(crate) fn infer_callable_params_from_static_receiver_fw( // 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())], + &[ResolvedType::from_class(cls.clone())], method_name, first_arg_text, ctx.class_loader, diff --git a/src/type_engine/variable/forward_walk/diagnostic_walk.rs b/src/type_engine/variable/forward_walk/diagnostic_walk.rs index a3074a22f..46662844f 100644 --- a/src/type_engine/variable/forward_walk/diagnostic_walk.rs +++ b/src/type_engine/variable/forward_walk/diagnostic_walk.rs @@ -663,7 +663,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 4fd0f9e1b..3ead213d9 100644 --- a/src/virtual_members/laravel/builder.rs +++ b/src/virtual_members/laravel/builder.rs @@ -22,47 +22,59 @@ 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, 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/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index fc643073f..450167fd3 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -14826,6 +14826,7 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\HasMany; class Team extends Model { + public function newEloquentBuilder($query): TeamBuilder { return new TeamBuilder(); } /** @return HasMany */ public function stocks(): HasMany {} public function scopeTeamOnly(Builder $query): void {} @@ -14843,6 +14844,7 @@ class Stock extends Model { use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Builder; class Warehouse extends Model { + public function newEloquentBuilder($query): WarehouseBuilder { return new WarehouseBuilder(); } public function scopeWarehouseOnly(Builder $query): void {} public function warehouseName(): string { return ''; } }"#; @@ -14850,6 +14852,23 @@ class Warehouse extends Model { ("src/Models/Team.php", model), ("src/Models/Stock.php", stock), ("src/Models/Warehouse.php", warehouse), + ( + "src/Models/TeamBuilder.php", + 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 });", @@ -14857,6 +14876,7 @@ class Warehouse extends Model { "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 });", ] { let header = "warehouseOnly()->firstOrFail()->warehouseName()" + "$q->warehouseCustom()->warehouseOnly()->firstOrFail()->warehouseName()" } else { - "$q->warehouseOnly()->firstOrFail()->warehouseName();" + "$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(); diff --git a/tests/phpstan_nsrt/laravel-builder-relations.php b/tests/phpstan_nsrt/laravel-builder-relations.php index c3d90e692..b47846564 100644 --- a/tests/phpstan_nsrt/laravel-builder-relations.php +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -198,6 +198,7 @@ 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; @@ -224,6 +225,8 @@ class Stock extends Model { 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 ''; } @@ -307,7 +310,7 @@ function relations(): void { 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 + assertType('Illuminate\Database\Eloquent\Builder', $query); }); Team::has('stocks.warehouse', '>=', 1, 'and', function ($query) { assertType('Illuminate\Database\Eloquent\Builder', $query); @@ -331,10 +334,19 @@ function relations(): void { 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 + assertType('BuilderRelationAudit\TeamBuilder', $query); }); Stock::whereHas('team', function (Builder $query) { - assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: related custom builder is replaced with the base builder + 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); }); } From f1bf33aad2a8463dee02a475a248aeedfe3c95c0 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Sun, 20 Sep 2026 23:41:43 +0600 Subject: [PATCH 4/7] fix(laravel): bind named relation constraint arguments --- docs/CHANGELOG.md | 2 + docs/todo.md | 1 - docs/todo/laravel.md | 21 ---- examples/laravel/app/Demo.php | 3 + examples/laravel/assertions.php | 6 + .../variable/closure_resolution.rs | 10 +- .../forward_walk/callable_inference.rs | 116 ++++++++++-------- .../variable/forward_walk/diagnostic_walk.rs | 6 - tests/integration/completion_laravel.rs | 12 ++ .../laravel-builder-relations.php | 17 ++- 10 files changed, 112 insertions(+), 82 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 087190f48..6522b1131 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -39,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Named relation callbacks keep their model types.** Completion and diagnostics now resolve relationship constraints when named arguments are reordered or optional arguments are omitted. Contributed by @shuvroroy. + - **Relation callbacks 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. diff --git a/docs/todo.md b/docs/todo.md index 177d0cbb6..cb0251127 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -158,7 +158,6 @@ 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 | diff --git a/docs/todo/laravel.md b/docs/todo/laravel.md index a941a5af1..daa0912c0 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -247,27 +247,6 @@ the collection return-type patches for the argument forms. The property lookup already exists for `model-property`; the work is threading a resolved key type through the generic substitution. -#### 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** diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index 98047453b..879ecb288 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -482,6 +482,9 @@ public function eloquentClosure(): void // receive the final related model's builder too. BlogAuthor::has('posts.author', '>=', 1, 'and', fn ($q) => $q->active()); // → Builder + // Named arguments may put the callback before the relation. + BlogAuthor::has(callback: fn ($q) => $q->active(), relation: 'posts.author'); // → Builder + // The related model chooses the builder, including with a bare hint. Bakery::whereHas('baguettes', function (Builder $q) { $q->stale(); // → LoafBuilder diff --git a/examples/laravel/assertions.php b/examples/laravel/assertions.php index 63e64a236..6a6808858 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -257,6 +257,12 @@ function assertMethodReturnType(string $class, string $method, string $expected) ($query = \App\Models\BlogAuthor::query())->has('posts.author', '>=', 1, 'and', fn ($related) => $related->active()) === $query ); +$namedCallbackModels = []; +\App\Models\BlogAuthor::query()->has(callback: function ($query) use (&$namedCallbackModels) { + $namedCallbackModels[] = get_class($query->active()->getModel()); +}, relation: 'posts.author'); +check('Reordered named relation arguments preserve the callback model', $namedCallbackModels === [\App\Models\BlogAuthor::class]); + // Check both callback invocations without executing the eager-load SQL. $callbackClasses = []; $query = \App\Models\BlogAuthor::withWhereHas('posts', function ($related) use (&$callbackClasses) { diff --git a/src/type_engine/variable/closure_resolution.rs b/src/type_engine/variable/closure_resolution.rs index 4d6c3a133..af09d907a 100644 --- a/src/type_engine/variable/closure_resolution.rs +++ b/src/type_engine/variable/closure_resolution.rs @@ -808,7 +808,7 @@ fn describes_elements(ty: &PhpType, allow_iterable: bool) -> bool { /// Check whether `method_name` is a relation-query method (e.g. /// `whereHas`, `orWhereHas`, `whereDoesntHave`, etc.) and the receiver /// is an Eloquent model or `Builder`. If so, resolve the -/// relation chain from the first argument string and return +/// relation chain from the bound relation string and return /// the final related model’s builder as the closure parameter type. /// /// Returns `None` when the override does not apply (not a relation-query @@ -818,14 +818,14 @@ fn describes_elements(ty: &PhpType, allow_iterable: bool) -> bool { fn try_relation_query_override( receivers: &[ResolvedType], method_name: &str, - first_arg_text: Option<&str>, + relation_name: Option<&str>, class_loader: &dyn Fn(&str) -> Option>, ) -> Option> { if !RELATION_QUERY_METHODS.contains(&method_name) { return None; } - let relation_name = first_arg_text?; + let relation_name = relation_name?; if relation_name.is_empty() { return None; } @@ -1009,10 +1009,10 @@ fn extract_model_from_builder(builder: &ClassInfo) -> Option { pub(in crate::type_engine) fn try_relation_query_override_pub( receivers: &[ResolvedType], method_name: &str, - first_arg_text: Option<&str>, + relation_name: Option<&str>, class_loader: &dyn Fn(&str) -> Option>, ) -> Option> { - try_relation_query_override(receivers, method_name, first_arg_text, class_loader) + try_relation_query_override(receivers, method_name, relation_name, class_loader) } pub(in crate::type_engine) fn build_receiver_self_type_pub( diff --git a/src/type_engine/variable/forward_walk/callable_inference.rs b/src/type_engine/variable/forward_walk/callable_inference.rs index b215d2b14..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( - &resolved_types, - method_name, - first_arg_text, - ctx.class_loader, - ) { + if let Some(override_params) = + infer_relation_callback_params(&resolved_types, method_name, arg_idx, argument_list, ctx) + { return override_params; } @@ -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( - &[ResolvedType::from_class(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 46662844f..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, ) diff --git a/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index 450167fd3..9ab0f15a9 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -220,6 +220,12 @@ class Builder { * @return $this */ public function has($relation, $operator = '>=', $count = 1, $boolean = 'and', ?\\Closure $callback = null) { return $this; } + /** + * @param \\Closure(\\Illuminate\\Database\\Eloquent\\Builder): mixed $column + * @return $this + */ + public function whereRelation($relation, $column, $operator = null, $value = null) { return $this; } + /** * @param string $relation * @param (\\Closure(\\Illuminate\\Database\\Eloquent\\Builder): mixed)|null $callback @@ -14878,6 +14884,12 @@ class WarehouseBuilder extends \Illuminate\Database\Eloquent\Builder { "Stock::query()->whereHas('warehouse', function ($q) { BODY });", "Team::query()->where('id', 1)->whereHas('stocks.warehouse', function ($q) { BODY });", "Team::whereHas('stocks.warehouse', function (Builder $q) { BODY });", + "Team::has(callback: function ($q) { BODY }, relation: 'stocks.warehouse');", + "Team::has(callback: fn ($q) => BODY, relation: 'stocks.warehouse');", + "Team::query()->has(callback: fn ($q) => BODY, relation: 'stocks.warehouse');", + "Team::query()?->whereHas(callback: function ($q) { BODY }, relation: 'stocks.warehouse');", + "Stock::whereHas(callback: function (Builder $q) { BODY }, relation: 'warehouse');", + "Team::whereRelation(column: fn ($q) => BODY, relation: 'stocks.warehouse');", ] { let header = "")); diff --git a/tests/phpstan_nsrt/laravel-builder-relations.php b/tests/phpstan_nsrt/laravel-builder-relations.php index b47846564..2dfe76cea 100644 --- a/tests/phpstan_nsrt/laravel-builder-relations.php +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -298,6 +298,21 @@ function unrelated(UnrelatedQuery $query): void { }); } + + class CustomArguments extends Team { + /** + * @param \Closure(Warehouse): void $operator + * @param \Closure(Builder): void $callback + */ + public static function has($relation, $operator = null, $count = 1, $boolean = 'and', ?\Closure $callback = null) {} + } + + function unrelatedArgument(): void { + CustomArguments::has('stocks', operator: function ($value) { + assertType('BuilderRelationAudit\Warehouse', $value); + }); + } + function relations(): void { Team::whereHas('stocks', function ($query) { assertType('Illuminate\Database\Eloquent\Builder', $query); @@ -331,7 +346,7 @@ function relations(): void { assertType('Illuminate\Database\Eloquent\Builder', $query); }); Team::has(callback: function ($query) { - assertType('Illuminate\Database\Eloquent\Builder', $query); // SKIP: relation argument must currently come first + assertType('Illuminate\Database\Eloquent\Builder', $query); }, relation: 'stocks.warehouse'); Stock::whereHas('team', function ($query) { assertType('BuilderRelationAudit\TeamBuilder', $query); From 4c35dc022e1fade66d78b71e6d6a96d15edc68b0 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Sun, 20 Sep 2026 23:48:09 +0600 Subject: [PATCH 5/7] fix(laravel): infer morph constraint candidate builders --- docs/CHANGELOG.md | 2 + docs/todo.md | 1 - docs/todo/laravel.md | 29 ----- examples/laravel/app/Demo.php | 6 + examples/laravel/assertions.php | 16 +++ .../variable/closure_resolution.rs | 101 ++++++++++++++-- .../forward_walk/callable_inference.rs | 42 ++++++- src/virtual_members/laravel/relationships.rs | 10 ++ tests/integration/completion_laravel.rs | 55 +++++++++ .../laravel-builder-relations.php | 108 ++++++++++++++++-- 10 files changed, 315 insertions(+), 55 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 6522b1131..6d81f0e0f 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -39,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Morph relation callbacks infer their candidate models.** Completion and diagnostics retain custom builders for polymorphic constraints, including unions and class-string variables, while unknown candidates use the relation’s declared model. Contributed by @shuvroroy. + - **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. diff --git a/docs/todo.md b/docs/todo.md index cb0251127..6bffb9dfe 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -161,7 +161,6 @@ 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 | -| 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 | diff --git a/docs/todo/laravel.md b/docs/todo/laravel.md index daa0912c0..cac1f0972 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -247,35 +247,6 @@ the collection return-type patches for the argument forms. The property lookup already exists for `model-property`; the work is threading a resolved key type through the generic substitution. -#### 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** diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index 879ecb288..92290695b 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -490,6 +490,12 @@ public function eloquentClosure(): void $q->stale(); // → LoafBuilder }); Bakery::query()->whereHas('headBaker', fn ($q) => $q->active()); // → BakerBuilder + + // Morph candidates choose the callback builder and keep the type string. + Review::whereHasMorph('reviewable', Loaf::class, function (Builder $q, string $type) { + $q->stale(); // → LoafBuilder + echo $type; // → string + }); } diff --git a/examples/laravel/assertions.php b/examples/laravel/assertions.php index 6a6808858..97c387366 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -286,6 +286,22 @@ function assertMethodReturnType(string $class, string $method, string $expected) ] ); +$morphCandidate = \App\Models\Loaf::class; +foreach (['hasMorph', 'doesntHaveMorph', 'whereHasMorph', 'orWhereHasMorph', 'whereDoesntHaveMorph', 'orWhereDoesntHaveMorph'] as $method) { + $seen = []; + \App\Models\Review::$method(callback: function ($query, $type) use (&$seen) { + $seen[] = [get_class($query->stale()), $type]; + }, types: $morphCandidate, relation: 'reviewable'); + check("$method binds named candidates and preserves the custom builder", $seen === [[\App\Models\LoafBuilder::class, $morphCandidate]]); +} +foreach (['whereMorphRelation', 'orWhereMorphRelation', 'whereMorphDoesntHaveRelation', 'orWhereMorphDoesntHaveRelation'] as $method) { + $seen = []; + \App\Models\Review::$method(column: function ($query) use (&$seen) { + $seen[] = get_class($query->stale()->getModel()); + }, types: $morphCandidate, relation: 'reviewable'); + check("$method passes the candidate builder to its column closure", $seen === [$morphCandidate]); +} + // 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 af09d907a..fcede046d 100644 --- a/src/type_engine/variable/closure_resolution.rs +++ b/src/type_engine/variable/closure_resolution.rs @@ -22,8 +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, model_builder_type, - resolve_relation_chain, + ELOQUENT_BUILDER_FQN, ELOQUENT_MODEL_FQN, RELATION_QUERY_METHODS, extends_eloquent_model, + model_builder_type, resolve_relation_chain, }; thread_local! { @@ -750,6 +750,11 @@ fn inferred_type_is_more_specific( inferred: &PhpType, class_loader: &dyn Fn(&str) -> Option>, ) -> bool { + if let TypeKind::Union(parts) = inferred.kind() { + return parts + .iter() + .all(|part| inferred_type_is_more_specific(explicit_hint, part, class_loader)); + } // The explicit hint must be a bare name (no generic args). let explicit_base = match explicit_hint.kind() { TypeKind::Named(name) => name.as_str(), @@ -819,6 +824,7 @@ fn try_relation_query_override( receivers: &[ResolvedType], method_name: &str, relation_name: Option<&str>, + candidates: Option<&PhpType>, class_loader: &dyn Fn(&str) -> Option>, ) -> Option> { if !RELATION_QUERY_METHODS.contains(&method_name) { @@ -835,13 +841,85 @@ fn try_relation_query_override( // `Builder` instance. 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 related = resolve_relation_chain(&model, relation_name, class_loader, None) + .and_then(|name| class_loader(&name)); + if let Some(candidates) = candidates { + let fallback = related + .as_ref() + .map(|model| model_builder_type(model, class_loader)) + .unwrap_or_else(|| { + PhpType::generic( + ELOQUENT_BUILDER_FQN, + vec![PhpType::named(atom(ELOQUENT_MODEL_FQN))], + ) + }); + return Some(vec![morph_candidate_builders( + candidates, + &fallback, + class_loader, + )]); + } + Some(vec![model_builder_type(related.as_deref()?, class_loader)]) +} - let related = class_loader(&related_fqn)?; - let builder_type = model_builder_type(&related, class_loader); +/// Map candidate class-strings through the same builder selector as model +/// queries. Unknown candidates contribute the declared relation's fallback; +/// they must not erase known candidates from a mixed array. +fn morph_candidate_builders( + candidates: &PhpType, + fallback: &PhpType, + class_loader: &dyn Fn(&str) -> Option>, +) -> PhpType { + match candidates.kind() { + TypeKind::Union(parts) => PhpType::union( + parts + .iter() + .map(|part| morph_candidate_builders(part, fallback, class_loader)) + .collect(), + ), + TypeKind::ClassString(Some(model)) => morph_model_builders(model, fallback, class_loader), + TypeKind::Literal(value) => value + .string_content() + .and_then(|name| { + class_loader(&name).filter(|model| { + model + .fqn() + .eq_ignore_ascii_case(name.trim_start_matches('\\')) + }) + }) + .filter(|model| { + model.fqn() == ELOQUENT_MODEL_FQN || extends_eloquent_model(model, class_loader) + }) + .map(|model| model_builder_type(&model, class_loader)) + .unwrap_or_else(|| fallback.clone()), + _ => candidates + .iterable_element_type() + .map(|element| morph_candidate_builders(&element, fallback, class_loader)) + .unwrap_or_else(|| fallback.clone()), + } +} - Some(vec![builder_type]) +fn morph_model_builders( + model: &PhpType, + fallback: &PhpType, + class_loader: &dyn Fn(&str) -> Option>, +) -> PhpType { + if let TypeKind::Union(parts) = model.kind() { + return PhpType::union( + parts + .iter() + .map(|part| morph_model_builders(part, fallback, class_loader)) + .collect(), + ); + } + model + .base_name() + .and_then(class_loader) + .filter(|model| { + model.fqn() == ELOQUENT_MODEL_FQN || extends_eloquent_model(model, class_loader) + }) + .map(|model| model_builder_type(&model, class_loader)) + .unwrap_or_else(|| fallback.clone()) } /// Build a `PhpType` representing the receiver class for `$this`/`static` @@ -1010,9 +1088,16 @@ pub(in crate::type_engine) fn try_relation_query_override_pub( receivers: &[ResolvedType], method_name: &str, relation_name: Option<&str>, + candidates: Option<&PhpType>, class_loader: &dyn Fn(&str) -> Option>, ) -> Option> { - try_relation_query_override(receivers, method_name, relation_name, class_loader) + try_relation_query_override( + receivers, + method_name, + relation_name, + candidates, + class_loader, + ) } pub(in crate::type_engine) fn build_receiver_self_type_pub( diff --git a/src/type_engine/variable/forward_walk/callable_inference.rs b/src/type_engine/variable/forward_walk/callable_inference.rs index c3431d773..7743c087a 100644 --- a/src/type_engine/variable/forward_walk/callable_inference.rs +++ b/src/type_engine/variable/forward_walk/callable_inference.rs @@ -106,9 +106,14 @@ 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) = - infer_relation_callback_params(&resolved_types, method_name, arg_idx, argument_list, ctx) - { + if let Some(override_params) = infer_relation_callback_params( + &resolved_types, + method_name, + arg_idx, + argument_list, + scope, + ctx, + ) { return override_params; } @@ -351,6 +356,7 @@ pub(crate) fn infer_callable_params_from_static_receiver_fw( method_name, arg_idx, argument_list, + scope, ctx, ) { return override_params; @@ -527,12 +533,13 @@ fn infer_relation_callback_params( method_name: &str, arg_idx: usize, argument_list: &ArgumentList<'_>, + scope: &ScopeState, ctx: &ForwardWalkCtx<'_>, ) -> Option> { if !crate::virtual_members::laravel::RELATION_QUERY_METHODS.contains(&method_name) { return None; } - let callback_name = if method_name == "whereRelation" { + let callback_name = if method_name.ends_with("Relation") { "column" } else { "callback" @@ -571,12 +578,35 @@ fn infer_relation_callback_params( else { return None; }; - return super::super::closure_resolution::try_relation_query_override_pub( + let candidates = if method_name.contains("Morph") { + let types = bound[param_index("types")?]?; + let scope_resolver = + |name: &str| scope.locals.get(&atom(name)).cloned().unwrap_or_default(); + let var_ctx = ctx.var_ctx_for_with_scope( + "$__infer", + types.span().start.offset, + &scope_resolver, + Some(scope.proofs()), + ); + Some( + super::super::resolution::resolve_arg_raw_type(types, &var_ctx) + .unwrap_or_else(PhpType::mixed), + ) + } else { + None + }; + let mut inferred = super::super::closure_resolution::try_relation_query_override_pub( receivers, method_name, relation.value.map(bytes_to_str), + candidates.as_ref(), ctx.class_loader, - ); + )?; + // Refine the query parameter without losing Laravel's second morph + // callback parameter (the candidate's class-string). + let declared = extract_callable_params_at_fw(&method.parameters, argument_list, arg_idx); + inferred.extend(declared.into_iter().skip(1)); + return Some(inferred); } None } diff --git a/src/virtual_members/laravel/relationships.rs b/src/virtual_members/laravel/relationships.rs index 0f1e3afb3..ea75f5436 100644 --- a/src/virtual_members/laravel/relationships.rs +++ b/src/virtual_members/laravel/relationships.rs @@ -35,6 +35,16 @@ pub(crate) const RELATION_QUERY_METHODS: &[&str] = &[ "whereDoesntHave", "orWhereDoesntHave", "whereRelation", + "hasMorph", + "doesntHaveMorph", + "whereHasMorph", + "orWhereHasMorph", + "whereDoesntHaveMorph", + "orWhereDoesntHaveMorph", + "whereMorphRelation", + "orWhereMorphRelation", + "whereMorphDoesntHaveRelation", + "orWhereMorphDoesntHaveRelation", ]; /// Fully-qualified relationship class names used by diff --git a/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index 9ab0f15a9..c5151b5a9 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -14937,6 +14937,61 @@ class WarehouseBuilder extends \Illuminate\Database\Eloquent\Builder { } } +#[tokio::test] +async fn test_morph_callback_candidates_reach_completion_and_diagnostics() { + let fixture = include_str!("../phpstan_nsrt/laravel-builder-relations.php"); + let backend = create_test_backend(); + let uri = Url::parse("file:///morph-callbacks.php").unwrap(); + for call in [ + "Comment::whereHasMorph('commentable', Team::class, function ($morphQuery, $type) { BODY });", + "Comment::hasMorph('commentable', Team::class, '>=', 1, 'and', fn ($morphQuery, $type) => BODY);", + "Comment::doesntHaveMorph(callback: function ($morphQuery, $type) { BODY }, types: Team::class, relation: 'commentable');", + "Comment::query()->orWhereHasMorph('commentable', $candidate, fn ($morphQuery, $type) => BODY);", + "Comment::query()?->whereDoesntHaveMorph('commentable', [Team::class], function (Builder $morphQuery) { BODY });", + "Comment::orWhereDoesntHaveMorph('commentable', Team::class, fn ($morphQuery) => BODY);", + "Comment::whereMorphRelation(column: fn ($morphQuery) => BODY, relation: 'commentable', types: Team::class);", + "Comment::orWhereMorphRelation('commentable', Team::class, fn ($morphQuery) => BODY);", + "Comment::whereMorphDoesntHaveRelation('commentable', Team::class, function ($morphQuery) { BODY });", + "Comment::orWhereMorphDoesntHaveRelation('commentable', Team::class, fn ($morphQuery) => BODY);", + "TeamComment::whereHasMorph('commentable', '*', fn ($morphQuery) => BODY);", + ] { + let source = format!( + "{fixture}\nnamespace BuilderRelationAudit {{ use Illuminate\\Database\\Eloquent\\Builder; $candidate = Team::class; {call} }}" + ); + let content = source.replace("BODY", "$morphQuery->"); + let position = crate::common::position_after(&content, "$morphQuery->"); + let items = + crate::common::complete_at(&backend, &uri, &content, position.line, position.character) + .await; + let methods = method_names(&items); + assert!(methods.contains(&"active"), "{call}: {methods:?}"); + assert!(methods.contains(&"where"), "{call}: {methods:?}"); + let body = if call.contains("fn (") { + "$morphQuery->active()->getModel()->stocks()" + } else { + "$morphQuery->active()->getModel()->stocks();" + }; + let content = source.replace("BODY", body); + 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( + "getModel()->stocks()", + "getModel()->missingMorphModelMethod()", + ); + 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("missingMorphModelMethod")); + } +} + #[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 index 2dfe76cea..dcb676892 100644 --- a/tests/phpstan_nsrt/laravel-builder-relations.php +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -163,6 +163,38 @@ public function whereDoesntHaveMorph($relation, $types, ?\Closure $callback = nu * @return $this */ public function orWhereDoesntHaveMorph($relation, $types, ?\Closure $callback = null) { return $this; } + /** + * @template TRelatedModel of Model + * @param Relations\MorphTo|string $relation + * @param (\Closure(Builder): mixed)|string $column + * @return $this + */ + public function whereMorphRelation($relation, $types, $column, $operator = null, $value = null) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\MorphTo|string $relation + * @param (\Closure(Builder): mixed)|string $column + * @return $this + */ + public function orWhereMorphRelation($relation, $types, $column, $operator = null, $value = null) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\MorphTo|string $relation + * @param (\Closure(Builder): mixed)|string $column + * @return $this + */ + public function whereMorphDoesntHaveRelation($relation, $types, $column, $operator = null, $value = null) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\MorphTo|string $relation + * @param (\Closure(Builder): mixed)|string $column + * @return $this + */ + public function orWhereMorphDoesntHaveRelation($relation, $types, $column, $operator = null, $value = null) { return $this; } + } } @@ -378,48 +410,102 @@ function eagerRelations(): void { assertType('BuilderRelationAudit\TeamBuilder', Team::withWhereHas('stocks', null)); } + /** + * @param class-string $teamClass + * @param list> $classes + * @param list $unknownClasses + */ + function morphCandidates(string $teamClass, array $classes, array $unknownClasses, string $unknown): void { + Comment::whereHasMorph('commentable', $teamClass, function (Builder $query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Comment::whereHasMorph('commentable', $classes, function (Builder $query) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder', $query); + }); + Comment::whereHasMorph('commentable', [Team::class, $unknown], function ($query) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder', $query); + }); + TeamComment::whereHasMorph('commentable', $unknownClasses, function (Builder $query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Comment::whereHasMorph('commentable', [Warehouse::class, Warehouse::class], function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + Comment::whereHasMorph('commentable', UnrelatedQuery::class, function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + Comment::whereHasMorph('commentable', PlainTeam::class, function (Builder $query) { + assertType('BuilderRelationAudit\PlainTeamBuilder', $query); + }); + Comment::whereHasMorph('commentable', 'BuilderRelationAudit\\Team', function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Comment::whereHasMorph('commentable', MissingModel::class, function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + $assigned = Team::class; + Comment::whereHasMorph('commentable', $assigned, function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Comment::whereHasMorph('commentable', Team::class, function (Warehouse $query) { + assertType('BuilderRelationAudit\Warehouse', $query); + }); + Comment::whereMorphRelation(column: function (Builder $query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }, types: Team::class, relation: 'commentable'); + Comment::orWhereMorphRelation(column: function (Builder $query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }, types: Team::class, relation: 'commentable'); + Comment::whereMorphDoesntHaveRelation(column: function (Builder $query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }, types: Team::class, relation: 'commentable'); + Comment::orWhereMorphDoesntHaveRelation(column: function (Builder $query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }, types: Team::class, relation: 'commentable'); + } + 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('Illuminate\Database\Eloquent\Builder', $query); 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('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder', $query); assertType('string', $type); }); Comment::query()->orWhereHasMorph('commentable', Team::class, function ($query, $type) { - assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('BuilderRelationAudit\TeamBuilder', $query); 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('BuilderRelationAudit\TeamBuilder', $query); assertType('string', $type); }); Comment::doesntHaveMorph('commentable', Team::class, 'and', function ($query, $type) { - assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('BuilderRelationAudit\TeamBuilder', $query); assertType('string', $type); }); Comment::whereDoesntHaveMorph('commentable', Team::class, function ($query, $type) { - assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('BuilderRelationAudit\TeamBuilder', $query); assertType('string', $type); }); Comment::orWhereDoesntHaveMorph('commentable', Team::class, function ($query, $type) { - assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('BuilderRelationAudit\TeamBuilder', $query); assertType('string', $type); }); Comment::whereHasMorph(callback: function ($query, $type) { - assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('BuilderRelationAudit\TeamBuilder', $query); 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('Illuminate\Database\Eloquent\Builder', $query); assertType('string', $type); }); Comment::whereHasMorph('commentable', [], function ($query) { - assertType('Illuminate\Database\Eloquent\Builder', $query); // SKIP: morph candidate builders are not inferred + assertType('Illuminate\Database\Eloquent\Builder', $query); }); TeamComment::whereHasMorph('commentable', '*', function ($query) { - assertType('BuilderRelationAudit\TeamBuilder', $query); // SKIP: morph candidate builders are not inferred + assertType('BuilderRelationAudit\TeamBuilder', $query); }); assertType('Illuminate\Database\Eloquent\Builder', Comment::whereHasMorph('commentable', [Team::class], null)); } From a70027416434a8cd7c9083b4f63755ac315f3666 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Sun, 20 Sep 2026 23:54:38 +0600 Subject: [PATCH 6/7] fix(laravel): preserve eager relation callback unions --- docs/CHANGELOG.md | 2 + docs/todo.md | 1 - docs/todo/laravel.md | 22 ------- examples/laravel/app/Demo.php | 6 ++ examples/laravel/assertions.php | 28 +++++++++ .../variable/closure_resolution.rs | 31 ++++++++-- src/virtual_members/laravel/mod.rs | 4 +- src/virtual_members/laravel/relationships.rs | 49 +++++++++++----- .../laravel/relationships_tests.rs | 58 +++++++++++++++++++ tests/integration/completion_laravel.rs | 49 ++++++++++++++++ .../laravel-builder-relations.php | 50 +++++++++++++--- 11 files changed, 248 insertions(+), 52 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 6d81f0e0f..1a448aa93 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -39,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Eager relation constraints keep both callback types.** `withWhereHas()` and `withWhereRelation()` now retain the related builder and relation through fluent calls, including dotted paths and explicit union hints. Contributed by @shuvroroy. + - **Morph relation callbacks infer their candidate models.** Completion and diagnostics retain custom builders for polymorphic constraints, including unions and class-string variables, while unknown candidates use the relation’s declared model. Contributed by @shuvroroy. - **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. diff --git a/docs/todo.md b/docs/todo.md index 6bffb9dfe..de47e629c 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -161,7 +161,6 @@ 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 | -| 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 cac1f0972..a1adb75a1 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -247,28 +247,6 @@ the collection return-type patches for the argument forms. The property lookup already exists for `model-property`; the work is threading a resolved key type through the generic substitution. -#### 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 **Impact: Low-Medium · Complexity: High** diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index 92290695b..ec68c412f 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -491,6 +491,12 @@ public function eloquentClosure(): void }); Bakery::query()->whereHas('headBaker', fn ($q) => $q->active()); // → BakerBuilder + // The same constraint runs on a builder and the eager-loaded relation. + Bakery::withWhereHas(callback: function (Builder|Relation $q) { + $q->where('weight_grams', '>', 500); // → LoafBuilder|HasMany + }, relation: 'baguettes'); + BlogPost::withWhereRelation('author.posts', fn ($q) => $q->where('published', true)); // → Builder|HasMany + // Morph candidates choose the callback builder and keep the type string. Review::whereHasMorph('reviewable', Loaf::class, function (Builder $q, string $type) { $q->stale(); // → LoafBuilder diff --git a/examples/laravel/assertions.php b/examples/laravel/assertions.php index 97c387366..478b3c374 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -274,6 +274,34 @@ function assertMethodReturnType(string $class, string $method, string $expected) $callbackClasses === [\Illuminate\Database\Eloquent\Builder::class, \Illuminate\Database\Eloquent\Relations\HasMany::class] ); +foreach (['withWhereHas', 'withWhereRelation'] as $method) { + $seen = []; + $constraint = function (\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Eloquent\Relations\Relation $query) use (&$seen) { + $seen[] = [get_class($query), get_class($query->where('weight_grams', '>', 500)), get_class($query->getModel())]; + }; + $query = \App\Models\Bakery::$method('baguettes', $constraint); + $query->getEagerLoads()['baguettes']((new \App\Models\Bakery())->baguettes()); + check("$method preserves the custom builder and relation through fluent calls", $seen === [ + [\App\Models\LoafBuilder::class, \App\Models\LoafBuilder::class, \App\Models\Loaf::class], + [\Illuminate\Database\Eloquent\Relations\HasMany::class, \Illuminate\Database\Eloquent\Relations\HasMany::class, \App\Models\Loaf::class], + ]); +} +$seen = []; +$query = \App\Models\BlogPost::withWhereRelation('author.posts', function ($query) use (&$seen) { + $seen[] = [get_class($query), get_class($query->getModel())]; +}); +$terminal = (new \App\Models\BlogAuthor())->posts(); +$query->getEagerLoads()['author.posts']($terminal); +check('Dotted eager callbacks use the terminal relation and declaring model', $seen === [ + [\Illuminate\Database\Eloquent\Builder::class, \App\Models\BlogPost::class], + [\Illuminate\Database\Eloquent\Relations\HasMany::class, \App\Models\BlogPost::class], +] && $terminal->getParent() instanceof \App\Models\BlogAuthor); +$seen = []; +$query = \App\Models\Bakery::withWhereHas(callback: function ($query) use (&$seen) { + $seen[] = get_class($query); +}, relation: 'baguettes:id,weight_grams'); +check('withWhereHas strips column selections for its existence query', $seen === [\App\Models\LoafBuilder::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]; diff --git a/src/type_engine/variable/closure_resolution.rs b/src/type_engine/variable/closure_resolution.rs index fcede046d..0a04f9e5c 100644 --- a/src/type_engine/variable/closure_resolution.rs +++ b/src/type_engine/variable/closure_resolution.rs @@ -23,7 +23,7 @@ use crate::php_type::{PhpType, TypeKind}; use crate::type_engine::resolver::ResolutionCtx; use crate::virtual_members::laravel::{ ELOQUENT_BUILDER_FQN, ELOQUENT_MODEL_FQN, RELATION_QUERY_METHODS, extends_eloquent_model, - model_builder_type, resolve_relation_chain, + model_builder_type, resolve_relation_chain_details, }; thread_local! { @@ -755,6 +755,11 @@ fn inferred_type_is_more_specific( .iter() .all(|part| inferred_type_is_more_specific(explicit_hint, part, class_loader)); } + if let TypeKind::Union(parts) = explicit_hint.kind() { + return parts + .iter() + .any(|part| inferred_type_is_more_specific(part, inferred, class_loader)); + } // The explicit hint must be a bare name (no generic args). let explicit_base = match explicit_hint.kind() { TypeKind::Named(name) => name.as_str(), @@ -841,12 +846,20 @@ fn try_relation_query_override( // `Builder` instance. let model = find_model_from_receivers(receivers, class_loader)?; - let related = resolve_relation_chain(&model, relation_name, class_loader, None) - .and_then(|name| class_loader(&name)); + // Only withWhereHas strips a column-selection suffix for its existence + // query; withWhereRelation passes the relation name through unchanged. + let chain = if method_name == "withWhereHas" { + relation_name + .split_once(':') + .map_or(relation_name, |(name, _)| name) + } else { + relation_name + }; + let related = resolve_relation_chain_details(&model, chain, class_loader, None); if let Some(candidates) = candidates { let fallback = related .as_ref() - .map(|model| model_builder_type(model, class_loader)) + .map(|relation| model_builder_type(&relation.model, class_loader)) .unwrap_or_else(|| { PhpType::generic( ELOQUENT_BUILDER_FQN, @@ -859,7 +872,15 @@ fn try_relation_query_override( class_loader, )]); } - Some(vec![model_builder_type(related.as_deref()?, class_loader)]) + let related = related?; + let builder = model_builder_type(&related.model, class_loader); + Some(vec![ + if matches!(method_name, "withWhereHas" | "withWhereRelation") { + PhpType::union(vec![builder, related.relation_type]) + } else { + builder + }, + ]) } /// Map candidate class-strings through the same builder selector as model diff --git a/src/virtual_members/laravel/mod.rs b/src/virtual_members/laravel/mod.rs index 8e85d473e..68c5c7625 100644 --- a/src/virtual_members/laravel/mod.rs +++ b/src/virtual_members/laravel/mod.rs @@ -214,7 +214,9 @@ pub(crate) use relationships::class_has_relation_method_ci; pub(crate) use relationships::classify_relationship_typed; pub(crate) use relationships::count_property_to_relationship_method; pub use relationships::infer_relationship_from_body; -pub(crate) use relationships::{RELATION_QUERY_METHODS, resolve_relation_chain}; +pub(crate) use relationships::{ + RELATION_QUERY_METHODS, resolve_relation_chain, resolve_relation_chain_details, +}; use relationships::{ RelationshipKind, build_property_type, count_property_name, extract_pivot_accessor_typed, extract_related_type_typed, diff --git a/src/virtual_members/laravel/relationships.rs b/src/virtual_members/laravel/relationships.rs index ea75f5436..e2d74289d 100644 --- a/src/virtual_members/laravel/relationships.rs +++ b/src/virtual_members/laravel/relationships.rs @@ -32,6 +32,7 @@ pub(crate) const RELATION_QUERY_METHODS: &[&str] = &[ "whereHas", "orWhereHas", "withWhereHas", + "withWhereRelation", "whereDoesntHave", "orWhereDoesntHave", "whereRelation", @@ -511,30 +512,46 @@ pub(crate) fn resolve_relation_chain( class_loader: &dyn Fn(&str) -> Option>, cache: Option<&super::super::ResolvedClassCache>, ) -> Option { - let segments: Vec<&str> = chain.split('.').collect(); - if segments.is_empty() { - return None; - } + resolve_relation_chain_details(model, chain, class_loader, cache) + .map(|relation| relation.model.fqn().to_string()) +} + +/// The terminal relationship and related model of a dotted relation path. +pub(crate) struct ResolvedRelation { + /// The final related model, used to select its query builder. + pub model: Arc, + /// The relationship with self references bound to its declaring model. + pub relation_type: PhpType, +} +/// Resolve a dotted path while retaining the terminal relationship's type. +/// Eager constraints receive this relationship, constructed on the model at +/// the preceding segment, as well as its builder for the existence query. +pub(crate) fn resolve_relation_chain_details( + model: &ClassInfo, + chain: &str, + class_loader: &dyn Fn(&str) -> Option>, + cache: Option<&super::super::ResolvedClassCache>, +) -> Option { + let mut segments = chain.split('.').peekable(); let mut current_class = resolve_class_with_inheritance(model, class_loader, cache); - for segment in &segments { + while let Some(segment) = segments.next() { let segment = segment.trim(); if segment.is_empty() { return None; } - - let method = current_class.get_method(segment)?; - - // Body-inferred relationship types are already stored in - // `return_type` by the parser, so no fallback is needed. - let return_type = method.return_type.as_ref()?; + let return_type = current_class.get_method(segment)?.return_type.as_ref()?; let related_type = extract_related_type_for_chain(return_type, ¤t_class)?; - - let resolved = resolve_related_fqn(&related_type, ¤t_class, class_loader)?; - current_class = resolve_class_with_inheritance(&resolved, class_loader, cache); + let related = resolve_related_fqn(&related_type, ¤t_class, class_loader)?; + if segments.peek().is_none() { + return Some(ResolvedRelation { + model: related, + relation_type: return_type.replace_self_bound(¤t_class.fqn(), None), + }); + } + current_class = resolve_class_with_inheritance(&related, class_loader, cache); } - - Some(current_class.fqn().to_string()) + None } /// Resolve a class fully (with inheritance and virtual members) so that diff --git a/src/virtual_members/laravel/relationships_tests.rs b/src/virtual_members/laravel/relationships_tests.rs index e663dffca..d37bf0356 100644 --- a/src/virtual_members/laravel/relationships_tests.rs +++ b/src/virtual_members/laravel/relationships_tests.rs @@ -805,3 +805,61 @@ fn with_pivot_absent_returns_empty() { let body = "{ return $this->belongsToMany(Role::class); }"; assert!(extract_with_pivot_columns(body).is_empty()); } + +#[test] +fn relation_chain_details_keep_the_terminal_declaring_model() { + let mut team = make_class("Team"); + for (name, return_type) in [ + ("stocks", Some("HasMany")), + ("scalar", Some("string")), + ("untyped", None), + ("unknown", Some("HasMany")), + ] { + team.methods.push(Arc::new(make_method(name, return_type))); + } + let mut stock = make_class("Stock"); + stock.methods.push(Arc::new(make_method( + "warehouse", + Some("BelongsTo"), + ))); + let mut warehouse = make_class("Warehouse"); + warehouse.methods.push(Arc::new(make_method( + "parent", + Some("HasOne<$this, static>"), + ))); + let classes = [Arc::new(team), Arc::new(stock), Arc::new(warehouse)]; + let loader = |name: &str| classes.iter().find(|class| class.fqn() == name).cloned(); + let relation = + resolve_relation_chain_details(&classes[0], " stocks . warehouse ", &loader, None).unwrap(); + assert_eq!(relation.model.fqn(), "Warehouse"); + assert_eq!( + relation.relation_type, + PhpType::parse("BelongsTo") + ); + let relation = + resolve_relation_chain_details(&classes[0], "stocks.warehouse.parent", &loader, None) + .unwrap(); + assert_eq!( + relation.relation_type, + PhpType::parse("HasOne") + ); + assert_eq!( + resolve_relation_chain(&classes[0], "stocks.warehouse", &loader, None).as_deref(), + Some("Warehouse") + ); + for chain in [ + "", + ".stocks", + "stocks.", + "stocks..warehouse", + "stocks.missing", + "scalar", + "untyped", + "unknown", + ] { + assert!( + resolve_relation_chain_details(&classes[0], chain, &loader, None).is_none(), + "{chain}" + ); + } +} diff --git a/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index c5151b5a9..3505f8685 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -14992,6 +14992,55 @@ async fn test_morph_callback_candidates_reach_completion_and_diagnostics() { } } +#[tokio::test] +async fn test_eager_callback_union_reaches_completion_and_diagnostics() { + let fixture = include_str!("../phpstan_nsrt/laravel-builder-relations.php"); + let backend = create_test_backend(); + let uri = Url::parse("file:///eager-callbacks.php").unwrap(); + for call in [ + "Stock::withWhereHas('team', fn ($eagerQuery) => BODY);", + "Stock::query()->withWhereHas(callback: function (Builder|Relation $eagerQuery) { BODY }, relation: 'team');", + "Team::withWhereHas('stocks.team:id', fn ($eagerQuery) => BODY);", + "Stock::withWhereRelation('team', function (Builder|Relation $eagerQuery) { BODY });", + "Stock::query()?->withWhereRelation(column: fn ($eagerQuery) => BODY, relation: 'team');", + ] { + let source = format!( + "{fixture}\nnamespace BuilderRelationAudit {{ use Illuminate\\Database\\Eloquent\\Builder; use Illuminate\\Database\\Eloquent\\Relations\\Relation; {call} }}" + ); + let content = source.replace("BODY", "$eagerQuery->where('id', 1)->"); + let position = crate::common::position_after(&content, "$eagerQuery->where('id', 1)->"); + let items = + crate::common::complete_at(&backend, &uri, &content, position.line, position.character) + .await; + let methods = method_names(&items); + assert!(methods.contains(&"active"), "{call}: {methods:?}"); + assert!(methods.contains(&"where"), "{call}: {methods:?}"); + let body = if call.contains("fn (") { + "$eagerQuery->where('id', 1)->firstOrFail()->teamName()" + } else { + "$eagerQuery->where('id', 1)->firstOrFail()->teamName();" + }; + let content = source.replace("BODY", body); + 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( + "firstOrFail()->teamName()", + "firstOrFail()->missingEagerModelMethod()", + ); + 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("missingEagerModelMethod")); + } +} + #[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 index dcb676892..7d09dbf0a 100644 --- a/tests/phpstan_nsrt/laravel-builder-relations.php +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -4,8 +4,7 @@ // relationship-query-callbacks.php at c328727e6103c1147d1c64cc96b3aedfda26bc20. // Framework declarations retain Laravel's callable signatures; none of the // model-specific expectations are supplied by the stubs themselves. -// Known gaps retain the desired type with the corpus's SKIP marker. Arrow -// callbacks are covered in completion_laravel.rs because this runner replaces +// Arrow callbacks are covered in completion_laravel.rs because this runner replaces // the whole assertion line, which would erase an enclosing arrow expression. namespace Illuminate\Support\Traits { @@ -26,6 +25,10 @@ public function count(): int { return 0; } } } +namespace Illuminate\Contracts\Database\Eloquent { + interface Builder {} +} + namespace Illuminate\Database\Eloquent { class Model { /** @return Builder */ @@ -42,7 +45,7 @@ public function newModelQuery() {} * @template TModel of Model * @mixin \Illuminate\Database\Query\Builder */ - class Builder { + class Builder implements \Illuminate\Contracts\Database\Eloquent\Builder { use \Illuminate\Support\Traits\ForwardsCalls; /** @return $this */ @@ -110,6 +113,10 @@ public function orWhereDoesntHave($relation, ?\Closure $callback = null) { retur */ public function withWhereHas($relation, ?\Closure $callback = null, $operator = '>=', $count = 1) { return $this; } + /** @return $this */ + public function withWhereRelation($relation, $column, $operator = null, $value = null) { return $this; } + + /** * @template TRelatedModel of Model * @param Relations\MorphTo|string $relation @@ -205,7 +212,7 @@ public function orWhereMorphDoesntHaveRelation($relation, $types, $column, $oper * @template TResult * @mixin \Illuminate\Database\Eloquent\Builder */ - class Relation { + class Relation implements \Illuminate\Contracts\Database\Eloquent\Builder { use \Illuminate\Support\Traits\ForwardsCalls; } /** @@ -229,6 +236,8 @@ class MorphTo extends Relation {} } namespace BuilderRelationAudit { + use Illuminate\Database\Eloquent\Relations\Relation; + use Illuminate\Contracts\Database\Eloquent\Builder as BuilderContract; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Builder as Query; use Illuminate\Database\Eloquent\Model; @@ -399,15 +408,42 @@ function relations(): void { 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 + assertType('Illuminate\Database\Eloquent\Builder|Illuminate\Database\Eloquent\Relations\HasMany', $query); }); 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 + assertType('Illuminate\Database\Eloquent\Builder|Illuminate\Database\Eloquent\Relations\BelongsTo', $query); }); 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('Illuminate\Database\Eloquent\Builder|Illuminate\Database\Eloquent\Relations\HasMany', $query); }); assertType('BuilderRelationAudit\TeamBuilder', Team::withWhereHas('stocks', null)); + Stock::withWhereHas(callback: function (Builder|Relation $query) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Relations\BelongsTo', $query); + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Relations\BelongsTo', $query->where('active', true)); + assertType('BuilderRelationAudit\Team', $query->where('active', true)->firstOrFail()); + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Relations\BelongsTo', $query->orderBy('id')); + assertType('int', $query->count()); + }, relation: 'team'); + Stock::withWhereHas('plainTeam', function (BuilderContract $query) { + assertType('BuilderRelationAudit\PlainTeamBuilder|Illuminate\Database\Eloquent\Relations\BelongsTo', $query); + }); + Team::withWhereHas('stocks.team:id', function ($query) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Relations\BelongsTo', $query); + }); + ChildTeam::withWhereHas('stocks', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder|Illuminate\Database\Eloquent\Relations\HasMany', $query); + }); + Stock::query()->withWhereRelation(column: function (Builder|Relation $query) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Relations\BelongsTo', $query); + }, relation: 'team'); + Team::withWhereHas('stocks', function (Warehouse $query) { + assertType('BuilderRelationAudit\Warehouse', $query); + }); + Team::whereHas('stocks', function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query->where('id', 1)); + }); + assertType('BuilderRelationAudit\TeamBuilder', Team::withWhereRelation('stocks', 'id', '>', 0)); + } /** From b439a4b90d5533a3521e83b1221ba96565a5cad3 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Mon, 21 Sep 2026 00:12:32 +0600 Subject: [PATCH 7/7] fix(laravel): infer relation callbacks from argument types Complete L60 by resolving relation-name variables, unions, and relation objects through the shared type engine. Retain custom builders for relation shortcuts and concrete relations for direct eager callbacks. Cover callback types, completion, diagnostics, navigation, and Laravel runtime behavior. Keep this follow-up separate from L59. --- docs/CHANGELOG.md | 2 + examples/laravel/app/Demo.php | 24 +++ examples/laravel/assertions.php | 36 +++++ .../variable/closure_resolution.rs | 152 +++++++++++------- .../forward_walk/callable_inference.rs | 34 ++-- src/virtual_members/laravel/relationships.rs | 15 +- tests/integration/completion_laravel.rs | 67 ++++++++ tests/integration/definition_laravel.rs | 23 +++ .../laravel-builder-relations.php | 118 +++++++++++++- 9 files changed, 385 insertions(+), 86 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 1a448aa93..3628a8f7b 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -39,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Relation callbacks follow their argument types.** Completion and diagnostics keep related model types when constraints use relation-name variables, unions, or relation objects. Relation shortcuts and direct eager-loading callbacks now retain their concrete builder or relation too. Contributed by @shuvroroy. + - **Eager relation constraints keep both callback types.** `withWhereHas()` and `withWhereRelation()` now retain the related builder and relation through fluent calls, including dotted paths and explicit union hints. Contributed by @shuvroroy. - **Morph relation callbacks infer their candidate models.** Completion and diagnostics retain custom builders for polymorphic constraints, including unions and class-string variables, while unknown candidates use the relation’s declared model. Contributed by @shuvroroy. diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index ec68c412f..d9ce05913 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -504,6 +504,30 @@ public function eloquentClosure(): void }); } + public function eloquentRelationArguments(bool $includeBaker): void + { + $name = 'headBaker'; + $bakery = new Bakery(); + Bakery::whereHas($name, fn ($q) => $q->active()); // → BakerBuilder + Bakery::whereHas($bakery->headBaker(), fn ($q) => $q->active()); // → BakerBuilder + Bakery::orWhereRelation($name, fn ($q) => $q->active()); + Bakery::whereDoesntHaveRelation($bakery->headBaker(), fn ($q) => $q->active()); + Bakery::orWhereDoesntHaveRelation(column: fn ($q) => $q->active(), relation: $name); + + // Each possible name contributes its builder and eager relation. + $relation = $includeBaker ? 'headBaker' : 'baguettes'; + Bakery::withWhereHas($relation, fn ($q) => $q->where('id', '>', 0)); + + // Direct eager callbacks receive a relation and retain its related model. + Bakery::query()->with(callback: function (Relation $q) { + $q->where('active', true)->getModel()->getName(); // → string (Baker::getName()) + }, relations: $name); // → HasOne + + Review::whereHasMorph((new Review())->reviewable(), Loaf::class, function ($q, $type) { + $q->stale(); // → LoafBuilder + echo $type; // → string + }); + } // ── Laravel Config & Env Navigation ───────────────────────────────────── diff --git a/examples/laravel/assertions.php b/examples/laravel/assertions.php index 478b3c374..01d900273 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -263,6 +263,42 @@ function assertMethodReturnType(string $class, string $method, string $expected) }, relation: 'posts.author'); check('Reordered named relation arguments preserve the callback model', $namedCallbackModels === [\App\Models\BlogAuthor::class]); +$relationName = 'headBaker'; +foreach (['whereHas', 'orWhereRelation', 'whereDoesntHaveRelation', 'orWhereDoesntHaveRelation'] as $method) { + foreach ([$relationName, (new \App\Models\Bakery())->headBaker()] as $relationArgument) { + $seen = []; + \App\Models\Bakery::$method($relationArgument, function ($query) use (&$seen) { + $seen[] = [get_class($query->active()), get_class($query->getModel())]; + }); + check("$method accepts a relation name or object with the custom builder", $seen === [[\App\Models\BakerBuilder::class, \App\Models\Baker::class]]); + } +} +foreach (['headBaker', 'baguettes'] as $relationArgument) { + $seen = []; + $query = \App\Models\Bakery::withWhereHas($relationArgument, function ($query) use (&$seen) { + $seen[] = [get_class($query->where('id', '>', 0)), get_class($query->getModel())]; + }); + $query->getEagerLoads()[$relationArgument]((new \App\Models\Bakery())->$relationArgument()); + $isBaker = $relationArgument === 'headBaker'; + $expectedModel = $isBaker ? \App\Models\Baker::class : \App\Models\Loaf::class; + check('Every alternative relation name supplies its builder and relation', $seen === [ + [$isBaker ? \App\Models\BakerBuilder::class : \App\Models\LoafBuilder::class, $expectedModel], + [$isBaker ? \Illuminate\Database\Eloquent\Relations\HasOne::class : \Illuminate\Database\Eloquent\Relations\HasMany::class, $expectedModel], + ]); +} +$seen = []; +$query = \App\Models\Bakery::query()->with(callback: function (\Illuminate\Database\Eloquent\Relations\Relation $query) use (&$seen) { + $seen[] = [get_class($query->where('active', true)), get_class($query->getModel()), is_string($query->getModel()->getName())]; +}, relations: $relationName); +check('Direct eager constraints are deferred until eager loading', $seen === []); +$query->getEagerLoads()[$relationName]((new \App\Models\Bakery())->headBaker()); +check('Named direct eager callbacks retain their relation and model', $seen === [[\Illuminate\Database\Eloquent\Relations\HasOne::class, \App\Models\Baker::class, true]]); +$seen = []; +\App\Models\Review::whereHasMorph((new \App\Models\Review())->reviewable(), \App\Models\Loaf::class, function ($query, $type) use (&$seen) { + $seen[] = [get_class($query->stale()), $type]; +}); +check('Morph relation objects retain candidate builders and the type string', $seen === [[\App\Models\LoafBuilder::class, \App\Models\Loaf::class]]); + // Check both callback invocations without executing the eager-load SQL. $callbackClasses = []; $query = \App\Models\BlogAuthor::withWhereHas('posts', function ($related) use (&$callbackClasses) { diff --git a/src/type_engine/variable/closure_resolution.rs b/src/type_engine/variable/closure_resolution.rs index 0a04f9e5c..9f1cf91a6 100644 --- a/src/type_engine/variable/closure_resolution.rs +++ b/src/type_engine/variable/closure_resolution.rs @@ -815,72 +815,108 @@ fn describes_elements(ty: &PhpType, allow_iterable: bool) -> bool { // ── Callable parameter inference helpers ──────────────────────────── -/// 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 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. +/// Infer an Eloquent constraint's query from the bound relation argument. +/// Names retain their literal alternatives through the shared expression +/// resolver; relation objects supply their own related-model binding. fn try_relation_query_override( receivers: &[ResolvedType], method_name: &str, - relation_name: Option<&str>, + relation: &PhpType, candidates: Option<&PhpType>, - class_loader: &dyn Fn(&str) -> Option>, + rctx: &ResolutionCtx<'_>, ) -> Option> { if !RELATION_QUERY_METHODS.contains(&method_name) { return None; } - - let relation_name = relation_name?; - if relation_name.is_empty() { - return None; + let model = find_model_from_receivers(receivers, rctx.class_loader)?; + let query = relation_callback_type(&model, method_name, relation, rctx); + if let Some(candidates) = candidates { + let fallback = query.unwrap_or_else(|| { + PhpType::generic( + ELOQUENT_BUILDER_FQN, + vec![PhpType::named(atom(ELOQUENT_MODEL_FQN))], + ) + }); + return Some(vec![ + morph_candidate_builders(candidates, &fallback, rctx.class_loader).simplified(), + ]); } + Some(vec![query?]) +} - // 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(receivers, class_loader)?; - - // Only withWhereHas strips a column-selection suffix for its existence - // query; withWhereRelation passes the relation name through unchanged. - let chain = if method_name == "withWhereHas" { - relation_name - .split_once(':') - .map_or(relation_name, |(name, _)| name) - } else { - relation_name - }; - let related = resolve_relation_chain_details(&model, chain, class_loader, None); - if let Some(candidates) = candidates { - let fallback = related - .as_ref() - .map(|relation| model_builder_type(&relation.model, class_loader)) - .unwrap_or_else(|| { - PhpType::generic( - ELOQUENT_BUILDER_FQN, - vec![PhpType::named(atom(ELOQUENT_MODEL_FQN))], - ) - }); - return Some(vec![morph_candidate_builders( - candidates, - &fallback, - class_loader, - )]); +/// Combine known relation alternatives without replacing unresolved eager +/// constraints with the outer model's builder. Their declaration remains the +/// fallback when the relation name cannot be determined. +fn relation_callback_type( + model: &ClassInfo, + method_name: &str, + relation: &PhpType, + rctx: &ResolutionCtx<'_>, +) -> Option { + if let TypeKind::Union(parts) = relation.kind() { + let queries: Vec<_> = parts + .iter() + .filter_map(|part| relation_callback_type(model, method_name, part, rctx)) + .collect(); + return (!queries.is_empty()).then(|| PhpType::union(queries).simplified()); } - let related = related?; - let builder = model_builder_type(&related.model, class_loader); - Some(vec![ - if matches!(method_name, "withWhereHas" | "withWhereRelation") { + let eager = matches!(method_name, "with" | "withWhereHas" | "withWhereRelation"); + if let Some(name) = relation + .as_literal() + .and_then(|value| value.string_content()) + { + // `with` and `withWhereHas` understand column selections. The + // existence half of withWhereRelation receives its name unchanged. + let chain = if matches!(method_name, "with" | "withWhereHas") { + name.split_once(':').map_or(name.as_ref(), |(name, _)| name) + } else { + name.as_ref() + }; + let related = resolve_relation_chain_details( + model, + chain, + rctx.class_loader, + rctx.resolved_class_cache, + )?; + if method_name == "with" { + return Some(related.relation_type); + } + let builder = model_builder_type(&related.model, rctx.class_loader); + return Some(if eager { PhpType::union(vec![builder, related.relation_type]) } else { builder - }, - ]) + }); + } + if eager { + return None; + } + if relation.is_string_type() { + return Some(model_builder_type(model, rctx.class_loader)); + } + + // Resolve the ancestor's actual template argument rather than assuming + // that a custom relation puts its related model in the first slot. + const RELATION_FQN: &str = "Illuminate\\Database\\Eloquent\\Relations\\Relation"; + if !crate::class_lookup::is_subtype_of_typed( + relation, + &PhpType::named(atom(RELATION_FQN)), + rctx.class_loader, + ) { + return None; + } + let mut related = + super::rhs_resolution::extract_generic_arg_from_ancestor(relation, RELATION_FQN, 0, rctx)?; + if let TypeKind::Generic(g) = relation.kind() + && let Some(class) = (rctx.class_loader)(&g.name) + { + related = related.substitute(&crate::inheritance::build_generic_subs(&class, &g.args)); + } + let fallback = PhpType::generic( + ELOQUENT_BUILDER_FQN, + vec![PhpType::named(atom(ELOQUENT_MODEL_FQN))], + ); + Some(morph_model_builders(&related, &fallback, rctx.class_loader)) } /// Map candidate class-strings through the same builder selector as model @@ -1108,17 +1144,11 @@ fn extract_model_from_builder(builder: &ClassInfo) -> Option { pub(in crate::type_engine) fn try_relation_query_override_pub( receivers: &[ResolvedType], method_name: &str, - relation_name: Option<&str>, + relation: &PhpType, candidates: Option<&PhpType>, - class_loader: &dyn Fn(&str) -> Option>, + rctx: &ResolutionCtx<'_>, ) -> Option> { - try_relation_query_override( - receivers, - method_name, - relation_name, - candidates, - class_loader, - ) + try_relation_query_override(receivers, method_name, relation, candidates, rctx) } pub(in crate::type_engine) fn build_receiver_self_type_pub( diff --git a/src/type_engine/variable/forward_walk/callable_inference.rs b/src/type_engine/variable/forward_walk/callable_inference.rs index 7743c087a..78cde1392 100644 --- a/src/type_engine/variable/forward_walk/callable_inference.rs +++ b/src/type_engine/variable/forward_walk/callable_inference.rs @@ -544,6 +544,11 @@ fn infer_relation_callback_params( } else { "callback" }; + let relation_name = if method_name == "with" { + "relations" + } else { + "relation" + }; let argument = argument_list.arguments.iter().nth(arg_idx)?.value(); for receiver in receivers { let Some(class) = receiver.class_info.as_ref() else { @@ -565,7 +570,7 @@ fn infer_relation_callback_params( .position(|p| p.name.trim_start_matches('$') == name) }; let (Some(relation_idx), Some(callback_idx)) = - (param_index("relation"), param_index(callback_name)) + (param_index(relation_name), param_index(callback_name)) else { continue; }; @@ -573,21 +578,18 @@ fn infer_relation_callback_params( 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; - }; + let relation = bound[relation_idx]?; + let scope_resolver = + |name: &str| scope.locals.get(&atom(name)).cloned().unwrap_or_default(); + let var_ctx = ctx.var_ctx_for_with_scope( + "$__infer", + relation.span().start.offset, + &scope_resolver, + Some(scope.proofs()), + ); + let relation_type = super::super::resolution::resolve_arg_raw_type(relation, &var_ctx)?; let candidates = if method_name.contains("Morph") { let types = bound[param_index("types")?]?; - let scope_resolver = - |name: &str| scope.locals.get(&atom(name)).cloned().unwrap_or_default(); - let var_ctx = ctx.var_ctx_for_with_scope( - "$__infer", - types.span().start.offset, - &scope_resolver, - Some(scope.proofs()), - ); Some( super::super::resolution::resolve_arg_raw_type(types, &var_ctx) .unwrap_or_else(PhpType::mixed), @@ -598,9 +600,9 @@ fn infer_relation_callback_params( let mut inferred = super::super::closure_resolution::try_relation_query_override_pub( receivers, method_name, - relation.value.map(bytes_to_str), + &relation_type, candidates.as_ref(), - ctx.class_loader, + &var_ctx.as_resolution_ctx(), )?; // Refine the query parameter without losing Laravel's second morph // callback parameter (the candidate's class-string). diff --git a/src/virtual_members/laravel/relationships.rs b/src/virtual_members/laravel/relationships.rs index e2d74289d..edab4b37b 100644 --- a/src/virtual_members/laravel/relationships.rs +++ b/src/virtual_members/laravel/relationships.rs @@ -16,14 +16,9 @@ use crate::util::{short_name, strip_fqn_prefix}; use super::helpers::{camel_to_snake, snake_to_camel}; -/// Methods on `Builder` / `QueriesRelationships` that accept a relation -/// name string as the first argument and a closure typed as -/// `Closure(Builder): mixed` as the second argument -/// (or at the listed position). -/// -/// When one of these methods is detected, the closure parameter -/// inference overrides `TModel` with the related model resolved from -/// the relation name string. +/// Eloquent query methods whose constraint receiver depends on a relation +/// argument. Argument binding locates each method's relation and callback; +/// callable inference refines the builder, eager relation, or both. pub(crate) const RELATION_QUERY_METHODS: &[&str] = &[ "has", "orHas", @@ -31,11 +26,15 @@ pub(crate) const RELATION_QUERY_METHODS: &[&str] = &[ "orDoesntHave", "whereHas", "orWhereHas", + "with", "withWhereHas", "withWhereRelation", "whereDoesntHave", "orWhereDoesntHave", "whereRelation", + "orWhereRelation", + "whereDoesntHaveRelation", + "orWhereDoesntHaveRelation", "hasMorph", "doesntHaveMorph", "whereHasMorph", diff --git a/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index 3505f8685..22afd07a9 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -15041,6 +15041,73 @@ async fn test_eager_callback_union_reaches_completion_and_diagnostics() { } } +#[tokio::test] +async fn test_relation_callback_arguments_reach_completion_and_diagnostics() { + let fixture = include_str!("../phpstan_nsrt/laravel-builder-relations.php"); + let backend = create_test_backend(); + let uri = Url::parse("file:///relation-arguments.php").unwrap(); + for call in [ + "Stock::whereHas($name, fn ($argumentQuery) => BODY);", + "Stock::query()?->whereHas(callback: function (Builder $argumentQuery) { BODY }, relation: $name);", + "Stock::whereHas($stock->team(), function ($argumentQuery) { BODY });", + "Stock::query()->whereHas(callback: fn ($argumentQuery) => BODY, relation: $stock->team());", + "Stock::orWhereRelation($name, fn ($argumentQuery) => BODY);", + "Stock::whereDoesntHaveRelation($stock->team(), function (Builder $argumentQuery) { BODY });", + "Stock::orWhereDoesntHaveRelation(column: fn ($argumentQuery) => BODY, relation: $name);", + "TeamComment::whereHasMorph($comment->commentable(), Team::class, function ($argumentQuery, $type) { BODY });", + "TeamComment::query()->whereHasMorph(callback: fn ($argumentQuery) => BODY, relation: $comment->commentable(), types: '*');", + "Stock::withWhereHas($name, function (Builder|Relation $argumentQuery) { BODY });", + "Stock::query()->withWhereRelation(column: fn ($argumentQuery) => BODY, relation: $name);", + "Stock::query()->with($name, function (Relation $argumentQuery) { BODY });", + "Stock::query()?->with(callback: fn ($argumentQuery) => BODY, relations: $name);", + ] { + let source = format!( + "{fixture}\nnamespace BuilderRelationAudit {{ use Illuminate\\Database\\Eloquent\\Builder; use Illuminate\\Database\\Eloquent\\Relations\\Relation; $name = 'team'; $stock = new Stock(); $comment = new TeamComment(); {call} }}" + ); + let content = source.replace("BODY", "$argumentQuery->where('id', 1)->firstOrFail()->"); + let position = crate::common::position_after( + &content, + "$argumentQuery->where('id', 1)->firstOrFail()->", + ); + let items = + crate::common::complete_at(&backend, &uri, &content, position.line, position.character) + .await; + let methods = method_names(&items); + assert!(methods.contains(&"teamName"), "{call}: {methods:?}"); + assert!( + !methods.contains(&"warehouseName"), + "{call}: unrelated model leaked" + ); + let body = if call.contains("fn (") { + "$argumentQuery->where('id', 1)->firstOrFail()->teamName()" + } else { + "$argumentQuery->where('id', 1)->firstOrFail()->teamName();" + }; + let content = source.replace("BODY", body); + 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( + "firstOrFail()->teamName()", + "firstOrFail()->missingRelationArgumentMethod()", + ); + 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("missingRelationArgumentMethod") + ); + } +} + #[tokio::test] async fn test_where_has_closure_resolves_to_related_model() { // Brand::whereHas('orders', function ($q) { $q-> }) diff --git a/tests/integration/definition_laravel.rs b/tests/integration/definition_laravel.rs index ad1529af2..1d5a3a288 100644 --- a/tests/integration/definition_laravel.rs +++ b/tests/integration/definition_laravel.rs @@ -4373,3 +4373,26 @@ async fn test_goto_definition_blade_each_directive() { target_uri ); } + +#[tokio::test] +async fn test_relation_argument_callback_navigates_to_custom_builder() { + let fixture = include_str!("../phpstan_nsrt/laravel-builder-relations.php"); + let backend = crate::common::create_test_backend(); + let uri = Url::parse("file:///relation-argument-definition.php").unwrap(); + let expected = crate::common::position_after(fixture, "public function active"); + for relation in ["$name", "$stock->team()"] { + let content = format!( + "{fixture}\nnamespace BuilderRelationAudit {{ $name = 'team'; $stock = new Stock(); Stock::whereHas({relation}, fn ($resolved) => $resolved->active()); }}" + ); + open_php(&backend, &uri, &content).await; + let position = crate::common::position_after(&content, "$resolved->act"); + let result = goto_definition_at(&backend, &uri, position.line, position.character).await; + let locations = crate::common::definition_locations(result); + assert_eq!(locations.len(), 1, "{relation}: {locations:?}"); + assert_eq!(locations[0].uri, uri); + assert_eq!( + locations[0].range.start.line, expected.line, + "{relation}: {locations:?}" + ); + } +} diff --git a/tests/phpstan_nsrt/laravel-builder-relations.php b/tests/phpstan_nsrt/laravel-builder-relations.php index 7d09dbf0a..c4312fad6 100644 --- a/tests/phpstan_nsrt/laravel-builder-relations.php +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -34,6 +34,8 @@ class Model { /** @return Builder */ public static function query() {} /** @return Builder */ + public static function with($relations) {} + /** @return Builder */ public function newQuery() {} /** @return Builder */ public function newQueryWithoutScopes() {} @@ -115,7 +117,35 @@ public function withWhereHas($relation, ?\Closure $callback = null, $operator = /** @return $this */ public function withWhereRelation($relation, $column, $operator = null, $value = null) { return $this; } + /** + * @template TRelatedModel of Model + * @param Relations\Relation|string $relation + * @param (\Closure(Builder): mixed)|string $column + * @return $this + */ + public function orWhereRelation($relation, $column, $operator = null, $value = null) { return $this; } + /** + * @template TRelatedModel of Model + * @param Relations\Relation|string $relation + * @param (\Closure(Builder): mixed)|string $column + * @return $this + */ + public function whereDoesntHaveRelation($relation, $column, $operator = null, $value = null) { return $this; } + + /** + * @template TRelatedModel of Model + * @param Relations\Relation|string $relation + * @param (\Closure(Builder): mixed)|string $column + * @return $this + */ + public function orWhereDoesntHaveRelation($relation, $column, $operator = null, $value = null) { return $this; } + + /** + * @param (\Closure(Relations\Relation<*, *, *>): mixed)|null $callback + * @return $this + */ + public function with($relations, $callback = null) { return $this; } /** * @template TRelatedModel of Model @@ -339,7 +369,6 @@ function unrelated(UnrelatedQuery $query): void { }); } - class CustomArguments extends Team { /** * @param \Closure(Warehouse): void $operator @@ -406,6 +435,93 @@ function relations(): void { }); } + /** @param 'team'|'warehouse' $relation */ + function relationNameArguments(string $relation): void { + $name = 'team'; + Stock::whereHas($name, function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Stock::whereHas($relation, function ($query) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder', $query); + }); + Stock::withWhereHas($name, function (Builder|Relation $query) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Relations\BelongsTo', $query); + }); + } + /** + * @template TParent of Model + * @template TRelated of Model + * @extends BelongsTo + */ + class ReorderedRelation extends BelongsTo {} + /** @extends BelongsTo */ + class FixedTeamRelation extends BelongsTo {} + + /** + * @param ReorderedRelation $reordered + * @param Relation $base + * @param BelongsTo $union + */ + function customRelationArguments(ReorderedRelation $reordered, FixedTeamRelation $fixed, Relation $base, BelongsTo $union): void { + Stock::whereHas($reordered, function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Stock::whereHas($fixed, function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Stock::whereHas($base, function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Stock::whereHas($union, function ($query) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder', $query); + }); + } + + function relationObjectArguments(Stock $stock, TeamComment $comment): void { + Stock::whereHas($stock->team(), function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + TeamComment::whereHasMorph($comment->commentable(), Warehouse::class, function ($query, $type) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + assertType('string', $type); + }); + } + + /** @param 'team'|'warehouse' $relation */ + function eagerRelationNames(string $relation, string $unknown): void { + Stock::withWhereHas($relation, function (Builder|Relation $query) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder|Illuminate\Database\Eloquent\Relations\BelongsTo|Illuminate\Database\Eloquent\Relations\BelongsTo', $query); + }); + Stock::query()->with(callback: function (Relation $query) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo|Illuminate\Database\Eloquent\Relations\BelongsTo', $query); + }, relations: $relation); + $name = 'stocks.warehouse:id'; + Team::query()->with($name, function (Relation $query) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo', $query->orderBy('id')); + }); + Stock::query()->with($unknown, function ($query) { + assertType('Illuminate\Database\Eloquent\Relations\Relation', $query); + }); + Stock::whereHas($unknown, function ($query) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + }); + } + + function relationShortcutCallbacks(): void { + Stock::orWhereRelation('team', function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Stock::whereDoesntHaveRelation('team', function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Stock::orWhereDoesntHaveRelation('team', function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); + Stock::query()->with('team', function ($query) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo', $query); + }); + } + function eagerRelations(): void { Team::withWhereHas('stocks', function ($query) { assertType('Illuminate\Database\Eloquent\Builder|Illuminate\Database\Eloquent\Relations\HasMany', $query);