From aa607af403e634867cc8de29941f9acfd2d0d9c0 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Sun, 20 Sep 2026 23:20:47 +0600 Subject: [PATCH 1/9] 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/9] 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/9] 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/9] 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/9] 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/9] 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/9] 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); From fce27bd9c5fc09ab5ffb2dcca36deb0f9f505e5a Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Mon, 21 Sep 2026 00:46:27 +0600 Subject: [PATCH 8/9] fix(laravel): preserve relation callback context --- docs/CHANGELOG.md | 2 + examples/laravel/app/Demo.php | 22 + examples/laravel/app/Models/BakerRelation.php | 15 + examples/laravel/app/Models/Bakery.php | 6 + examples/laravel/assertions.php | 47 ++ examples/php/completion.php | 7 + examples/php/scaffolding/assertions.php | 4 + src/inheritance/ancestor.rs | 155 +++++++ src/inheritance/mod.rs | 3 + .../call_resolution/template_subs.rs | 7 +- src/type_engine/types/resolution.rs | 12 +- .../variable/closure_resolution.rs | 112 ++--- .../forward_walk/callable_inference.rs | 88 ---- .../variable/forward_walk/closures.rs | 13 + .../variable/forward_walk/diagnostic_walk.rs | 145 +++--- src/type_engine/variable/forward_walk/mod.rs | 2 + .../forward_walk/relation_callbacks.rs | 412 ++++++++++++++++++ .../variable/rhs_resolution/calls.rs | 6 +- .../variable/rhs_resolution/instantiation.rs | 129 ------ .../variable/rhs_resolution/mod.rs | 2 +- src/virtual_members/laravel/mod.rs | 3 +- src/virtual_members/laravel/relationships.rs | 141 ++++-- .../laravel/relationships_tests.rs | 2 +- tests/integration/completion_generics.rs | 42 ++ tests/integration/completion_laravel.rs | 163 +++++++ tests/integration/definition_laravel.rs | 37 ++ .../laravel-builder-relations.php | 234 ++++++++++ 27 files changed, 1446 insertions(+), 365 deletions(-) create mode 100644 examples/laravel/app/Models/BakerRelation.php create mode 100644 src/inheritance/ancestor.rs create mode 100644 src/type_engine/variable/forward_walk/relation_callbacks.rs diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 3628a8f7b..17b24c802 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 keep their full context.** Completion, navigation, and diagnostics retain generic model alternatives through custom relationships and eager-loading arrays. Application-defined callback signatures take precedence, and relation query methods work regardless of letter case. Contributed by @shuvroroy. + - **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. diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index d9ce05913..3979ee6b8 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -529,6 +529,28 @@ public function eloquentRelationArguments(bool $includeBaker): void }); } + public function eloquentRelationContexts(bool $bakerQuery): void + { + // Custom relations preserve their ancestor's related-model binding. + Bakery::whereHas('leadBaker', fn ($q) => $q->active()); // → BakerBuilder + Bakery::WHEREHAS('headBaker', fn ($q) => $q->active()); // PHP methods ignore case. + Bakery::with(['leadBaker' => function (Relation $q) { + $q->getModel()->getName(); // → Baker::getName(); $q is BakerRelation. + }]); + BlogPost::query()->with(['author' => ['posts' => function ($q) { + $q->getModel()->getTitle(); // → BlogPost::getTitle(); the terminal eager relation is HasMany. + }]]); + + if ($bakerQuery) { + $query = Bakery::query(); + $relation = 'headBaker'; + } else { + $query = BlogAuthor::query(); + $relation = 'posts'; + } + $query->whereHas($relation, fn ($q) => $q->where('id', '>', 0)); // → BakerBuilder|Builder + } + // ── Laravel Config & Env Navigation ───────────────────────────────────── /** diff --git a/examples/laravel/app/Models/BakerRelation.php b/examples/laravel/app/Models/BakerRelation.php new file mode 100644 index 000000000..7a476f984 --- /dev/null +++ b/examples/laravel/app/Models/BakerRelation.php @@ -0,0 +1,15 @@ + + */ +class BakerRelation extends HasOne +{ +} diff --git a/examples/laravel/app/Models/Bakery.php b/examples/laravel/app/Models/Bakery.php index 6fae53e15..b97f3a401 100644 --- a/examples/laravel/app/Models/Bakery.php +++ b/examples/laravel/app/Models/Bakery.php @@ -52,6 +52,12 @@ public function baguettes(): mixed { return $this->hasMany(Loaf::class); } /** @return HasOne */ public function headBaker(): mixed { return $this->hasOne(Baker::class); } + /** @return BakerRelation<$this, Baker> */ + public function leadBaker(): BakerRelation + { + return new BakerRelation((new Baker())->newQuery(), $this, 'bakers.bakery_id', 'id'); + } + /** @return BelongsToMany */ public function masterRecipe(): mixed { return $this->belongsToMany(BakeryRecipe::class)->using(RecipeIngredient::class)->withPivot('quantity', 'unit'); } diff --git a/examples/laravel/assertions.php b/examples/laravel/assertions.php index 01d900273..40da2da22 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -299,6 +299,53 @@ function assertMethodReturnType(string $class, string $method, string $expected) }); check('Morph relation objects retain candidate builders and the type string', $seen === [[\App\Models\LoafBuilder::class, \App\Models\Loaf::class]]); +$seen = []; +\App\Models\Bakery::whereHas('leadBaker', function ($query) use (&$seen) { + $seen[] = get_class($query->active()); +}); +$query = \App\Models\Bakery::with(['leadBaker' => function ($relation) use (&$seen) { + $seen[] = [get_class($relation), get_class($relation->getModel()), is_string($relation->getModel()->getName())]; +}]); +$query->getEagerLoads()['leadBaker']((new \App\Models\Bakery())->leadBaker()); +check('Custom relation names retain their related builder and eager relation', $seen === [ + \App\Models\BakerBuilder::class, + [\App\Models\BakerRelation::class, \App\Models\Baker::class, true], +]); +$seen = []; +\App\Models\Bakery::WHEREHAS('headBaker', function ($query) use (&$seen) { + $seen[] = get_class($query); +}); +\App\Models\Bakery::query()->WhereHas('headBaker', function ($query) use (&$seen) { + $seen[] = get_class($query); +}); +check('Relation query methods ignore letter case', $seen === [\App\Models\BakerBuilder::class, \App\Models\BakerBuilder::class]); +$seen = []; +$query = \App\Models\BlogPost::query()->with(['author' => ['posts' => function ($relation) use (&$seen) { + $model = $relation->getModel(); + $model->title = 'Example'; + $seen[] = [get_class($relation), get_class($model), $model->getTitle()]; +}]]); +$query->getEagerLoads()['author.posts']((new \App\Models\BlogAuthor())->posts()); +check('Nested eager arrays use the terminal relationship', $seen === [[\Illuminate\Database\Eloquent\Relations\HasMany::class, \App\Models\BlogPost::class, 'Example']]); +$seen = []; +foreach ([[\App\Models\Bakery::query(), 'headBaker'], [\App\Models\BlogAuthor::query(), 'posts']] as [$query, $relation]) { + $query->whereHas($relation, function ($related) use (&$seen) { + $seen[] = [get_class($related->where('id', '>', 0)), get_class($related->getModel())]; + }); +} +check('Receiver alternatives each contribute their own related model and builder', $seen === [ + [\App\Models\BakerBuilder::class, \App\Models\Baker::class], + [\Illuminate\Database\Eloquent\Builder::class, \App\Models\BlogPost::class], +]); +$ownCallback = new class extends \App\Models\Bakery { + /** @param \Closure(\App\Models\Loaf): mixed $callback */ + public static function whereHas($relation, ?\Closure $callback = null, $operator = '>=', $count = 1) + { + return $callback(new \App\Models\Loaf()); + } +}; +check('Application declarations control their callback contract', $ownCallback::whereHas('headBaker', fn ($model) => get_class($model)) === \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/examples/php/completion.php b/examples/php/completion.php index 22105a7ec..c741274f7 100644 --- a/examples/php/completion.php +++ b/examples/php/completion.php @@ -1453,6 +1453,13 @@ public function gate(?string $grade): string class GenericsDemo { + /** @param Scaffolding\Box|Scaffolding\Box $box */ + public function boxedAlternatives(Scaffolding\Box $box): string + { + // Try completion after unwrap()->: both Pen and Pencil members appear. + return $box->unwrap()->label(); // → string; both instantiations retain their value type. + } + public function demo(): void { $repo = new Scaffolding\PenRepository(); diff --git a/examples/php/scaffolding/assertions.php b/examples/php/scaffolding/assertions.php index 8ef110df3..87c49cbc8 100644 --- a/examples/php/scaffolding/assertions.php +++ b/examples/php/scaffolding/assertions.php @@ -80,6 +80,10 @@ function runDemoAssertions(): void assert($selfOutBox->value instanceof Scaffolding\Pencil, 'replace() swaps the contents, so the box holds a Scaffolding\Pencil after the call'); assert(!$selfOutBox->value instanceof Scaffolding\Pen, 'a Scaffolding\Pencil is not a Scaffolding\Pen — the template argument really changed'); + $genericDemo = new GenericsDemo(); + assert($genericDemo->boxedAlternatives(new Scaffolding\Box(new Scaffolding\Pen())) === 'pen', 'a Box unwraps to a Pen'); + assert($genericDemo->boxedAlternatives(new Scaffolding\Box(new Scaffolding\Pencil())) === 'pencil', 'a Box retains the other generic alternative'); + // ── Trait `return $this` fluent chain ─────────────────────────────── $page = new Scaffolding\TestablePage(); assert($page->assertSee('a') instanceof Scaffolding\TestablePage, 'trait return $this resolves to the using class'); diff --git a/src/inheritance/ancestor.rs b/src/inheritance/ancestor.rs new file mode 100644 index 000000000..ab1afdf7b --- /dev/null +++ b/src/inheritance/ancestor.rs @@ -0,0 +1,155 @@ +//! Generic arguments projected through declared class and interface ancestry. + +use std::collections::HashMap; +use std::sync::Arc; + +use crate::php_type::{PhpType, TypeKind}; +use crate::types::ClassInfo; + +/// Extract a generic type argument from a class's ancestor chain. +/// +/// Given an argument type (e.g. `FooContainer`) and a target wrapper class +/// (e.g. `Container`), walks the `@extends` chain to find where the argument +/// type (or one of its ancestors) extends the wrapper class, then extracts the +/// generic argument at `tpl_position`. +/// +/// For example, if `FooContainer` has `@extends Container`, calling +/// `extract_generic_arg_from_ancestor(FooContainer, "Container", 0, ...)` returns `Foo`. +pub(crate) fn extract_generic_arg_from_ancestor( + arg_type: &PhpType, + wrapper_name: &str, + tpl_position: usize, + class_loader: &dyn Fn(&str) -> Option>, +) -> Option { + if let TypeKind::Union(parts) = arg_type.kind() { + let args: Vec<_> = parts + .iter() + .filter_map(|part| { + extract_generic_arg_from_ancestor(part, wrapper_name, tpl_position, class_loader) + }) + .collect(); + return (!args.is_empty()).then(|| PhpType::union(args).simplified()); + } + let class_name = match arg_type.kind() { + TypeKind::Named(n) => n.as_str(), + TypeKind::Generic(g) => g.name.as_str(), + _ => return None, + }; + + // If the arg type itself is already generic with the wrapper name, + // extract directly. E.g. argument type is `Container`. + if let TypeKind::Generic(g) = arg_type.kind() + && ancestor_name_matches(&g.name, wrapper_name) + { + return g.args.get(tpl_position).cloned(); + } + + let cls = class_loader(class_name)?; + + let subs = match arg_type.kind() { + TypeKind::Generic(g) => super::build_generic_subs(&cls, &g.args), + _ => HashMap::new(), + }; + let mut visited = Vec::new(); + ancestor_generic_arg( + &cls, + wrapper_name, + tpl_position, + &subs, + &mut visited, + class_loader, + ) +} + +/// Maximum ancestry depth walked while looking for an ancestor's generic +/// argument. A backstop against an `extends`/`implements` cycle the +/// loader hands back; the `visited` set is what actually bounds the work. +const MAX_ANCESTOR_GENERIC_DEPTH: usize = 15; + +/// The type argument `target_name` receives at `position`, as seen from +/// `cls`. +/// +/// Walks the parent chain **and** the interface list, threading each +/// level's `@extends`/`@implements` arguments into the next, so a class +/// that reaches the ancestor only through an intermediate generic +/// interface still reports a concrete argument. +/// `X implements CollectorWithPaths` together with +/// `CollectorWithPaths extends Collector` is what says +/// `Collector`'s value argument is that `array{…}`. +fn ancestor_generic_arg( + cls: &ClassInfo, + target_name: &str, + position: usize, + subs: &HashMap, + visited: &mut Vec, + class_loader: &dyn Fn(&str) -> Option>, +) -> Option { + if visited.len() > MAX_ANCESTOR_GENERIC_DEPTH { + return None; + } + let fqn = cls.fqn(); + if visited.contains(&fqn) { + return None; + } + visited.push(fqn); + + if let Some(arg) = find_extends_generic_arg(cls, target_name, position) { + return Some(if subs.is_empty() { + arg + } else { + arg.substitute(subs) + }); + } + + for ancestor_name in cls.parent_class.iter().chain(cls.interfaces.iter()) { + let Some(ancestor) = class_loader(ancestor_name) else { + continue; + }; + let next_subs = crate::inheritance::build_substitution_map( + &crate::inheritance::ClassRef::Borrowed(cls), + &ancestor, + subs, + ); + if let Some(arg) = ancestor_generic_arg( + &ancestor, + target_name, + position, + &next_subs, + visited, + class_loader, + ) { + return Some(arg); + } + } + + None +} + +/// Find a generic arg at `position` from a class's `@extends` generics +/// matching a qualified or unqualified ancestor name. +fn find_extends_generic_arg( + cls: &ClassInfo, + target_name: &str, + position: usize, +) -> Option { + for (name, args) in cls + .extends_generics + .iter() + .chain(cls.implements_generics.iter()) + { + if ancestor_name_matches(name, target_name) { + return args.get(position).cloned(); + } + } + None +} + +fn ancestor_name_matches(actual: &str, target: &str) -> bool { + let target = target.trim_start_matches('\\'); + let actual = actual.trim_start_matches('\\'); + if target.contains('\\') { + actual.eq_ignore_ascii_case(target) + } else { + crate::util::short_name(actual).eq_ignore_ascii_case(target) + } +} diff --git a/src/inheritance/mod.rs b/src/inheritance/mod.rs index 38a94d119..4a567cb1d 100644 --- a/src/inheritance/mod.rs +++ b/src/inheritance/mod.rs @@ -17,6 +17,9 @@ //! properties have their template parameter references replaced with the //! concrete types. +mod ancestor; +pub(crate) use ancestor::extract_generic_arg_from_ancestor; + pub mod enrichment; pub mod generics; pub mod traits; diff --git a/src/type_engine/call_resolution/template_subs.rs b/src/type_engine/call_resolution/template_subs.rs index ca08e2302..5d9a760d8 100644 --- a/src/type_engine/call_resolution/template_subs.rs +++ b/src/type_engine/call_resolution/template_subs.rs @@ -1501,8 +1501,11 @@ fn ancestor_bound_binding( }; let position = g.args.iter().position(|a| a.is_named(tpl_name))?; let subject = bound_to.unwrap_class_string_inner().unwrap_or(bound_to); - crate::type_engine::variable::rhs_resolution::extract_generic_arg_from_ancestor( - subject, &g.name, position, ctx, + crate::inheritance::extract_generic_arg_from_ancestor( + subject, + &g.name, + position, + ctx.class_loader, ) } diff --git a/src/type_engine/types/resolution.rs b/src/type_engine/types/resolution.rs index d8a1dace4..cc1e31eee 100644 --- a/src/type_engine/types/resolution.rs +++ b/src/type_engine/types/resolution.rs @@ -203,6 +203,7 @@ fn type_hint_to_classes_typed_depth( // ── Union type ───────────────────────────────────────────── TypeKind::Union(members) => { let mut results: Vec> = Vec::new(); + let mut generic_members = Vec::new(); for member in members { let resolved = type_hint_to_classes_typed_depth( member, @@ -211,7 +212,16 @@ fn type_hint_to_classes_typed_depth( class_loader, depth, ); - ClassInfo::extend_unique_arc(&mut results, resolved); + if matches!(member.kind(), TypeKind::Generic(_)) { + // Different arguments can instantiate the same class with + // different members. FQN-only deduplication loses a branch. + if !generic_members.contains(&member) { + generic_members.push(member); + results.extend(resolved); + } + } else { + ClassInfo::extend_unique_arc(&mut results, resolved); + } } results } diff --git a/src/type_engine/variable/closure_resolution.rs b/src/type_engine/variable/closure_resolution.rs index 9f1cf91a6..d925bc5cc 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_details, + model_builder_type, related_model_type, resolve_relation_chain_details, }; thread_local! { @@ -828,20 +828,26 @@ fn try_relation_query_override( if !RELATION_QUERY_METHODS.contains(&method_name) { 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(), - ]); + let mut queries = Vec::new(); + for model in find_models_from_receivers(receivers, rctx) { + 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))], + ) + }); + queries.push(morph_candidate_builders( + candidates, + &fallback, + rctx.class_loader, + )); + } else if let Some(query) = query { + queries.push(query); + } } - Some(vec![query?]) + (!queries.is_empty()).then(|| vec![PhpType::union(queries).simplified()]) } /// Combine known relation alternatives without replacing unresolved eager @@ -881,7 +887,14 @@ fn relation_callback_type( if method_name == "with" { return Some(related.relation_type); } - let builder = model_builder_type(&related.model, rctx.class_loader); + let builder = PhpType::union( + related + .models + .iter() + .map(|model| model_builder_type(model, rctx.class_loader)) + .collect(), + ) + .simplified(); return Some(if eager { PhpType::union(vec![builder, related.relation_type]) } else { @@ -895,23 +908,7 @@ fn relation_callback_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 related = related_model_type(relation, rctx.class_loader)?; let fallback = PhpType::generic( ELOQUENT_BUILDER_FQN, vec![PhpType::named(atom(ELOQUENT_MODEL_FQN))], @@ -1075,46 +1072,57 @@ fn extract_generic_args_from_methods(class: &ClassInfo, class_fqn: &str) -> Opti /// 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( +fn find_models_from_receivers( receivers: &[ResolvedType], - class_loader: &dyn Fn(&str) -> Option>, -) -> Option> { + rctx: &ResolutionCtx<'_>, +) -> Vec> { + let mut models = Vec::new(); 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)); + if cls.fqn() == ELOQUENT_MODEL_FQN || extends_eloquent_model(cls, rctx.class_loader) { + models.push(Arc::clone(cls)); + continue; } if cls.fqn() != ELOQUENT_BUILDER_FQN && !crate::virtual_members::laravel::helpers::extends_eloquent_builder( cls, - class_loader, + rctx.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, - } + let model_type = crate::inheritance::extract_generic_arg_from_ancestor( + &receiver.type_string, + ELOQUENT_BUILDER_FQN, + 0, + rctx.class_loader, + ) .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); + if let Some(model_type) = model_type { + models.extend( + crate::type_engine::type_resolution::type_hint_to_classes_typed( + &model_type, + &rctx + .current_class + .map_or_else(|| atom(""), |class| class.name), + rctx.all_classes, + rctx.class_loader, + ) + .into_iter() + .filter(|model| { + model.fqn() == ELOQUENT_MODEL_FQN + || extends_eloquent_model(model, rctx.class_loader) + }), + ); } } - None + models } /// Extract the model type from a resolved `Builder` class by diff --git a/src/type_engine/variable/forward_walk/callable_inference.rs b/src/type_engine/variable/forward_walk/callable_inference.rs index 78cde1392..d4f42c0f0 100644 --- a/src/type_engine/variable/forward_walk/callable_inference.rs +++ b/src/type_engine/variable/forward_walk/callable_inference.rs @@ -525,94 +525,6 @@ pub(crate) fn extract_callable_params_at_fw( vec![] } -/// 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<'_>, - scope: &ScopeState, - ctx: &ForwardWalkCtx<'_>, -) -> Option> { - if !crate::virtual_members::laravel::RELATION_QUERY_METHODS.contains(&method_name) { - return None; - } - let callback_name = if method_name.ends_with("Relation") { - "column" - } 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 { - 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_name), 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 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")?]?; - 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_type, - candidates.as_ref(), - &var_ctx.as_resolution_ctx(), - )?; - // 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 -} - /// Seed `$this` in the scope with the enclosing class's type. Callers /// invoke this only for non-static class methods; the scope-based /// variable resolver then serves `$this` lookups from this entry diff --git a/src/type_engine/variable/forward_walk/closures.rs b/src/type_engine/variable/forward_walk/closures.rs index 72f7d7dd6..2bf33054a 100644 --- a/src/type_engine/variable/forward_walk/closures.rs +++ b/src/type_engine/variable/forward_walk/closures.rs @@ -333,6 +333,19 @@ pub(crate) fn try_enter_closure_expr<'b>( Argument::Positional(a) => a.value, Argument::Named(a) => a.value, }; + if let Some(entries) = eager_callback_arguments(call, arg_idx, scope, ctx) { + for entry in entries { + if try_enter_closure_expr( + entry.expression, + scope, + ctx, + Some(&entry.parameters), + ) { + return true; + } + } + continue; + } let inferred = infer_callable_params_for_call(call, arg_idx, scope, ctx); let inferred_opt = if inferred.is_empty() { None diff --git a/src/type_engine/variable/forward_walk/diagnostic_walk.rs b/src/type_engine/variable/forward_walk/diagnostic_walk.rs index 6ff29d84d..8edc5ebb4 100644 --- a/src/type_engine/variable/forward_walk/diagnostic_walk.rs +++ b/src/type_engine/variable/forward_walk/diagnostic_walk.rs @@ -299,7 +299,7 @@ pub(crate) fn walk_closures_in_expr<'b>( } Expression::Instantiation(inst) => { if let Some(ref args) = inst.argument_list { - walk_closures_in_call_args(&args.arguments, outer_scope, ctx, |_| vec![]); + walk_closures_in_call_args(&args.arguments, outer_scope, ctx, None, |_| vec![]); } } Expression::AnonymousClass(anon) => { @@ -366,19 +366,25 @@ pub(crate) fn walk_closures_in_call<'b>( Expression::Identifier(ident) => Some(bytes_to_str(ident.value()).to_string()), _ => None, }; - walk_closures_in_call_args(&fc.argument_list.arguments, outer_scope, ctx, |arg_idx| { - if let Some(ref name) = func_name { - infer_callable_params_from_function_fw( - name, - arg_idx, - &fc.argument_list, - outer_scope, - ctx, - ) - } else { - vec![] - } - }); + walk_closures_in_call_args( + &fc.argument_list.arguments, + outer_scope, + ctx, + Some(call), + |arg_idx| { + if let Some(ref name) = func_name { + infer_callable_params_from_function_fw( + name, + arg_idx, + &fc.argument_list, + outer_scope, + ctx, + ) + } else { + vec![] + } + }, + ); } Call::Method(mc) => { walk_closures_in_expr(mc.object, outer_scope, ctx, None); @@ -389,20 +395,26 @@ pub(crate) fn walk_closures_in_call<'b>( None }; let obj_span = mc.object.span(); - 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( - (obj_span.start.offset, obj_span.end.offset), - name, - arg_idx, - &mc.argument_list, - outer_scope, - ctx, - ) - } else { - vec![] - } - }); + walk_closures_in_call_args( + &mc.argument_list.arguments, + outer_scope, + ctx, + Some(call), + |arg_idx| { + if let Some(ref name) = method_name { + infer_callable_params_from_receiver_fw( + (obj_span.start.offset, obj_span.end.offset), + name, + arg_idx, + &mc.argument_list, + outer_scope, + ctx, + ) + } else { + vec![] + } + }, + ); } Call::NullSafeMethod(mc) => { walk_closures_in_expr(mc.object, outer_scope, ctx, None); @@ -413,20 +425,26 @@ pub(crate) fn walk_closures_in_call<'b>( None }; let obj_span = mc.object.span(); - 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( - (obj_span.start.offset, obj_span.end.offset), - name, - arg_idx, - &mc.argument_list, - outer_scope, - ctx, - ) - } else { - vec![] - } - }); + walk_closures_in_call_args( + &mc.argument_list.arguments, + outer_scope, + ctx, + Some(call), + |arg_idx| { + if let Some(ref name) = method_name { + infer_callable_params_from_receiver_fw( + (obj_span.start.offset, obj_span.end.offset), + name, + arg_idx, + &mc.argument_list, + outer_scope, + ctx, + ) + } else { + vec![] + } + }, + ); } Call::StaticMethod(sc) => { walk_closures_in_expr(sc.class, outer_scope, ctx, None); @@ -436,20 +454,26 @@ pub(crate) fn walk_closures_in_call<'b>( } else { None }; - 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( - sc.class, - name, - arg_idx, - &sc.argument_list, - outer_scope, - ctx, - ) - } else { - vec![] - } - }); + walk_closures_in_call_args( + &sc.argument_list.arguments, + outer_scope, + ctx, + Some(call), + |arg_idx| { + if let Some(ref name) = method_name { + infer_callable_params_from_static_receiver_fw( + sc.class, + name, + arg_idx, + &sc.argument_list, + outer_scope, + ctx, + ) + } else { + vec![] + } + }, + ); } } } @@ -462,6 +486,7 @@ pub(crate) fn walk_closures_in_call_args<'b, F>( arguments: &'b TokenSeparatedSequence<'b, Argument<'b>>, outer_scope: &ScopeState, ctx: &ForwardWalkCtx<'_>, + call: Option<&Call<'b>>, infer_fn: F, ) where F: Fn(usize) -> Vec, @@ -471,6 +496,14 @@ pub(crate) fn walk_closures_in_call_args<'b, F>( Argument::Positional(a) => a.value, Argument::Named(a) => a.value, }; + if let Some(entries) = + call.and_then(|call| eager_callback_arguments(call, arg_idx, outer_scope, ctx)) + { + for entry in entries { + walk_closures_in_expr(entry.expression, outer_scope, ctx, Some(&entry.parameters)); + } + continue; + } match arg_expr { Expression::Closure(_) | Expression::ArrowFunction(_) => { let inferred = infer_fn(arg_idx); diff --git a/src/type_engine/variable/forward_walk/mod.rs b/src/type_engine/variable/forward_walk/mod.rs index 770db077f..4302e5c27 100644 --- a/src/type_engine/variable/forward_walk/mod.rs +++ b/src/type_engine/variable/forward_walk/mod.rs @@ -54,6 +54,7 @@ mod diagnostic_walk; mod loop_control; mod param_seeding; mod reachability; +mod relation_callbacks; mod scope_state; mod snapshot_narrowing; mod static_locals; @@ -69,6 +70,7 @@ pub(crate) use diagnostic_walk::*; pub(crate) use loop_control::*; pub(crate) use param_seeding::*; pub(crate) use reachability::*; +pub(crate) use relation_callbacks::*; pub(crate) use scope_state::*; pub(crate) use snapshot_narrowing::*; pub(crate) use walk_ctx::*; diff --git a/src/type_engine/variable/forward_walk/relation_callbacks.rs b/src/type_engine/variable/forward_walk/relation_callbacks.rs new file mode 100644 index 000000000..b66848890 --- /dev/null +++ b/src/type_engine/variable/forward_walk/relation_callbacks.rs @@ -0,0 +1,412 @@ +//! Context-sensitive Eloquent relation callback inference shared by both walks. + +use super::*; +use crate::atom::atom; +use crate::php_type::PhpType; +use crate::types::{ClassInfo, MAX_INHERITANCE_DEPTH, ResolvedType}; +use std::sync::Arc; + +/// 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. +pub(crate) fn infer_relation_callback_params( + receivers: &[ResolvedType], + method_name: &str, + arg_idx: usize, + argument_list: &ArgumentList<'_>, + scope: &ScopeState, + ctx: &ForwardWalkCtx<'_>, +) -> Option> { + let method_name = *crate::virtual_members::laravel::RELATION_QUERY_METHODS + .iter() + .find(|name| name.eq_ignore_ascii_case(method_name))?; + let callback_name = if method_name.ends_with("Relation") { + "column" + } else { + "callback" + }; + let relation_name = if method_name == "with" { + "relations" + } else { + "relation" + }; + let argument = argument_list.arguments.iter().nth(arg_idx)?.value(); + let mut alternatives = Vec::new(); + let mut refined = false; + 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 declared = extract_callable_params_at_fw(&method.parameters, argument_list, arg_idx); + if !is_framework_relation_method(class, method_name, ctx) { + alternatives.push(declared); + 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_name), 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 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")?]?; + Some( + super::super::resolution::resolve_arg_raw_type(types, &var_ctx) + .unwrap_or_else(PhpType::mixed), + ) + } else { + None + }; + let Some(mut inferred) = super::super::closure_resolution::try_relation_query_override_pub( + std::slice::from_ref(receiver), + method_name, + &relation_type, + candidates.as_ref(), + &var_ctx.as_resolution_ctx(), + ) else { + alternatives.push(declared); + continue; + }; + // Refine the query parameter without losing Laravel's second morph + // callback parameter (the candidate's class-string). + inferred.extend(declared.into_iter().skip(1)); + alternatives.push(inferred); + refined = true; + } + refined.then(|| merge_callback_parameters(alternatives)) +} + +fn merge_callback_parameters(mut alternatives: Vec>) -> Vec { + if alternatives.len() == 1 { + return alternatives.pop().unwrap_or_default(); + } + let count = alternatives.iter().map(Vec::len).max().unwrap_or(0); + (0..count) + .map(|index| { + PhpType::union( + alternatives + .iter() + .filter_map(|params| params.get(index).cloned()) + .collect(), + ) + .simplified() + }) + .collect() +} + +fn is_framework_relation_method(class: &ClassInfo, method: &str, ctx: &ForwardWalkCtx<'_>) -> bool { + let Some(raw) = (ctx.class_loader)(&class.fqn()) else { + return false; + }; + if let Some(framework) = declared_relation_method(&raw, method, ctx.class_loader, 0) { + return framework; + } + if !crate::virtual_members::laravel::extends_eloquent_model(&raw, ctx.class_loader) { + return false; + } + let builder = crate::virtual_members::laravel::model_builder_type(&raw, ctx.class_loader); + builder + .base_name() + .and_then(ctx.class_loader) + .is_some_and(|builder| { + declared_relation_method(&builder, method, ctx.class_loader, 0) == Some(true) + }) +} + +// Only framework declarations carry Eloquent's relation-argument semantics. +// Raw members and trait aliases take precedence over forwarded builder methods. +fn declared_relation_method( + class: &ClassInfo, + method: &str, + loader: &dyn Fn(&str) -> Option>, + depth: u32, +) -> Option { + if depth >= MAX_INHERITANCE_DEPTH { + return Some(false); + } + if class.get_method(method).is_some() + || class.doc_members.as_ref().is_some_and(|doc| { + doc.methods + .iter() + .any(|m| m.name.eq_ignore_ascii_case(method)) + }) + || class.trait_aliases.iter().any(|alias| { + alias + .alias + .is_some_and(|name| name.eq_ignore_ascii_case(method)) + }) + { + return Some( + matches!( + class.fqn().as_str(), + "Illuminate\\Database\\Eloquent\\Builder" + | "Illuminate\\Database\\Eloquent\\Concerns\\QueriesRelationships" + ) || (method == "with" && class.fqn() == "Illuminate\\Database\\Eloquent\\Model"), + ); + } + for name in &class.used_traits { + if class.trait_precedences.iter().any(|precedence| { + precedence.method_name.eq_ignore_ascii_case(method) + && precedence.insteadof.contains(name) + }) { + continue; + } + if let Some(class) = loader(name) + && let Some(result) = declared_relation_method(&class, method, loader, depth + 1) + { + return Some(result); + } + } + class + .parent_class + .as_ref() + .and_then(|name| loader(name)) + .and_then(|parent| declared_relation_method(&parent, method, loader, depth + 1)) +} + +/// An expression inside an eager-loading array and its contextual parameters. +/// Non-callback expressions are retained so both walkers still visit them. +pub(crate) struct EagerCallbackArgument<'a> { + /// The array key, value, or closure to visit. + pub expression: &'a Expression<'a>, + /// Parameters inferred for a direct constraint closure, if any. + pub parameters: Vec, +} + +/// Resolve eager-array callbacks once for both cursor and diagnostic walks. +/// Keys describe relation paths; nested arrays extend the parent key's path. +pub(crate) fn eager_callback_arguments<'a>( + call: &Call<'a>, + arg_idx: usize, + scope: &ScopeState, + ctx: &ForwardWalkCtx<'_>, +) -> Option>> { + let (selector, arguments) = match call { + Call::Method(call) => (&call.method, &call.argument_list), + Call::NullSafeMethod(call) => (&call.method, &call.argument_list), + Call::StaticMethod(call) => (&call.method, &call.argument_list), + _ => return None, + }; + let ClassLikeMemberSelector::Identifier(name) = selector else { + return None; + }; + if !crate::atom::bytes_to_str(name.value).eq_ignore_ascii_case("with") { + return None; + } + let argument = arguments.arguments.iter().nth(arg_idx)?.value(); + let mut array = argument; + while let Expression::Parenthesized(inner) = array { + array = inner.expression; + } + crate::parser::array_literal_elements(array)?; + if !contains_eager_callback(argument) { + return None; + } + let scope_resolver = |name: &str| scope.locals.get(&atom(name)).cloned().unwrap_or_default(); + let var_ctx = ctx.var_ctx_for_with_scope( + "$__infer", + argument.span().start.offset, + &scope_resolver, + Some(scope.proofs()), + ); + let rctx = var_ctx.as_resolution_ctx(); + let receivers = match call { + Call::Method(call) => { + let span = call.object.span(); + super::super::closure_resolution::resolve_receiver_types( + span.start.offset, + span.end.offset, + &rctx, + ) + } + Call::NullSafeMethod(call) => { + let span = call.object.span(); + super::super::closure_resolution::resolve_receiver_types( + span.start.offset, + span.end.offset, + &rctx, + ) + } + Call::StaticMethod(call) => { + let name = super::super::closure_resolution::static_receiver_class_name( + call.class, + Some(ctx.current_class), + )?; + let owner = super::super::closure_resolution::find_owner_by_name( + &name, + ctx.all_classes, + ctx.class_loader, + )?; + vec![ResolvedType::from_class(owner)] + } + _ => return None, + }; + let receivers: Vec<_> = receivers + .into_iter() + .filter(|receiver| { + let Some(class) = receiver.class_info.as_ref() else { + return false; + }; + if !is_framework_relation_method(class, "with", ctx) { + return false; + } + let Some(method) = class.get_method_arc("with").or_else(|| { + crate::virtual_members::resolve_class_fully_maybe_cached( + class, + ctx.class_loader, + ctx.resolved_class_cache, + ) + .get_method_arc("with") + }) else { + return false; + }; + let Some(index) = method + .parameters + .iter() + .position(|p| p.name.trim_start_matches('$') == "relations") + else { + return false; + }; + crate::call_args::bind_args_to_params(&method.parameters, arguments)[index] + .is_some_and(|bound| bound.span() == argument.span()) + }) + .collect(); + if receivers.is_empty() { + return None; + } + let mut entries = Vec::new(); + collect_eager_arguments(argument, None, &receivers, &var_ctx, &mut entries); + Some(entries) +} + +// Most eager-load lists contain only names. Avoid resolving their receiver +// or constructing contextual entries when there is no callback to type. +fn contains_eager_callback(expression: &Expression<'_>) -> bool { + match expression { + Expression::Closure(_) | Expression::ArrowFunction(_) => true, + Expression::Parenthesized(inner) => contains_eager_callback(inner.expression), + _ => crate::parser::array_literal_elements(expression).is_some_and(|elements| { + elements + .iter() + .filter_map(crate::parser::array_element_value) + .any(contains_eager_callback) + }), + } +} + +fn collect_eager_arguments<'a>( + expression: &'a Expression<'a>, + prefix: Option<&PhpType>, + receivers: &[ResolvedType], + ctx: &crate::type_engine::resolver::VarResolutionCtx<'_>, + entries: &mut Vec>, +) { + if let Expression::Parenthesized(inner) = expression { + collect_eager_arguments(inner.expression, prefix, receivers, ctx, entries); + return; + } + if let Some(elements) = crate::parser::array_literal_elements(expression) { + for element in elements.iter() { + let key = crate::parser::array_element_key(element); + let path = key + .and_then(|key| super::super::resolution::resolve_arg_raw_type(key, ctx)) + .map(|key| join_relation_path(prefix, &key)); + if let Some(key) = key { + entries.push(EagerCallbackArgument { + expression: key, + parameters: Vec::new(), + }); + } + if let Some(value) = crate::parser::array_element_value(element) { + collect_eager_arguments(value, path.as_ref(), receivers, ctx, entries); + } + } + return; + } + let parameters = if matches!( + expression, + Expression::Closure(_) | Expression::ArrowFunction(_) + ) { + prefix + .map(|path| { + super::super::closure_resolution::try_relation_query_override_pub( + receivers, + "with", + path, + None, + &ctx.as_resolution_ctx(), + ) + .unwrap_or_else(|| { + vec![PhpType::generic( + "Illuminate\\Database\\Eloquent\\Relations\\Relation", + vec![PhpType::mixed(); 3], + )] + }) + }) + .unwrap_or_default() + } else { + Vec::new() + }; + entries.push(EagerCallbackArgument { + expression, + parameters, + }); +} + +fn join_relation_path(prefix: Option<&PhpType>, key: &PhpType) -> PhpType { + let Some(prefix) = prefix else { + return key.clone(); + }; + if let crate::php_type::TypeKind::Union(parts) = prefix.kind() { + return PhpType::union( + parts + .iter() + .map(|part| join_relation_path(Some(part), key)) + .collect(), + ) + .simplified(); + } + if let crate::php_type::TypeKind::Union(parts) = key.kind() { + return PhpType::union( + parts + .iter() + .map(|part| join_relation_path(Some(prefix), part)) + .collect(), + ) + .simplified(); + } + let parent = prefix.as_literal().and_then(|value| value.string_content()); + let child = key.as_literal().and_then(|value| value.string_content()); + match (parent, child) { + (Some(parent), Some(child)) => PhpType::literal_string_value(format!("{parent}.{child}")), + _ => PhpType::named(atom("string")), + } +} diff --git a/src/type_engine/variable/rhs_resolution/calls.rs b/src/type_engine/variable/rhs_resolution/calls.rs index 82ded71c3..554d455eb 100644 --- a/src/type_engine/variable/rhs_resolution/calls.rs +++ b/src/type_engine/variable/rhs_resolution/calls.rs @@ -5,6 +5,8 @@ use std::collections::HashMap; use std::sync::Arc; +use crate::inheritance::extract_generic_arg_from_ancestor; + use mago_span::HasSpan; use mago_syntax::cst::*; @@ -21,7 +23,7 @@ use crate::type_engine::variable::resolution::build_var_resolver_from_ctx; use super::array_access::{class_string_inner_binding, insert_or_union}; use super::instantiation::{ TemplateBindingMode, array_element_binding, candidate_binding_modes, classify_template_binding, - extract_array_position, extract_generic_arg_from_ancestor, + extract_array_position, }; use super::{ extract_closure_or_arrow_return_type, resolve_rhs_expression, resolve_var_types, @@ -223,7 +225,7 @@ fn apply_template_binding_mode( &resolved_type, wrapper_name, tpl_position, - rctx, + rctx.class_loader, ) { insert_or_union(subs, tpl_name.to_string(), concrete); diff --git a/src/type_engine/variable/rhs_resolution/instantiation.rs b/src/type_engine/variable/rhs_resolution/instantiation.rs index 57732d3aa..a579a63f2 100644 --- a/src/type_engine/variable/rhs_resolution/instantiation.rs +++ b/src/type_engine/variable/rhs_resolution/instantiation.rs @@ -361,135 +361,6 @@ pub(super) fn extract_class_string_inner(resolved: &[ResolvedType]) -> Option`, calling -/// `extract_generic_arg_from_ancestor(FooContainer, "Container", 0, ...)` returns `Foo`. -pub(crate) fn extract_generic_arg_from_ancestor( - arg_type: &PhpType, - wrapper_name: &str, - tpl_position: usize, - rctx: &crate::type_engine::resolver::ResolutionCtx<'_>, -) -> Option { - let class_name = match arg_type.kind() { - TypeKind::Named(n) => n.as_str(), - TypeKind::Generic(g) => g.name.as_str(), - _ => return None, - }; - - // If the arg type itself is already generic with the wrapper name, - // extract directly. E.g. argument type is `Container`. - if let TypeKind::Generic(g) = arg_type.kind() { - let n_short = crate::util::short_name(&g.name); - let wrapper_short = crate::util::short_name(wrapper_name); - if n_short.eq_ignore_ascii_case(wrapper_short) { - return g.args.get(tpl_position).cloned(); - } - } - - let class_loader = rctx.class_loader; - let cls = class_loader(class_name)?; - - let wrapper_short = crate::util::short_name(wrapper_name); - let mut visited = Vec::new(); - ancestor_generic_arg( - &cls, - wrapper_short, - tpl_position, - &HashMap::new(), - &mut visited, - class_loader, - ) -} - -/// Maximum ancestry depth walked while looking for an ancestor's generic -/// argument. A backstop against an `extends`/`implements` cycle the -/// loader hands back; the `visited` set is what actually bounds the work. -const MAX_ANCESTOR_GENERIC_DEPTH: usize = 15; - -/// The type argument `ancestor_short` receives at `position`, as seen from -/// `cls`. -/// -/// Walks the parent chain **and** the interface list, threading each -/// level's `@extends`/`@implements` arguments into the next, so a class -/// that reaches the ancestor only through an intermediate generic -/// interface still reports a concrete argument. -/// `X implements CollectorWithPaths` together with -/// `CollectorWithPaths extends Collector` is what says -/// `Collector`'s value argument is that `array{…}`. -fn ancestor_generic_arg( - cls: &ClassInfo, - ancestor_short: &str, - position: usize, - subs: &HashMap, - visited: &mut Vec, - class_loader: &dyn Fn(&str) -> Option>, -) -> Option { - if visited.len() > MAX_ANCESTOR_GENERIC_DEPTH { - return None; - } - let fqn = cls.fqn(); - if visited.contains(&fqn) { - return None; - } - visited.push(fqn); - - if let Some(arg) = find_extends_generic_arg(cls, ancestor_short, position) { - return Some(if subs.is_empty() { - arg - } else { - arg.substitute(subs) - }); - } - - for ancestor_name in cls.parent_class.iter().chain(cls.interfaces.iter()) { - let Some(ancestor) = class_loader(ancestor_name) else { - continue; - }; - let next_subs = crate::inheritance::build_substitution_map( - &crate::inheritance::ClassRef::Borrowed(cls), - &ancestor, - subs, - ); - if let Some(arg) = ancestor_generic_arg( - &ancestor, - ancestor_short, - position, - &next_subs, - visited, - class_loader, - ) { - return Some(arg); - } - } - - None -} - -/// Find a generic arg at `position` from a class's `@extends` generics -/// matching a target short name. -pub(super) fn find_extends_generic_arg( - cls: &ClassInfo, - target_short: &str, - position: usize, -) -> Option { - for (name, args) in cls - .extends_generics - .iter() - .chain(cls.implements_generics.iter()) - { - if crate::util::short_name(name) == target_short { - return args.get(position).cloned(); - } - } - None -} - /// Remap constructor template substitutions from ancestor param names to child /// param names when a constructor is inherited. /// diff --git a/src/type_engine/variable/rhs_resolution/mod.rs b/src/type_engine/variable/rhs_resolution/mod.rs index ec75551c3..cd28f5a43 100644 --- a/src/type_engine/variable/rhs_resolution/mod.rs +++ b/src/type_engine/variable/rhs_resolution/mod.rs @@ -72,7 +72,7 @@ pub(crate) use calls::{ }; pub(crate) use instantiation::{ TemplateBindingMode, array_element_binding, classify_template_binding, extract_array_position, - extract_generic_arg_from_ancestor, remap_inherited_ctor_subs, type_contains_name, + remap_inherited_ctor_subs, type_contains_name, }; /// The type of a member access whose name PHP only works out at runtime. diff --git a/src/virtual_members/laravel/mod.rs b/src/virtual_members/laravel/mod.rs index 68c5c7625..a5867977e 100644 --- a/src/virtual_members/laravel/mod.rs +++ b/src/virtual_members/laravel/mod.rs @@ -215,7 +215,8 @@ 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, resolve_relation_chain_details, + RELATION_QUERY_METHODS, related_model_type, resolve_relation_chain, + resolve_relation_chain_details, }; use relationships::{ RelationshipKind, build_property_type, count_property_name, extract_pivot_accessor_typed, diff --git a/src/virtual_members/laravel/relationships.rs b/src/virtual_members/laravel/relationships.rs index edab4b37b..6595124ee 100644 --- a/src/virtual_members/laravel/relationships.rs +++ b/src/virtual_members/laravel/relationships.rs @@ -512,13 +512,13 @@ pub(crate) fn resolve_relation_chain( cache: Option<&super::super::ResolvedClassCache>, ) -> Option { resolve_relation_chain_details(model, chain, class_loader, cache) - .map(|relation| relation.model.fqn().to_string()) + .and_then(|relation| relation.models.first().map(|model| 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, + /// All final related models, retaining instantiated generic members. + pub models: Vec>, /// The relationship with self references bound to its declaring model. pub relation_type: PhpType, } @@ -533,55 +533,132 @@ pub(crate) fn resolve_relation_chain_details( cache: Option<&super::super::ResolvedClassCache>, ) -> Option { let mut segments = chain.split('.').peekable(); - let mut current_class = resolve_class_with_inheritance(model, class_loader, cache); + let mut current = vec![crate::inheritance::ClassRef::Borrowed(model)]; while let Some(segment) = segments.next() { let segment = segment.trim(); if segment.is_empty() { return None; } - 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 related = resolve_related_fqn(&related_type, ¤t_class, class_loader)?; + let mut models = Vec::new(); + let mut relations = Vec::new(); + for class in ¤t { + // Instantiated receivers already carry substituted members. Only + // resolve the class again when the method is not present on it. + let method = class.get_method_arc(segment).or_else(|| { + crate::virtual_members::resolve_class_fully_maybe_cached(class, class_loader, cache) + .get_method_arc(segment) + }); + let Some(return_type) = method + .as_ref() + .and_then(|method| method.return_type.as_ref()) + else { + continue; + }; + let relation = return_type.replace_self_bound(&class.fqn(), None); + collect_relation_models( + &relation, + class, + class_loader, + cache, + &mut models, + &mut relations, + ); + } + if models.is_empty() { + return None; + } if segments.peek().is_none() { return Some(ResolvedRelation { - model: related, - relation_type: return_type.replace_self_bound(¤t_class.fqn(), None), + models, + relation_type: PhpType::union(relations).simplified(), }); } - current_class = resolve_class_with_inheritance(&related, class_loader, cache); + current = models + .into_iter() + .map(crate::inheritance::ClassRef::Owned) + .collect(); } None } -/// Resolve a class fully (with inheritance and virtual members) so that -/// relationship methods from traits and parent classes are visible. -fn resolve_class_with_inheritance( - class: &ClassInfo, +fn collect_relation_models( + relation: &PhpType, + declaring: &ClassInfo, class_loader: &dyn Fn(&str) -> Option>, cache: Option<&super::super::ResolvedClassCache>, -) -> Arc { - crate::virtual_members::resolve_class_fully_maybe_cached(class, class_loader, cache) + models: &mut Vec>, + relations: &mut Vec, +) { + if let TypeKind::Union(parts) = relation.kind() { + for part in parts { + collect_relation_models(part, declaring, class_loader, cache, models, relations); + } + return; + } + let Some(related) = related_model_type(relation, class_loader) else { + return; + }; + if collect_related_classes(&related, declaring, class_loader, cache, models) { + relations.push(relation.clone()); + } } -/// Extract the related type from a relationship return type string, -/// resolving `$this` / `static` to the declaring class. -fn extract_related_type_for_chain( - return_type: &PhpType, - declaring_class: &ClassInfo, -) -> Option { - classify_relationship_typed(return_type)?; +/// Project a relationship's related-model binding through custom ancestry. +/// Built-in relationship hints can also resolve without installed stubs. +pub(crate) fn related_model_type( + relation: &PhpType, + class_loader: &dyn Fn(&str) -> Option>, +) -> Option { + if classify_relationship_typed(relation).is_some() { + return extract_related_type_typed(relation).cloned(); + } + const RELATION: &str = "Illuminate\\Database\\Eloquent\\Relations\\Relation"; + if !crate::class_lookup::is_subtype_of_typed( + relation, + &PhpType::named(atom(RELATION)), + class_loader, + ) { + return None; + } + crate::inheritance::extract_generic_arg_from_ancestor(relation, RELATION, 0, class_loader) +} - // Check the first generic arg directly as a PhpType before - // stringifying, so we can use the `is_self_ref()` predicate - // instead of comparing raw strings. - if let TypeKind::Generic(g) = return_type.kind() { - let first = g.args.first()?; - if first.is_self_ref() { - return Some(declaring_class.fqn().to_string()); +fn collect_related_classes( + related: &PhpType, + declaring: &ClassInfo, + class_loader: &dyn Fn(&str) -> Option>, + cache: Option<&super::super::ResolvedClassCache>, + models: &mut Vec>, +) -> bool { + if let TypeKind::Union(parts) = related.kind() { + let mut found = false; + for part in parts { + found |= collect_related_classes(part, declaring, class_loader, cache, models); } + return found; } - - extract_related_type_typed(return_type).and_then(|t| t.base_name().map(|s| s.to_string())) + let Some(model) = related + .base_name() + .and_then(|name| resolve_related_fqn(name, declaring, class_loader)) + else { + return false; + }; + let model = if let TypeKind::Generic(g) = related.kind() { + let strings = g.args.iter().map(ToString::to_string).collect::>(); + crate::virtual_members::resolve_class_fully_with_generics( + &model, + class_loader, + cache, + &strings, + &g.args, + ) + } else { + model + }; + if !models.iter().any(|existing| Arc::ptr_eq(existing, &model)) { + models.push(model); + } + true } /// Resolve a short or FQN related type to a loadable FQN. diff --git a/src/virtual_members/laravel/relationships_tests.rs b/src/virtual_members/laravel/relationships_tests.rs index d37bf0356..928b49df9 100644 --- a/src/virtual_members/laravel/relationships_tests.rs +++ b/src/virtual_members/laravel/relationships_tests.rs @@ -831,7 +831,7 @@ fn relation_chain_details_keep_the_terminal_declaring_model() { 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.models[0].fqn(), "Warehouse"); assert_eq!( relation.relation_type, PhpType::parse("BelongsTo") diff --git a/tests/integration/completion_generics.rs b/tests/integration/completion_generics.rs index d6bceb98a..1acf6467a 100644 --- a/tests/integration/completion_generics.rs +++ b/tests/integration/completion_generics.rs @@ -9861,3 +9861,45 @@ async fn test_short_extends_arg_right_aligns_when_collecting_implements() { _ => panic!("Expected CompletionResponse::Array"), } } + +#[tokio::test] +async fn generic_union_keeps_each_instantiation_through_members() { + let backend = create_test_backend(); + let uri = Url::parse("file:///generic-union-members.php").unwrap(); + let source = r#"|ValueBox $box */ +function demonstrate(ValueBox $box): void { BODY } +"#; + for expression in ["$box->get()->", "$box->value->"] { + let content = source.replace("BODY", expression); + let position = crate::common::position_after(&content, expression); + let items = + crate::common::complete_at(&backend, &uri, &content, position.line, position.character) + .await; + for name in ["leftOnly", "rightOnly", "label"] { + assert!( + crate::common::method_names(&items).contains(&name), + "{expression}: missing {name}, got {:?}", + crate::common::method_names(&items) + ); + } + for (method, expected) in [("label", 0), ("missingUnionMethod", 1)] { + let content = source.replace("BODY", &format!("{expression}{method}();")); + let diagnostics = crate::common::unknown_member_diagnostics_with_scope_cache( + &backend, + uri.as_str(), + &content, + ); + assert_eq!(diagnostics.len(), expected, "{expression}: {diagnostics:?}"); + } + } +} diff --git a/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index 22afd07a9..02c3efc85 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -15820,3 +15820,166 @@ class MPFilterTest { labels ); } + +#[tokio::test] +async fn test_relation_callback_context_reaches_completion_and_diagnostics() { + let fixture = include_str!("../phpstan_nsrt/laravel-builder-relations.php"); + let backend = create_test_backend(); + let uri = Url::parse("file:///relation-context.php").unwrap(); + for (call, expected, direct) in [ + ( + "SpecialStock::whereHas('custom', fn ($contextQuery) => BODY);", + &["teamName"][..], + false, + ), + ( + "SpecialStock::whereHas('fixed', function ($contextQuery) { BODY });", + &["teamName"][..], + false, + ), + ( + "SpecialStock::whereHas('either', fn ($contextQuery) => BODY);", + &["teamName", "warehouseName"][..], + false, + ), + ( + "SpecialStock::whereHas('relationUnion', fn ($contextQuery) => BODY);", + &["teamName", "warehouseName"][..], + false, + ), + ( + "$builderUnion->whereHas('team', fn ($contextQuery) => BODY);", + &["teamName", "warehouseName"][..], + false, + ), + ( + "$modelUnion->whereHas('team', fn ($contextQuery) => BODY);", + &["teamName", "warehouseName"][..], + false, + ), + ( + "$generic->whereHas('target', fn ($contextQuery) => BODY);", + &["teamName"][..], + false, + ), + ( + "$reordered->whereHas('team', fn ($contextQuery) => BODY);", + &["teamName"][..], + false, + ), + ( + "OwnCallback::whereHas('team', fn ($contextQuery) => BODY);", + &["warehouseName"][..], + true, + ), + ( + "TraitCallback::whereHas('team', fn ($contextQuery) => BODY);", + &["warehouseName"][..], + true, + ), + ( + "AliasedCallback::whereHas('team', fn ($contextQuery) => BODY);", + &["warehouseName"][..], + true, + ), + ( + "Stock::WHEREHAS('team', fn ($contextQuery) => BODY);", + &["teamName"][..], + false, + ), + ( + "Stock::query()->WhereHas('team', function ($contextQuery) { BODY });", + &["teamName"][..], + false, + ), + ( + "Stock::with(['team' => fn ($contextQuery) => BODY]);", + &["teamName"][..], + false, + ), + ( + "Stock::query()?->WITH(relations: ['team' => function (Relation $contextQuery) { BODY }]);", + &["teamName"][..], + false, + ), + ( + "Team::with(['stocks' => ['warehouse' => fn ($contextQuery) => BODY]]);", + &["warehouseName"][..], + false, + ), + ( + "Team::query()->with(array('stocks.warehouse' => function ($contextQuery) { BODY }));", + &["warehouseName"][..], + false, + ), + ( + "Stock::with((['team' => fn ($contextQuery) => BODY]));", + &["teamName"][..], + false, + ), + ( + "Stock::with(['team' => (function ($contextQuery) { BODY })]);", + &["teamName"][..], + false, + ), + ( + "$builderUnion->with(['team' => fn ($contextQuery) => BODY]);", + &["teamName", "warehouseName"][..], + false, + ), + ] { + let source = format!( + r#"{fixture} +namespace BuilderRelationAudit {{ + use Illuminate\Database\Eloquent\Builder; + use Illuminate\Database\Eloquent\Relations\Relation; + /** + * @param Builder|Builder $builderUnion + * @param GenericStock $generic + * @param ReorderedBuilder $reordered + */ + function contextCalls(Builder $builderUnion, Stock|OtherStock $modelUnion, GenericStock $generic, ReorderedBuilder $reordered): void {{ {call} }} +}} +"# + ); + let receiver = if direct { + "$contextQuery->" + } else { + "$contextQuery->getModel()->" + }; + let content = source.replace("BODY", receiver); + let position = crate::common::position_after(&content, receiver); + let items = + crate::common::complete_at(&backend, &uri, &content, position.line, position.character) + .await; + let methods = method_names(&items); + for name in expected { + assert!( + methods.contains(name), + "{call}: missing {name}, got {methods:?}" + ); + } + let valid = if direct { + "$contextQuery->warehouseName()" + } else { + "$contextQuery->getModel()" + }; + for (expression, errors) in [(valid, 0), ("$contextQuery->missingContextMethod()", 1)] { + let body = if call.contains("fn (") { + expression.to_string() + } else { + format!("{expression};") + }; + let content = source.replace("BODY", &body); + let diagnostics = crate::common::unknown_member_diagnostics_with_scope_cache( + &backend, + uri.as_str(), + &content, + ); + assert_eq!(diagnostics.len(), errors, "{call}: {diagnostics:?}"); + if errors != 0 { + assert!(diagnostics[0].message.contains("missingContextMethod")); + } + } + } +} diff --git a/tests/integration/definition_laravel.rs b/tests/integration/definition_laravel.rs index 1d5a3a288..da3d16556 100644 --- a/tests/integration/definition_laravel.rs +++ b/tests/integration/definition_laravel.rs @@ -4396,3 +4396,40 @@ async fn test_relation_argument_callback_navigates_to_custom_builder() { ); } } + +#[tokio::test] +async fn relation_context_callbacks_navigate_to_their_declared_members() { + let fixture = include_str!("../phpstan_nsrt/laravel-builder-relations.php"); + let backend = crate::common::create_test_backend(); + let uri = Url::parse("file:///relation-context-definition.php").unwrap(); + for (call, member, declaration) in [ + ( + "SpecialStock::whereHas('custom', fn ($q) => $q->active());", + "$q->act", + "public function active", + ), + ( + "OwnCallback::whereHas('team', fn ($q) => $q->warehouseName());", + "$q->warehouseNa", + "public function warehouseName", + ), + ( + "Stock::with(['team' => fn ($q) => $q->getModel()->teamName()]);", + "getModel()->teamNa", + "public function teamName", + ), + ] { + let content = format!("{fixture}\nnamespace BuilderRelationAudit {{ {call} }}"); + open_php(&backend, &uri, &content).await; + let position = crate::common::position_after(&content, member); + let expected = crate::common::position_after(fixture, declaration); + let result = goto_definition_at(&backend, &uri, position.line, position.character).await; + let locations = crate::common::definition_locations(result); + assert_eq!(locations.len(), 1, "{call}: {locations:?}"); + assert_eq!(locations[0].uri, uri); + assert_eq!( + locations[0].range.start.line, expected.line, + "{call}: {locations:?}" + ); + } +} diff --git a/tests/phpstan_nsrt/laravel-builder-relations.php b/tests/phpstan_nsrt/laravel-builder-relations.php index c4312fad6..edd49af66 100644 --- a/tests/phpstan_nsrt/laravel-builder-relations.php +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -662,3 +662,237 @@ function morphs(): void { assertType('Illuminate\Database\Eloquent\Builder', Comment::whereHasMorph('commentable', [Team::class], null)); } } + +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\Relation; + use function PHPStan\Testing\assertType; + + class SpecialStock extends Stock { + /** @return ReorderedRelation<$this, Team> */ + public function custom(): ReorderedRelation {} + /** @return FixedTeamRelation */ + public function fixed(): FixedTeamRelation {} + /** @return BelongsTo */ + public function either(): BelongsTo {} + /** @return BelongsTo|HasMany */ + public function relationUnion(): BelongsTo|HasMany {} + /** @return BelongsTo|HasMany */ + public function sharedTeam(): BelongsTo|HasMany {} + } + function customNamedRelations(): void { + SpecialStock::whereHas('custom', function ($q) { + assertType('BuilderRelationAudit\TeamBuilder', $q); + }); + SpecialStock::whereHas('fixed', function ($q) { + assertType('BuilderRelationAudit\TeamBuilder', $q); + }); + SpecialStock::query()->with('custom', function ($q) { + assertType('BuilderRelationAudit\ReorderedRelation', $q); + }); + SpecialStock::whereHas('either', function ($q) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder', $q); + }); + SpecialStock::whereHas('relationUnion', function ($q) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder', $q); + }); + } + + class OtherStock extends Model { + /** @return BelongsTo */ + public function team(): BelongsTo {} + } + /** @param Builder|Builder $q */ + function receiverAlternatives(Builder $q, Stock|OtherStock $model): void { + $q->whereHas('team', function ($related) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder', $related); + }); + $model->whereHas('team', function ($related) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder', $related); + }); + } + + /** + * @template TExtra + * @template TRelated of Model + * @extends Builder + */ + class ReorderedBuilder extends Builder {} + /** @param ReorderedBuilder $builder */ + function reorderedBuilderReceiver(ReorderedBuilder $builder): void { + $builder->whereHas('team', function ($related) { + assertType('BuilderRelationAudit\TeamBuilder', $related); + }); + } + + class OwnCallback extends Stock { + /** @param \Closure(Warehouse): mixed $callback */ + public static function whereHas($relation, ?\Closure $callback = null, $operator = '>=', $count = 1) {} + } + function ownDeclaration(): void { + OwnCallback::whereHas('team', function ($related) { + assertType('BuilderRelationAudit\Warehouse', $related); + }); + } + + function caseInsensitiveMethod(): void { + Stock::WHEREHAS('team', function ($related) { + assertType('BuilderRelationAudit\TeamBuilder', $related); + }); + Stock::query()->WhereHas('team', function ($related) { + assertType('BuilderRelationAudit\TeamBuilder', $related); + }); + } + + /** + * @template TModel of Model + */ + class GenericStock extends Model { + /** @return BelongsTo */ + public function target(): BelongsTo {} + } + /** @param GenericStock $stock */ + function genericModelReceiver(GenericStock $stock): void { + $stock->whereHas('target', function ($related) { + assertType('BuilderRelationAudit\TeamBuilder', $related); + }); + } + + function nestedEagerClosure(): void { + Stock::with(['team' => function ($related) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo', $related); + }]); + } +} + +namespace BuilderRelationAudit { + use Illuminate\Database\Eloquent\Builder; + use Illuminate\Database\Eloquent\Relations\Relation; + use function PHPStan\Testing\assertType; + + trait CallbackDeclaration { + /** @param \Closure(Warehouse): mixed $callback */ + public static function whereHas($relation, ?\Closure $callback = null, $operator = '>=', $count = 1) {} + } + trait NestedCallbackDeclaration { use CallbackDeclaration; } + class TraitCallback extends Stock { use NestedCallbackDeclaration; } + class ChildCallback extends OwnCallback {} + trait AliasedCallbackDeclaration { + /** @param \Closure(Warehouse): mixed $callback */ + public static function constraint($relation, ?\Closure $callback = null) {} + } + class AliasedCallback extends Stock { use AliasedCallbackDeclaration { constraint as whereHas; } } + class OwnCallbackBuilder extends Builder { + /** @param \Closure(Warehouse): mixed $callback */ + public function whereHas($relation, ?\Closure $callback = null, $operator = '>=', $count = 1) { return $this; } + } + function callbackDeclarations(OwnCallbackBuilder $builder): void { + TraitCallback::whereHas('team', function ($q) { + assertType('BuilderRelationAudit\Warehouse', $q); + }); + ChildCallback::whereHas('team', function ($q) { + assertType('BuilderRelationAudit\Warehouse', $q); + }); + AliasedCallback::whereHas('team', function ($q) { + assertType('BuilderRelationAudit\Warehouse', $q); + }); + $builder->whereHas('team', function ($q) { + assertType('BuilderRelationAudit\Warehouse', $q); + }); + } + /** @param Stock|OwnCallback $model */ + function mixedCallbackDeclarations(Stock $model): void { + $model->whereHas('team', function ($q) { + assertType('BuilderRelationAudit\TeamBuilder|BuilderRelationAudit\Warehouse', $q); + }); + } + + function nestedNamedRelations(): void { + SpecialStock::query()->with('sharedTeam', function ($q) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo|Illuminate\Database\Eloquent\Relations\HasMany', $q); + }); + assertType('Illuminate\Database\Eloquent\Builder', Stock::with(['team', 'warehouse'])); + assertType('Illuminate\Database\Eloquent\Builder', Stock::query()->with(['team' => ['stocks']])); + SpecialStock::whereHas('custom.stocks.warehouse', function ($q) { + assertType('Illuminate\Database\Eloquent\Builder', $q); + }); + SpecialStock::whereHas('either.stocks', function ($q) { + assertType('Illuminate\Database\Eloquent\Builder', $q); + }); + SpecialStock::withWhereHas('either', function (Builder|Relation $q) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder|Illuminate\Database\Eloquent\Relations\BelongsTo', $q); + }); + } + /** @param Builder|Builder $builder */ + function eagerReceiverAlternatives(Builder $builder): void { + $builder->with('team', function ($q) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo|Illuminate\Database\Eloquent\Relations\BelongsTo', $q); + }); + } + + /** @param 'team'|'warehouse' $name */ + function eagerArrayKeys(string $name, string $unknown): void { + $key = 'team'; + Stock::WITH(relations: [$key => function (Relation $q) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo', $q); + }]); + Stock::query()?->with(relations: [$name => function ($q) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo|Illuminate\Database\Eloquent\Relations\BelongsTo', $q); + }]); + Stock::with([$unknown => function ($q) { + assertType('Illuminate\Database\Eloquent\Relations\Relation', $q); + }]); + Team::with(['stocks' => ['warehouse' => function ($q) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo', $q); + }]]); + Team::query()->with(array('stocks.warehouse' => function ($q) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo', $q); + })); + Stock::with(['team' => (function ($q) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo', $q); + })]); + Stock::with(['team' => function (Warehouse $q) { + assertType('BuilderRelationAudit\Warehouse', $q); + }]); + SpecialStock::with(['custom' => function ($q) { + assertType('BuilderRelationAudit\ReorderedRelation', $q); + }]); + } +} + +namespace BuilderRelationAudit { + use function PHPStan\Testing\assertType; + /** @param GenericStock|GenericStock $model */ + function genericReceiverAlternatives(GenericStock $model): void { + $model->whereHas('target', function ($query) { + assertType('BuilderRelationAudit\TeamBuilder|Illuminate\Database\Eloquent\Builder', $query); + }); + } +} + +namespace CustomRelations { + /** + * @template TParent of \Illuminate\Database\Eloquent\Model + * @template TRelated of \Illuminate\Database\Eloquent\Model + * @extends \Illuminate\Database\Eloquent\Relations\BelongsTo + */ + class Relation extends \Illuminate\Database\Eloquent\Relations\BelongsTo {} +} +namespace BuilderRelationAudit { + use function PHPStan\Testing\assertType; + class RelationNamedStock extends Stock { + /** @return \CustomRelations\Relation<$this, Team> */ + public function custom(): \CustomRelations\Relation {} + } + function relationNameCollision(RelationNamedStock $model): void { + RelationNamedStock::whereHas('custom', function ($q) { + assertType('BuilderRelationAudit\TeamBuilder', $q); + }); + RelationNamedStock::whereHas($model->custom(), function ($q) { + assertType('BuilderRelationAudit\TeamBuilder', $q); + }); + } +} From c77963854fbe17a997ef0d4ba717950710a27bc4 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Mon, 21 Sep 2026 01:09:27 +0600 Subject: [PATCH 9/9] test(laravel): complete relation callback patch coverage Cover fallback and recovery paths across the stacked L61 diff, including missing stubs, stale metadata, ancestry cycles, union paths, and dynamic callbacks. Simplify unreachable relation-chain and eager-call fallthroughs. Keep assertions outside the parser panic boundary so failures cannot be swallowed. Verify 947 of 947 changed executable lines against main with a clean LLVM coverage run, plus the full tests and required lint checks. --- src/inheritance/ancestor.rs | 73 ++++++ .../variable/closure_resolution.rs | 4 + .../variable/closure_resolution_tests.rs | 131 ++++++++++ .../forward_walk/relation_callbacks.rs | 60 ++--- .../forward_walk/relation_callbacks_tests.rs | 231 ++++++++++++++++++ src/virtual_members/laravel/relationships.rs | 5 +- .../laravel/relationships_tests.rs | 31 +++ src/virtual_members/laravel/tests.rs | 10 + tests/integration/completion_laravel.rs | 34 +++ .../laravel-builder-relations.php | 23 +- 10 files changed, 564 insertions(+), 38 deletions(-) create mode 100644 src/type_engine/variable/closure_resolution_tests.rs create mode 100644 src/type_engine/variable/forward_walk/relation_callbacks_tests.rs diff --git a/src/inheritance/ancestor.rs b/src/inheritance/ancestor.rs index ab1afdf7b..7c23336f3 100644 --- a/src/inheritance/ancestor.rs +++ b/src/inheritance/ancestor.rs @@ -153,3 +153,76 @@ fn ancestor_name_matches(actual: &str, target: &str) -> bool { crate::util::short_name(actual).eq_ignore_ascii_case(target) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::atom::atom; + use crate::test_fixtures::{make_class, no_loader}; + + #[test] + fn projection_combines_union_bindings_and_ignores_non_classes() { + for (input, expected) in [ + ("Container|Container|int", Some("Left|Right")), + ("int|string", None), + ("array{item: Container}", None), + ] { + assert_eq!( + extract_generic_arg_from_ancestor( + &PhpType::parse(input), + "Container", + 0, + &no_loader + ), + expected.map(PhpType::parse), + "{input}", + ); + } + assert!( + extract_generic_arg_from_ancestor( + &PhpType::parse("Container"), + "Container", + 1, + &no_loader + ) + .is_none() + ); + } + + #[test] + fn projection_survives_missing_ancestors_and_cycles() { + let mut first = make_class("First"); + first.parent_class = Some(atom("Second")); + first.interfaces = vec![atom("Missing"), atom("Values")]; + let mut second = make_class("Second"); + second.parent_class = Some(atom("First")); + let mut values = make_class("Values"); + values.extends_generics = vec![("Container".into(), vec![PhpType::parse("Item")])]; + let classes = [Arc::new(first), Arc::new(second), Arc::new(values)]; + let loader = |name: &str| classes.iter().find(|class| class.fqn() == name).cloned(); + assert_eq!( + extract_generic_arg_from_ancestor(&PhpType::parse("First"), "Container", 0, &loader), + Some(PhpType::parse("Item")), + ); + assert!( + extract_generic_arg_from_ancestor(&PhpType::parse("First"), "Unknown", 0, &loader) + .is_none() + ); + } + + #[test] + fn projection_bounds_deep_ancestry() { + let classes: Vec<_> = (0..=MAX_ANCESTOR_GENERIC_DEPTH + 2) + .map(|index| { + let mut class = make_class(&format!("Level{index}")); + class.parent_class = Some(atom(&format!("Level{}", index + 1))); + Arc::new(class) + }) + .collect(); + let loader = |name: &str| classes.iter().find(|class| class.fqn() == name).cloned(); + assert!( + extract_generic_arg_from_ancestor(&PhpType::parse("Level0"), "Container", 0, &loader) + .is_none() + ); + } +} diff --git a/src/type_engine/variable/closure_resolution.rs b/src/type_engine/variable/closure_resolution.rs index d925bc5cc..d203d5d58 100644 --- a/src/type_engine/variable/closure_resolution.rs +++ b/src/type_engine/variable/closure_resolution.rs @@ -1173,3 +1173,7 @@ pub(in crate::type_engine) fn inferred_type_is_more_specific_pub( ) -> bool { inferred_type_is_more_specific(explicit_hint, inferred, class_loader) } + +#[cfg(test)] +#[path = "closure_resolution_tests.rs"] +mod tests; diff --git a/src/type_engine/variable/closure_resolution_tests.rs b/src/type_engine/variable/closure_resolution_tests.rs new file mode 100644 index 000000000..f99ebc10e --- /dev/null +++ b/src/type_engine/variable/closure_resolution_tests.rs @@ -0,0 +1,131 @@ +use super::*; +use crate::test_fixtures::{make_class, make_method, no_loader}; + +fn context(loader: &dyn Fn(&str) -> Option>) -> ResolutionCtx<'_> { + ResolutionCtx { + current_class: None, + all_classes: &[], + content: "", + cursor_offset: 0, + class_loader: loader, + backend: None, + laravel_macro_this_resolver: None, + resolved_class_cache: None, + function_loader: None, + scope_var_resolver: None, + is_in_static_method: false, + preserve_static: false, + } +} + +#[test] +fn relation_receivers_ignore_scalars_and_unrelated_classes() { + let receivers = [ + ResolvedType::from_type_string(PhpType::parse("string")), + ResolvedType::from_class(make_class("UnrelatedQuery")), + ResolvedType::from_class(make_class(ELOQUENT_MODEL_FQN)), + ]; + let ctx = context(&no_loader); + let models = find_models_from_receivers(&receivers, &ctx); + assert_eq!(models.len(), 1); + assert_eq!(models[0].fqn(), ELOQUENT_MODEL_FQN); + assert!( + try_relation_query_override(&receivers, "map", &PhpType::parse("'items'"), None, &ctx) + .is_none() + ); + assert!( + try_relation_query_override( + &receivers[..2], + "whereHas", + &PhpType::parse("string"), + None, + &ctx + ) + .is_none() + ); +} + +#[test] +fn builder_receiver_recovers_model_from_instantiated_methods() { + let mut model = make_class("App\\Product"); + model.parent_class = Some(atom(ELOQUENT_MODEL_FQN)); + let model = Arc::new(model); + let base = Arc::new(make_class(ELOQUENT_MODEL_FQN)); + let loader = |name: &str| match name { + "App\\Product" => Some(Arc::clone(&model)), + ELOQUENT_MODEL_FQN => Some(Arc::clone(&base)), + _ => None, + }; + for (method, return_type) in [ + ("getModel", "App\\Product"), + ( + "where", + "Illuminate\\Database\\Eloquent\\Builder", + ), + ] { + let mut builder = make_class(ELOQUENT_BUILDER_FQN); + builder + .methods + .push(Arc::new(make_method(method, Some(return_type)))); + let models = + find_models_from_receivers(&[ResolvedType::from_class(builder)], &context(&loader)); + assert_eq!(models.len(), 1, "{method}"); + assert_eq!(models[0].fqn(), "App\\Product", "{method}"); + } + assert!( + find_models_from_receivers( + &[ResolvedType::from_class(make_class(ELOQUENT_BUILDER_FQN))], + &context(&loader), + ) + .is_empty() + ); +} + +#[test] +fn unresolved_morph_relations_use_base_builder_without_discarding_candidates() { + let mut product = make_class("App\\Product"); + product.parent_class = Some(atom(ELOQUENT_MODEL_FQN)); + let product = Arc::new(product); + let model = Arc::new(make_class(ELOQUENT_MODEL_FQN)); + let loader = |name: &str| match name { + "App\\Product" => Some(Arc::clone(&product)), + ELOQUENT_MODEL_FQN => Some(Arc::clone(&model)), + _ => None, + }; + let receivers = [ResolvedType::from_arc(Arc::clone(&model))]; + for (candidates, expected) in [ + ( + "'*'", + "Illuminate\\Database\\Eloquent\\Builder", + ), + ( + "class-string", + "Illuminate\\Database\\Eloquent\\Builder", + ), + ( + "class-string|string", + "Illuminate\\Database\\Eloquent\\Builder|Illuminate\\Database\\Eloquent\\Builder", + ), + ] { + assert_eq!( + try_relation_query_override( + &receivers, + "whereHasMorph", + &PhpType::parse("'missing'"), + Some(&PhpType::parse(candidates)), + &context(&loader) + ), + Some(vec![PhpType::parse(expected).simplified()]), + "{candidates}", + ); + } + assert!( + relation_callback_type( + &model, + "whereHas", + &PhpType::parse("int"), + &context(&loader) + ) + .is_none() + ); +} diff --git a/src/type_engine/variable/forward_walk/relation_callbacks.rs b/src/type_engine/variable/forward_walk/relation_callbacks.rs index b66848890..baf2c5245 100644 --- a/src/type_engine/variable/forward_walk/relation_callbacks.rs +++ b/src/type_engine/variable/forward_walk/relation_callbacks.rs @@ -209,10 +209,10 @@ pub(crate) fn eager_callback_arguments<'a>( scope: &ScopeState, ctx: &ForwardWalkCtx<'_>, ) -> Option>> { - let (selector, arguments) = match call { - Call::Method(call) => (&call.method, &call.argument_list), - Call::NullSafeMethod(call) => (&call.method, &call.argument_list), - Call::StaticMethod(call) => (&call.method, &call.argument_list), + let (selector, arguments, receiver, is_static) = match call { + Call::Method(call) => (&call.method, &call.argument_list, call.object, false), + Call::NullSafeMethod(call) => (&call.method, &call.argument_list, call.object, false), + Call::StaticMethod(call) => (&call.method, &call.argument_list, call.class, true), _ => return None, }; let ClassLikeMemberSelector::Identifier(name) = selector else { @@ -238,36 +238,24 @@ pub(crate) fn eager_callback_arguments<'a>( Some(scope.proofs()), ); let rctx = var_ctx.as_resolution_ctx(); - let receivers = match call { - Call::Method(call) => { - let span = call.object.span(); - super::super::closure_resolution::resolve_receiver_types( - span.start.offset, - span.end.offset, - &rctx, - ) - } - Call::NullSafeMethod(call) => { - let span = call.object.span(); - super::super::closure_resolution::resolve_receiver_types( - span.start.offset, - span.end.offset, - &rctx, - ) - } - Call::StaticMethod(call) => { - let name = super::super::closure_resolution::static_receiver_class_name( - call.class, - Some(ctx.current_class), - )?; - let owner = super::super::closure_resolution::find_owner_by_name( - &name, - ctx.all_classes, - ctx.class_loader, - )?; - vec![ResolvedType::from_class(owner)] - } - _ => return None, + let receivers = if is_static { + let name = super::super::closure_resolution::static_receiver_class_name( + receiver, + Some(ctx.current_class), + )?; + let owner = super::super::closure_resolution::find_owner_by_name( + &name, + ctx.all_classes, + ctx.class_loader, + )?; + vec![ResolvedType::from_class(owner)] + } else { + let span = receiver.span(); + super::super::closure_resolution::resolve_receiver_types( + span.start.offset, + span.end.offset, + &rctx, + ) }; let receivers: Vec<_> = receivers .into_iter() @@ -410,3 +398,7 @@ fn join_relation_path(prefix: Option<&PhpType>, key: &PhpType) -> PhpType { _ => PhpType::named(atom("string")), } } + +#[cfg(test)] +#[path = "relation_callbacks_tests.rs"] +mod tests; diff --git a/src/type_engine/variable/forward_walk/relation_callbacks_tests.rs b/src/type_engine/variable/forward_walk/relation_callbacks_tests.rs new file mode 100644 index 000000000..f12ec887b --- /dev/null +++ b/src/type_engine/variable/forward_walk/relation_callbacks_tests.rs @@ -0,0 +1,231 @@ +use super::*; +use crate::test_fixtures::{ + make_class, make_method, make_method_with_params, make_param, no_loader, +}; +use crate::type_engine::resolver::Loaders; +use crate::types::TraitPrecedence; +use crate::virtual_members::laravel::ELOQUENT_BUILDER_FQN; + +fn context<'a>( + class: &'a ClassInfo, + content: &'a str, + loader: &'a dyn Fn(&str) -> Option>, +) -> ForwardWalkCtx<'a> { + ForwardWalkCtx { + current_class: class, + all_classes: &[], + content, + cursor_offset: content.len() as u32, + class_loader: loader, + backend: None, + loaders: Loaders::default(), + resolved_class_cache: None, + enclosing_return_type: None, + top_level_scope: None, + } +} + +#[test] +fn callback_declaration_ownership_respects_docblocks_trait_precedence_and_cycles() { + let mut documented = make_class("Documented"); + documented.set_class_docblock(Some( + "/** @method void whereHas(string $relation, callable $callback) */".into(), + )); + assert_eq!( + declared_relation_method(&documented, "whereHas", &no_loader, 0), + Some(false) + ); + + let framework_name = "Illuminate\\Database\\Eloquent\\Concerns\\QueriesRelationships"; + let mut framework = make_class(framework_name); + framework + .methods + .push(Arc::new(make_method("whereHas", None))); + let mut excluded = make_class("Excluded"); + excluded + .methods + .push(Arc::new(make_method("whereHas", None))); + let classes = [ + Arc::new(framework), + Arc::new(excluded), + Arc::new(make_class("Empty")), + ]; + let loader = |name: &str| classes.iter().find(|class| class.fqn() == name).cloned(); + let mut owner = make_class("Owner"); + owner.used_traits = vec![atom("Excluded"), atom("Empty"), atom(framework_name)]; + owner.trait_precedences.push(TraitPrecedence { + trait_name: atom(framework_name), + method_name: atom("whereHas"), + insteadof: vec![atom("Excluded")], + }); + assert_eq!( + declared_relation_method(&owner, "whereHas", &loader, 0), + Some(true) + ); + assert_eq!(declared_relation_method(&owner, "with", &loader, 0), None); + + let mut cycle = make_class("Cycle"); + cycle.parent_class = Some(atom("Cycle")); + let cycle = Arc::new(cycle); + let loader = |name: &str| (name == "Cycle").then(|| Arc::clone(&cycle)); + assert_eq!( + declared_relation_method(&cycle, "whereHas", &loader, 0), + Some(false) + ); + assert!(!is_framework_relation_method( + &owner, + "with", + &context(&owner, "", &no_loader) + )); + let loader = |name: &str| (name == "Owner").then(|| Arc::new(owner.clone())); + assert!(!is_framework_relation_method( + &owner, + "with", + &context(&owner, "", &loader) + )); +} + +#[test] +fn relation_callback_inference_rejects_incomplete_receivers_and_signatures() { + let content = " match statement.expression { + Expression::Call(Call::StaticMethod(call)) => Some(call), + _ => None, + }, + _ => None, + })?; + Some( + infer_relation_callback_params( + std::slice::from_ref(&receiver), + "whereHas", + 1, + &call.argument_list, + &ScopeState::new(), + &ctx, + ) + .is_none(), + ) + }, + ); + assert_eq!(inferred, Some(true), "{:?}", receiver.type_string); + } +} + +#[test] +fn eager_callbacks_require_a_known_framework_receiver_and_relations_parameter() { + let mut builder = make_class(ELOQUENT_BUILDER_FQN); + builder.methods.push(Arc::new(make_method_with_params( + "with", + None, + vec![make_param("$relations", None, true)], + ))); + let raw = Arc::new(builder.clone()); + let empty = make_class(""); + let instance = "with(['items' => function ($item) {}]);"; + let static_call = + " function ($item) {}]);"; + for (content, receiver) in [ + ( + instance, + ResolvedType::from_type_string(PhpType::parse("string")), + ), + (instance, ResolvedType::from_class(make_class("Unrelated"))), + ( + static_call, + ResolvedType::from_class(make_class(ELOQUENT_BUILDER_FQN)), + ), + (static_call, { + let mut renamed = builder.clone(); + renamed.methods = vec![Arc::new(make_method_with_params( + "with", + None, + vec![make_param("$loads", None, true)], + ))] + .into(); + ResolvedType::from_class(renamed) + }), + ] { + let loader = |name: &str| (name == ELOQUENT_BUILDER_FQN).then(|| Arc::clone(&raw)); + // A local declaration or cached class can lag behind the loader's + // current method metadata during an edit. + let all_classes: Vec<_> = receiver.class_info.iter().cloned().collect(); + let cache = crate::virtual_members::new_resolved_class_cache(); + for class in &all_classes { + crate::virtual_members::resolve_class_fully_cached(class, &no_loader, &cache); + } + let ctx = ForwardWalkCtx { + all_classes: &all_classes, + resolved_class_cache: Some(&cache), + ..context(&empty, content, &loader) + }; + let mut scope = ScopeState::new(); + scope.locals.insert(atom("$query"), vec![receiver]); + assert_no_eager_context(content, &scope, &ctx); + } + let ctx = context(&empty, "", &no_loader); + for content in [ + "{$method}(['items' => function ($item) {}]);", + " function ($item) {}]);", + " function ($item) {}]);", + ] { + assert_no_eager_context( + content, + &ScopeState::new(), + &ForwardWalkCtx { + content, + ..ctx.with_cursor_offset(content.len() as u32) + }, + ); + } +} + +fn assert_no_eager_context(content: &str, scope: &ScopeState, ctx: &ForwardWalkCtx<'_>) { + let rejected = + crate::parser::with_parsed_program(content, "unresolved_eager_callback", |program, _| { + let call = program + .statements + .iter() + .find_map(|statement| match statement { + Statement::Expression(statement) => match statement.expression { + Expression::Call(call) => Some(call), + _ => None, + }, + _ => None, + })?; + Some(eager_callback_arguments(call, 0, scope, ctx).is_none()) + }); + assert_eq!(rejected, Some(true), "{content}"); +} diff --git a/src/virtual_members/laravel/relationships.rs b/src/virtual_members/laravel/relationships.rs index 6595124ee..70a467156 100644 --- a/src/virtual_members/laravel/relationships.rs +++ b/src/virtual_members/laravel/relationships.rs @@ -534,8 +534,8 @@ pub(crate) fn resolve_relation_chain_details( ) -> Option { let mut segments = chain.split('.').peekable(); let mut current = vec![crate::inheritance::ClassRef::Borrowed(model)]; - while let Some(segment) = segments.next() { - let segment = segment.trim(); + loop { + let segment = segments.next()?.trim(); if segment.is_empty() { return None; } @@ -578,7 +578,6 @@ pub(crate) fn resolve_relation_chain_details( .map(crate::inheritance::ClassRef::Owned) .collect(); } - None } fn collect_relation_models( diff --git a/src/virtual_members/laravel/relationships_tests.rs b/src/virtual_members/laravel/relationships_tests.rs index 928b49df9..e49dfc2ff 100644 --- a/src/virtual_members/laravel/relationships_tests.rs +++ b/src/virtual_members/laravel/relationships_tests.rs @@ -863,3 +863,34 @@ fn relation_chain_details_keep_the_terminal_declaring_model() { ); } } + +#[test] +fn relation_chain_instantiates_generic_related_models() { + let mut owner = make_class("Owner"); + owner.methods.push(Arc::new(make_method( + "holder", + Some("HasOne, $this>"), + ))); + let mut holder = make_class("Holder"); + holder.template_params = vec!["TItem".into()]; + holder + .methods + .push(Arc::new(make_method("item", Some("HasOne")))); + let classes = [ + Arc::new(owner), + Arc::new(holder), + Arc::new(make_class("Item")), + ]; + let loader = |name: &str| classes.iter().find(|class| class.fqn() == name).cloned(); + let cache = crate::virtual_members::new_resolved_class_cache(); + let relation = + resolve_relation_chain_details(&classes[0], "holder", &loader, Some(&cache)).unwrap(); + assert_eq!( + relation.models[0].get_method("item").unwrap().return_type, + Some(PhpType::parse("HasOne")) + ); + assert_eq!( + resolve_relation_chain(&classes[0], "holder.item", &loader, Some(&cache)).as_deref(), + Some("Item") + ); +} diff --git a/src/virtual_members/laravel/tests.rs b/src/virtual_members/laravel/tests.rs index 3ff0fd7e0..8f8a6a87d 100644 --- a/src/virtual_members/laravel/tests.rs +++ b/src/virtual_members/laravel/tests.rs @@ -10,6 +10,16 @@ use std::sync::Arc; // ── applies_to ────────────────────────────────────────────────────── +#[test] +fn model_builder_type_survives_missing_framework_and_custom_stubs() { + let mut model = make_class("App\\Models\\Product"); + model.parent_class = Some(atom(ELOQUENT_MODEL_FQN)); + let expected = PhpType::parse("Illuminate\\Database\\Eloquent\\Builder"); + assert_eq!(model_builder_type(&model, &no_loader), expected); + model.laravel_mut().custom_builder = Some(PhpType::parse("App\\MissingBuilder")); + assert_eq!(model_builder_type(&model, &no_loader), expected); +} + #[test] fn applies_to_model_subclass() { let provider = LaravelModelProvider; diff --git a/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index 02c3efc85..3b0f6cc31 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -15983,3 +15983,37 @@ namespace BuilderRelationAudit {{ } } } + +#[tokio::test] +async fn test_dynamic_callback_calls_preserve_explicit_parameter_types() { + let backend = create_test_backend(); + let uri = Url::parse("file:///dynamic-callbacks.php").unwrap(); + for call in [ + "$callable(function (Item $item) { BODY });", + "$object->{$method}(function (Item $item) { BODY });", + "$object?->{$method}(function (Item $item) { BODY });", + "Item::{$method}(function (Item $item) { BODY });", + ] { + let source = format!( + ""); + let position = crate::common::position_after(&content, "$item->"); + let items = + crate::common::complete_at(&backend, &uri, &content, position.line, position.character) + .await; + assert!(method_names(&items).contains(&"label"), "{call}: {items:?}"); + for (body, errors) in [("$item->label();", 0), ("$item->missing();", 1)] { + let content = source.replace("BODY", body); + let diagnostics = crate::common::unknown_member_diagnostics_with_scope_cache( + &backend, + uri.as_str(), + &content, + ); + assert_eq!(diagnostics.len(), errors, "{call}: {diagnostics:?}"); + if errors != 0 { + assert!(diagnostics[0].message.contains("missing")); + } + } + } +} diff --git a/tests/phpstan_nsrt/laravel-builder-relations.php b/tests/phpstan_nsrt/laravel-builder-relations.php index edd49af66..13e2af102 100644 --- a/tests/phpstan_nsrt/laravel-builder-relations.php +++ b/tests/phpstan_nsrt/laravel-builder-relations.php @@ -523,6 +523,11 @@ function relationShortcutCallbacks(): void { } function eagerRelations(): void { + Stock::query()->with(['team' => function ($query) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo', $query); + }], function ($query) { + assertType('Illuminate\Database\Eloquent\Relations\Relation', $query); + }); Team::withWhereHas('stocks', function ($query) { assertType('Illuminate\Database\Eloquent\Builder|Illuminate\Database\Eloquent\Relations\HasMany', $query); }); @@ -617,6 +622,13 @@ function morphCandidates(string $teamClass, array $classes, array $unknownClasse } function morphs(): void { + Comment::whereHasMorph('missing', '*', function ($query, $type) { + assertType('Illuminate\Database\Eloquent\Builder', $query); + assertType('string', $type); + }); + Comment::whereHasMorph('missing', Team::class, function ($query) { + assertType('BuilderRelationAudit\TeamBuilder', $query); + }); Comment::whereHasMorph('commentable', Warehouse::class, function ($query, $type) { assertType('Illuminate\Database\Eloquent\Builder', $query); assertType('string', $type); @@ -762,7 +774,7 @@ function genericModelReceiver(GenericStock $stock): void { } function nestedEagerClosure(): void { - Stock::with(['team' => function ($related) { + Stock::with(['warehouse', 'team' => function ($related) { assertType('Illuminate\Database\Eloquent\Relations\BelongsTo', $related); }]); } @@ -860,6 +872,15 @@ function eagerArrayKeys(string $name, string $unknown): void { SpecialStock::with(['custom' => function ($q) { assertType('BuilderRelationAudit\ReorderedRelation', $q); }]); + Team::with(['stocks' => [$name => function ($q) { + assertType('Illuminate\Database\Eloquent\Relations\BelongsTo|Illuminate\Database\Eloquent\Relations\BelongsTo', $q); + }]]); + Stock::with([$name => ['stocks' => function ($q) { + assertType('Illuminate\Database\Eloquent\Relations\HasMany', $q); + }]]); + Stock::with([$unknown => ['stocks' => function ($q) { + assertType('Illuminate\Database\Eloquent\Relations\Relation', $q); + }]]); } }