From ed541aaab6bbd7f68a76edcc549cdc7ad8856d04 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Sun, 2 Aug 2026 12:26:10 +0600 Subject: [PATCH 1/3] feat: factory count-conditional return types --- docs/todo.md | 1 - docs/todo/laravel.md | 38 -- examples/laravel/app/Demo.php | 31 +- examples/laravel/assertions.php | 34 ++ .../call_resolution/return_types.rs | 17 + src/virtual_members/laravel/factory.rs | 37 +- src/virtual_members/laravel/factory_count.rs | 209 +++++++ .../laravel/factory_count_tests.rs | 233 ++++++++ src/virtual_members/laravel/mod.rs | 2 + tests/integration/completion_laravel.rs | 545 +++++++++++++++++- 10 files changed, 1089 insertions(+), 58 deletions(-) create mode 100644 src/virtual_members/laravel/factory_count.rs create mode 100644 src/virtual_members/laravel/factory_count_tests.rs diff --git a/docs/todo.md b/docs/todo.md index 23595f6b..fcb34c06 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -185,7 +185,6 @@ unlikely to move the needle for most users. | L12 | [`HasUuids` / `HasUlids` trait — `$id` typed as `string`](todo/laravel.md#l12-hasuuids-hasulids-trait-id-typed-as-string) | Low-Medium | Low | | L6 | Factory `has*`/`for*` relationship methods | Low-Medium | Medium | | L7 | `$pivot` property on BelongsToMany | Medium | Medium-High | -| L13 | [Factory count-conditional return types](todo/laravel.md#l13-factory-count-conditional-return-types) | Medium | Medium-High | | L8 | `withSum`/`withAvg`/`withMin`/`withMax` aggregate properties | Low-Medium | Medium-High | | L45 | [`*_count` properties are offered on every relationship](todo/laravel.md#l45-_count-properties-are-offered-on-every-relationship) | Low-Medium | Medium-High | | L10 | `View::withX()` / `RedirectResponse::withX()` dynamic methods | Low | Low | diff --git a/docs/todo/laravel.md b/docs/todo/laravel.md index e112e102..fcf9a7f1 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -399,44 +399,6 @@ Alternatively, if the stubs for these traits include `@property` tags or a typed `$id` override, the PHPDoc provider may handle it automatically once the traits are loaded. -#### L13. Factory count-conditional return types - -**Impact: Medium · Effort: Medium-High** - -`User::factory()->create()` returns `User`, but -`User::factory(3)->create()` and -`User::factory()->count(3)->create()` return -`Collection`. Larastan resolves this via conditional -return type extensions that inspect whether `count()` or the -`$count` constructor argument was called with a non-null value. - -Currently PHPantom always resolves `create()` and `make()` to the -single model type. This is correct for the common case but wrong -when `count()` or `times()` has been called, or when the factory -constructor receives an integer argument. - -Larastan's `model-factories.php` has 64 assertions covering these -patterns, including `count(null)` resetting back to single-model -return. - -**Where to change:** This requires tracking call-chain state -(whether `count()`/`times()` was called) through the resolution -pipeline, which is non-trivial. Possible approaches: - -1. **Conservative:** Always return `User|Collection` - union when count state is ambiguous. Simple but noisy. -2. **Chain-aware:** Track whether `count()`/`times()` appears in - the chain before `create()`/`make()`. If yes, return - `Collection`. If no, return `User`. Requires - the resolver to inspect preceding calls in the chain. -3. **Argument-aware:** Also check `User::factory($count)` for a - literal integer argument. Most complex but most accurate. - -Approach 2 is likely the best balance. The factory virtual member -provider already knows the model type; it would need to emit -different return types for `create()`/`make()` based on whether -the chain includes a count-setting call. - #### L41. Morph aliases in `*_type` column comparisons **Impact: Low-Medium · Effort: Medium** diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index 5a89e7d7..8c6a2fc6 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -14,6 +14,7 @@ use App\Models\BlogPost; use App\Models\Review; use App\Models\ReviewCollection; +use Database\Factories\BlogAuthorFactory; use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Http\Request; use Carbon\CarbonImmutable; @@ -170,8 +171,8 @@ public function factories(): void BlogAuthor::factory()->hasProfile(); // HasOne profile → factory // The chain stays on the factory, so create() still returns the model. + // hasPosts(3) counts *related* models, not the ones being built. BlogAuthor::factory()->hasPosts(3)->create(); // → BlogAuthor - BlogAuthor::factory()->count(2)->hasPosts(3)->create(); // → BlogAuthor // for{Relationship}() — for the inverse (BelongsTo) side. BlogPost::factory()->forAuthor(['name' => 'Ada']); // BelongsTo author → factory @@ -183,6 +184,34 @@ public function factories(): void } + // ── Factory count state ───────────────────────────────────────────────── + // create()/make() build one model, or a Collection of them once the + // chain sets a count — through count(), times(), or an integer argument + // to factory(). count(null) clears it again. + + public function factoryCounts(): void + { + // No count → a single model. + BlogAuthor::factory()->create()->displayName; // → BlogAuthor + BlogAuthor::factory(['name' => 'Ada'])->make(); // array is state → BlogAuthor + + // Count set → a collection of them. BlogAuthor declares + // #[CollectedBy(AuthorCollection::class)], so that is what comes back. + BlogAuthor::factory(3)->create()->first(); // → BlogAuthor|null + BlogAuthor::factory()->count(2)->create()->emails(); // → array + BlogAuthor::factory()->times(2)->make()->byName(); // → AuthorCollection + BlogAuthorFactory::times(2)->create()->active(); // → AuthorCollection + + // The count carries past intervening calls, and count(null) clears it. + BlogAuthor::factory()->count(2)->hasPosts(3)->create()->first(); // → BlogAuthor|null + BlogAuthor::factory(3)->count(null)->create()->displayName; // → BlogAuthor + + foreach (BlogAuthor::factory(3)->create() as $author) { + $author->displayName; // → BlogAuthor + } + } + + // ── Custom Eloquent Collections ───────────────────────────────────────── public function customCollection(): void diff --git a/examples/laravel/assertions.php b/examples/laravel/assertions.php index 6cd08841..8979edcb 100644 --- a/examples/laravel/assertions.php +++ b/examples/laravel/assertions.php @@ -320,6 +320,40 @@ class_uses_recursive(\App\Models\BlogPost::class), \App\Models\BlogPost::factory()->trashed() instanceof \Illuminate\Database\Eloquent\Factories\Factory ); +// ─── Factory count-conditional return types ────────────────────────────────── + +// create()/make() hand back a single model until the chain sets a count, at +// which point they hand back a collection — the one the model itself builds, +// so BlogAuthor's #[CollectedBy(AuthorCollection::class)] applies here too. +check( + 'BlogAuthor::factory()->make() builds one model', + \App\Models\BlogAuthor::factory()->make() instanceof \App\Models\BlogAuthor +); +check( + 'an array argument to factory() is state, not a count', + \App\Models\BlogAuthor::factory(['name' => 'Ada'])->make() instanceof \App\Models\BlogAuthor +); +check( + 'BlogAuthor::factory(3)->make() builds the model collection', + \App\Models\BlogAuthor::factory(3)->make() instanceof \App\Models\AuthorCollection +); +check( + 'BlogAuthor::factory()->count(2)->make() builds two models', + \App\Models\BlogAuthor::factory()->count(2)->make()->count() === 2 +); +check( + 'BlogAuthorFactory::times(2)->make() builds the model collection', + \Database\Factories\BlogAuthorFactory::times(2)->make() instanceof \App\Models\AuthorCollection +); +check( + 'count(null) clears a count set through factory($count)', + \App\Models\BlogAuthor::factory(3)->count(null)->make() instanceof \App\Models\BlogAuthor +); +check( + 'hasPosts() counts related models, not the ones being built', + \App\Models\BlogAuthor::factory()->hasPosts(3)->make() instanceof \App\Models\BlogAuthor +); + // ─── Carbon macro closure scope binding ────────────────────────────────────── // Carbon binds macro closures with the target class as scope, so `self::` diff --git a/src/type_engine/call_resolution/return_types.rs b/src/type_engine/call_resolution/return_types.rs index a541f116..d00d3d4a 100644 --- a/src/type_engine/call_resolution/return_types.rs +++ b/src/type_engine/call_resolution/return_types.rs @@ -320,6 +320,23 @@ impl Backend { return classes; } + // Laravel factory count state: `create()`/`make()` build + // a single model, or a collection of them when the chain + // set a count (`factory(3)`, `count(3)`, `times(3)`). + if let Some((classes, hint)) = + crate::virtual_members::laravel::resolve_factory_count_return( + base, + method_name, + &lhs_resolved, + ctx, + ) + { + if let Some(ref mut hint_out) = return_type_hint_out { + **hint_out = Some(hint); + } + return classes; + } + // Laravel validated input: `validated()`, `validate([…])` // and `safe()->only([…])` return the array shape the // validation rules in scope describe. An array shape has diff --git a/src/virtual_members/laravel/factory.rs b/src/virtual_members/laravel/factory.rs index a732f7a4..f325915c 100644 --- a/src/virtual_members/laravel/factory.rs +++ b/src/virtual_members/laravel/factory.rs @@ -119,7 +119,7 @@ fn is_eloquent_factory(class_name: &str) -> bool { /// `Illuminate\Database\Eloquent\Factories\Factory`. /// /// Returns `true` if the class itself is `Factory` or any ancestor is. -fn extends_eloquent_factory( +pub(crate) fn extends_eloquent_factory( class: &ClassInfo, class_loader: &dyn Fn(&str) -> Option>, ) -> bool { @@ -135,6 +135,30 @@ fn has_factory_extends_generic(class: &ClassInfo) -> bool { }) } +/// The model type a factory class builds. +/// +/// Prefers the explicit `@extends Factory` annotation +/// that Laravel's own factory stub carries, and falls back to the naming +/// convention (e.g. `Database\Factories\UserFactory` → `App\Models\User`) +/// for factories written without it. The convention branch only answers +/// when the model class actually exists, so a `Factory` subclass that +/// happens to be named `SomethingFactory` without a matching model does +/// not invent one. +pub(crate) fn factory_model_type( + class: &ClassInfo, + class_loader: &dyn Fn(&str) -> Option>, +) -> Option { + if let Some(model) = class.extends_generics.iter().find_map(|(name, args)| { + let short = name.rsplit('\\').next().unwrap_or(name); + (short == "Factory").then(|| args.last()).flatten() + }) { + return Some(model.clone()); + } + + let model_fqn = factory_to_model_fqn(&class.name)?; + class_loader(&model_fqn).map(|_| PhpType::named(atom(model_fqn.as_ref()))) +} + /// Build virtual `create()` and `make()` methods for a factory class /// that does not have `@extends Factory`. /// @@ -144,18 +168,11 @@ fn build_factory_model_methods( class: &ClassInfo, class_loader: &dyn Fn(&str) -> Option>, ) -> Vec { - let model_fqn = match factory_to_model_fqn(&class.name) { - Some(fqn) => fqn, + let model_type = match factory_model_type(class, class_loader) { + Some(ty) => ty, None => return Vec::new(), }; - // Verify the model class actually exists. - if class_loader(&model_fqn).is_none() { - return Vec::new(); - } - - let model_type = PhpType::named(atom(model_fqn.as_ref())); - vec![ MethodInfo::virtual_method_typed("create", Some(&model_type)), MethodInfo::virtual_method_typed("make", Some(&model_type)), diff --git a/src/virtual_members/laravel/factory_count.rs b/src/virtual_members/laravel/factory_count.rs new file mode 100644 index 00000000..f7a3b5bd --- /dev/null +++ b/src/virtual_members/laravel/factory_count.rs @@ -0,0 +1,209 @@ +//! Count-conditional return types for Eloquent factory chains. +//! +//! `User::factory()->create()` builds a single `User`, but +//! `User::factory(3)->create()`, `User::factory()->count(3)->create()` and +//! `UserFactory::times(3)->create()` all build a +//! `Collection`. Laravel expresses both outcomes with one +//! `@return Collection|TModel` annotation on +//! `Factory::create()`/`make()`, which is ambiguous at every call site. +//! +//! This module reads the count state off the receiver chain and picks the +//! branch the call actually produces, mirroring Larastan's conditional +//! return type extensions. Only the syntactic chain is inspected: when a +//! factory travels through a variable (`$factory = User::factory(); …`) +//! the count state is unknown and the single-model branch is used, which +//! is the common case. + +use std::sync::Arc; + +use crate::atom::atom; +use crate::php_type::PhpType; +use crate::type_engine::conditional_resolution::split_text_args; +use crate::type_engine::resolver::ResolutionCtx; +use crate::type_engine::subject_expr::SubjectExpr; +use crate::types::{ClassInfo, ELOQUENT_COLLECTION_FQN, ResolvedType}; + +use super::factory::{extends_eloquent_factory, factory_model_type}; + +/// How many models a factory chain builds. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum FactoryCount { + /// No count was set, or a previously set count was cleared with + /// `count(null)`. + One, + /// `count()`, `times()`, or an integer `factory($count)` argument set + /// a count, so the chain builds a collection. + Many, +} + +/// Whether `name` is one of the `Factory` methods whose return type +/// depends on the chain's count state. +/// +/// `createOne()`/`makeOne()` always build one model and +/// `createMany()`/`makeMany()` always build a collection, so neither is +/// count-conditional. +pub(crate) fn is_count_conditional_method(name: &str) -> bool { + matches!(name, "create" | "createQuietly" | "make") +} + +/// Read the count state off a factory receiver chain. +/// +/// The chain is walked outermost-first, so the *last* count-setting call +/// wins — `User::factory(3)->count(null)` builds one model and +/// `User::factory()->count(2)` builds two. Calls that are not +/// count-setting (`state()`, `hasPosts()`, `trashed()`, …) are stepped +/// over, and anything that is not a call expression (a variable, a +/// property) ends the walk with [`FactoryCount::One`]. +pub(crate) fn chain_count(receiver: &SubjectExpr) -> FactoryCount { + let mut current = receiver; + let mut depth = 0u32; + + // Chains are short in practice; the bound only guards against a + // pathological expression. + while depth < 64 { + depth += 1; + let SubjectExpr::CallExpr { callee, args_text } = current else { + return FactoryCount::One; + }; + match callee.as_ref() { + SubjectExpr::MethodCall { base, method } => { + if let Some(state) = instance_count_state(method, args_text) { + return state; + } + current = base; + } + // A static call is the head of the chain: `Model::factory()`, + // `UserFactory::new()`, `UserFactory::times(3)`. + SubjectExpr::StaticMethodCall { method, .. } => { + return static_count_state(method, args_text); + } + _ => return FactoryCount::One, + } + } + + FactoryCount::One +} + +/// Count state contributed by an instance call in the chain, or `None` +/// when the call does not touch the count. +fn instance_count_state(method: &str, args_text: &str) -> Option { + match method { + // `count(?int $count)` — the only way to clear a count is to pass + // a literal `null`. A non-literal argument is assumed to be the + // integer the parameter asks for. + "count" => Some(match split_text_args(args_text).first() { + None => FactoryCount::One, + Some(arg) => { + if arg.trim().eq_ignore_ascii_case("null") { + FactoryCount::One + } else { + FactoryCount::Many + } + } + }), + // `times(int $count)` cannot be given null. + "times" => Some(FactoryCount::Many), + _ => None, + } +} + +/// Count state contributed by the static call that opens the chain. +fn static_count_state(method: &str, args_text: &str) -> FactoryCount { + match method { + // `Model::factory($count)` forwards `$count` to `count()` only + // when it is numeric; an array or callable is state, not a count. + "factory" if first_arg_is_numeric_literal(args_text) => FactoryCount::Many, + "times" => FactoryCount::Many, + _ => FactoryCount::One, + } +} + +/// Whether the first argument is a literal Laravel would consider numeric. +/// +/// Only literals count: a variable could hold an array of state just as +/// easily as an integer, and guessing wrong would turn a single model +/// into a collection. +fn first_arg_is_numeric_literal(args_text: &str) -> bool { + let Some(first) = split_text_args(args_text).first().map(|a| a.trim()) else { + return false; + }; + let unquoted = crate::text_scan::unquote_php_string(first).unwrap_or(first); + !unquoted.is_empty() && unquoted.parse::().is_ok() +} + +/// Resolve `create()` / `createQuietly()` / `make()` on an Eloquent +/// factory to the type the call-site chain actually builds. +/// +/// Returns `None` — leaving the declared return type alone — when the +/// method is not count-conditional, the receiver is not a factory, the +/// factory declares the method itself, or the model type cannot be +/// determined. +pub(crate) fn resolve_factory_count_return( + receiver: &SubjectExpr, + method_name: &str, + owners: &[ResolvedType], + ctx: &ResolutionCtx<'_>, +) -> Option<(Vec>, PhpType)> { + if !is_count_conditional_method(method_name) { + return None; + } + + let factory = owners.iter().find_map(|rt| { + rt.class_info + .as_ref() + .filter(|ci| extends_eloquent_factory(ci, ctx.class_loader)) + })?; + + // A factory that writes its own `create()`/`make()` keeps whatever it + // declared — only the signature inherited from Laravel's `Factory` + // (and the single-model stand-in PHPantom synthesizes for + // convention-based factories) is ours to reinterpret. The receiver + // may already be a merged class, so the own-member check goes through + // the loader, which hands back the class as parsed. + let fqn = factory.fqn(); + if (ctx.class_loader)(fqn.as_str()).is_some_and(|raw| raw.get_method_ci(method_name).is_some()) + { + return None; + } + + let model = factory_model_type(factory, ctx.class_loader)?; + + // The call has to resolve to *some* inherited or synthesized method; + // a factory with no `create()` at all gets no return type from us. + let merged = crate::virtual_members::resolve_class_fully_maybe_cached( + factory, + ctx.class_loader, + ctx.resolved_class_cache, + ); + merged.get_method_ci(method_name)?; + + let resolved = match chain_count(receiver) { + FactoryCount::One => model, + FactoryCount::Many => { + let collection = PhpType::generic( + ELOQUENT_COLLECTION_FQN, + vec![PhpType::named(atom("int")), model], + ); + super::replace_eloquent_collections_in_type(&collection, ctx.class_loader) + .unwrap_or(collection) + } + }; + + let classes = crate::type_engine::type_resolution::type_hint_to_classes_typed( + &resolved, + fqn.as_str(), + ctx.all_classes, + ctx.class_loader, + ); + if classes.is_empty() { + return None; + } + + Some((classes, resolved)) +} + +// ─── Tests ────────────────────────────────────────────────────────────────── + +#[cfg(test)] +#[path = "factory_count_tests.rs"] +mod tests; diff --git a/src/virtual_members/laravel/factory_count_tests.rs b/src/virtual_members/laravel/factory_count_tests.rs new file mode 100644 index 00000000..72ebc937 --- /dev/null +++ b/src/virtual_members/laravel/factory_count_tests.rs @@ -0,0 +1,233 @@ +use super::*; +use crate::test_fixtures::{make_class, no_loader}; + +/// Parse a receiver expression the way the resolver hands it to +/// [`chain_count`]: the text to the left of the final `->create()`. +fn count_of(receiver: &str) -> FactoryCount { + chain_count(&SubjectExpr::parse(receiver)) +} + +// ── is_count_conditional_method ───────────────────────────────────── + +#[test] +fn count_conditional_methods_are_create_and_make() { + assert!(is_count_conditional_method("create")); + assert!(is_count_conditional_method("createQuietly")); + assert!(is_count_conditional_method("make")); +} + +#[test] +fn one_and_many_methods_are_not_count_conditional() { + for name in [ + "createOne", + "createOneQuietly", + "createMany", + "createManyQuietly", + "makeOne", + "makeMany", + "state", + "count", + ] { + assert!( + !is_count_conditional_method(name), + "{name} should not be count-conditional" + ); + } +} + +// ── chain_count: single-model chains ──────────────────────────────── + +#[test] +fn bare_factory_call_builds_one() { + assert_eq!(count_of("User::factory()"), FactoryCount::One); +} + +#[test] +fn factory_with_state_array_builds_one() { + assert_eq!( + count_of("User::factory(['name' => 'Ada'])"), + FactoryCount::One + ); +} + +#[test] +fn factory_with_closure_builds_one() { + assert_eq!( + count_of("User::factory(fn () => ['name' => 'Ada'])"), + FactoryCount::One + ); +} + +#[test] +fn factory_with_null_builds_one() { + assert_eq!(count_of("User::factory(null)"), FactoryCount::One); +} + +#[test] +fn factory_with_variable_count_builds_one() { + // A variable could hold state just as easily as an integer, so the + // single-model branch is the safe reading. + assert_eq!(count_of("User::factory($count)"), FactoryCount::One); +} + +#[test] +fn factory_new_builds_one() { + assert_eq!(count_of("UserFactory::new()"), FactoryCount::One); +} + +#[test] +fn relationship_calls_do_not_set_a_count() { + // `hasPosts(3)` takes a count for the *relationship*, not the factory. + assert_eq!(count_of("User::factory()->hasPosts(3)"), FactoryCount::One); + assert_eq!( + count_of("User::factory()->forAuthor()->trashed()"), + FactoryCount::One + ); +} + +#[test] +fn count_null_clears_a_count() { + assert_eq!(count_of("User::factory()->count(null)"), FactoryCount::One); + assert_eq!(count_of("User::factory(3)->count(null)"), FactoryCount::One); + assert_eq!(count_of("User::factory()->count(NULL)"), FactoryCount::One); +} + +#[test] +fn count_without_arguments_builds_one() { + assert_eq!(count_of("User::factory()->count()"), FactoryCount::One); +} + +#[test] +fn non_call_receiver_builds_one() { + assert_eq!(count_of("$factory"), FactoryCount::One); + assert_eq!(count_of("$this->factory"), FactoryCount::One); +} + +// ── chain_count: collection chains ────────────────────────────────── + +#[test] +fn count_call_builds_many() { + assert_eq!(count_of("User::factory()->count(3)"), FactoryCount::Many); +} + +#[test] +fn count_with_variable_builds_many() { + // `count(?int $count)` only takes an integer or null, so a variable + // argument that is not literally `null` sets a count. + assert_eq!(count_of("User::factory()->count($n)"), FactoryCount::Many); +} + +#[test] +fn count_zero_builds_many() { + // `count(0)` yields an empty collection, not a single model. + assert_eq!(count_of("User::factory()->count(0)"), FactoryCount::Many); +} + +#[test] +fn instance_times_builds_many() { + assert_eq!(count_of("User::factory()->times(3)"), FactoryCount::Many); +} + +#[test] +fn static_times_builds_many() { + assert_eq!(count_of("UserFactory::times(3)"), FactoryCount::Many); +} + +#[test] +fn integer_factory_argument_builds_many() { + assert_eq!(count_of("User::factory(3)"), FactoryCount::Many); +} + +#[test] +fn numeric_string_factory_argument_builds_many() { + // Laravel gates on `is_numeric()`, which accepts numeric strings. + assert_eq!(count_of("User::factory('3')"), FactoryCount::Many); +} + +#[test] +fn count_survives_later_non_count_calls() { + assert_eq!( + count_of("User::factory()->count(3)->hasPosts(2)->trashed()"), + FactoryCount::Many + ); +} + +#[test] +fn last_count_call_wins() { + assert_eq!( + count_of("User::factory()->count(null)->count(2)"), + FactoryCount::Many + ); + assert_eq!( + count_of("User::factory()->count(2)->count(null)"), + FactoryCount::One + ); +} + +#[test] +fn count_on_a_new_factory_instance_builds_many() { + assert_eq!(count_of("new UserFactory()->count(3)"), FactoryCount::Many); +} + +#[test] +fn a_new_expression_head_without_a_count_builds_one() { + assert_eq!(count_of("new UserFactory()->state([])"), FactoryCount::One); +} + +// ── factory_model_type ────────────────────────────────────────────── + +#[test] +fn model_type_prefers_extends_generic() { + let mut factory = make_class("UserFactory"); + factory.file_namespace = Some(atom("Database\\Factories")); + factory.extends_generics = vec![( + atom("Illuminate\\Database\\Eloquent\\Factories\\Factory"), + vec![PhpType::named(atom("App\\Domain\\Person"))], + )]; + + assert_eq!( + factory_model_type(&factory, &no_loader).map(|t| t.to_string()), + Some("App\\Domain\\Person".to_string()), + "the @extends annotation names the model outright" + ); +} + +#[test] +fn model_type_falls_back_to_the_naming_convention() { + let mut factory = make_class("UserFactory"); + factory.file_namespace = Some(atom("Database\\Factories")); + + let model = Arc::new(make_class("User")); + let loader = move |name: &str| -> Option> { + (name == "App\\Models\\User").then(|| Arc::clone(&model)) + }; + + assert_eq!( + factory_model_type(&factory, &loader).map(|t| t.to_string()), + Some("App\\Models\\User".to_string()) + ); +} + +#[test] +fn model_type_is_none_when_the_conventional_model_is_missing() { + let mut factory = make_class("WidgetFactory"); + factory.file_namespace = Some(atom("Database\\Factories")); + + assert_eq!(factory_model_type(&factory, &no_loader), None); +} + +#[test] +fn model_type_ignores_generics_for_other_parents() { + let mut factory = make_class("UserFactory"); + factory.file_namespace = Some(atom("Database\\Factories")); + factory.extends_generics = vec![( + atom("Illuminate\\Support\\Collection"), + vec![PhpType::named(atom("App\\Models\\Other"))], + )]; + + assert_eq!( + factory_model_type(&factory, &no_loader), + None, + "only an @extends Factory<…> annotation names the model" + ); +} diff --git a/src/virtual_members/laravel/mod.rs b/src/virtual_members/laravel/mod.rs index 5e3bb9b7..34ad2401 100644 --- a/src/virtual_members/laravel/mod.rs +++ b/src/virtual_members/laravel/mod.rs @@ -88,6 +88,7 @@ pub(crate) mod config_values; pub(crate) mod database_schema; mod env_vars; mod factory; +pub(crate) mod factory_count; mod helpers; mod higher_order_proxy; mod macros; @@ -178,6 +179,7 @@ pub use factory::LaravelFactoryProvider; pub(crate) use factory::{ factory_to_model_fqn, is_factory_class, is_has_factory_trait, model_to_factory_fqn, }; +pub(crate) use factory_count::resolve_factory_count_return; use crate::atom::{AtomSet, ascii_lowercase_atom}; use crate::php_type::{PhpType, TypeKind}; diff --git a/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index 7f368645..ebfef532 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -368,7 +368,7 @@ namespace Illuminate\\Database\\Eloquent\\Factories; */ trait HasFactory { /** @return TFactory */ - public static function factory() {} + public static function factory($count = null, $state = []) {} } "; @@ -379,12 +379,16 @@ namespace Illuminate\\Database\\Eloquent\\Factories; * @template TModel of \\Illuminate\\Database\\Eloquent\\Model */ class Factory { - /** @return TModel */ + /** @return \\Illuminate\\Database\\Eloquent\\Collection|TModel */ public function create(array $attributes = []) {} - /** @return TModel */ + /** @return \\Illuminate\\Database\\Eloquent\\Collection|TModel */ public function make(array $attributes = []) {} /** @return static */ - public function count(int $count): static { return $this; } + public static function new(array $attributes = []): static {} + /** @return static */ + public static function times(int $count): static {} + /** @return static */ + public function count(?int $count): static { return $this; } /** @return static */ public function state(array $state): static { return $this; } } @@ -8217,34 +8221,350 @@ class UserFactory extends Factory { ); } +// ─── Factory count-conditional return types ───────────────────────────────── + +/// A model + convention-based factory pair, shared by the +/// count-conditional tests below. +const COUNT_USER_PHP: &str = "\ +->` in a scratch file that already imports +/// `App\Models\User`, against the shared model/factory pair. +async fn complete_after_factory_chain(expr: &str) -> Vec { + let (backend, dir) = make_workspace(&[ + ("src/Models/User.php", COUNT_USER_PHP), + ("database/factories/UserFactory.php", COUNT_USER_FACTORY_PHP), + ]); + + let line = format!("{expr}->"); + let content = + format!("count(3)->create()").await; +} + +#[tokio::test] +async fn test_factory_count_then_make_returns_collection() { + assert_builds_many("User::factory()->count(3)->make()").await; +} + #[tokio::test] -async fn test_factory_convention_based_chain_count_then_create() { +async fn test_factory_integer_argument_returns_collection() { + assert_builds_many("User::factory(3)->create()").await; +} + +#[tokio::test] +async fn test_factory_instance_times_returns_collection() { + assert_builds_many("User::factory()->times(3)->create()").await; +} + +#[tokio::test] +async fn test_factory_static_times_returns_collection() { + assert_builds_many("UserFactory::times(3)->create()").await; +} + +#[tokio::test] +async fn test_factory_count_survives_intervening_state_calls() { + assert_builds_many("User::factory()->count(3)->state([])->create()").await; +} + +#[tokio::test] +async fn test_factory_count_null_resets_to_single_model() { + assert_builds_one("User::factory()->count(null)->create()").await; + assert_builds_one("User::factory(3)->count(null)->create()").await; +} + +#[tokio::test] +async fn test_factory_state_argument_is_not_a_count() { + assert_builds_one("User::factory(['name' => 'Ada'])->create()").await; +} + +#[tokio::test] +async fn test_factory_new_without_count_builds_single_model() { + assert_builds_one("UserFactory::new()->create()").await; +} + +/// `hasPosts(3)` counts related models, not the models the factory +/// itself builds, so it must not flip the return type to a collection. +#[tokio::test] +async fn test_factory_relationship_count_is_not_a_factory_count() { + let post_php = "\ + */ + public function posts(): HasMany { return $this->hasMany(Post::class); } } "; - let factory_php = "\ + let (backend, dir) = make_workspace(&[ + ("src/Models/Post.php", post_php), + ("src/Models/User.php", user_php), + ("database/factories/UserFactory.php", COUNT_USER_FACTORY_PHP), + ]); + + let items = complete_at( + &backend, + &dir, + "src/test.php", + "hasPosts(3)->create()->\n", + 2, + 41, + ) + .await; + + let methods = method_names(&items); + assert!( + methods.contains(&"greet"), + "hasPosts(3) should not turn create() into a collection, got: {:?}", + methods + ); +} + +/// The count state reaches the assigned variable, and indexing back +/// through the collection recovers the model. +#[tokio::test] +async fn test_factory_collection_assignment_and_first() { + let methods = + complete_after_factory_chain("User::factory()->count(3)->create()->first()").await; + assert!( + methods.iter().any(|m| m == "greet"), + "first() on the created collection should yield a User, got: {methods:?}" + ); +} + +#[tokio::test] +async fn test_factory_collection_survives_variable_assignment() { + let (backend, dir) = make_workspace(&[ + ("src/Models/User.php", COUNT_USER_PHP), + ("database/factories/UserFactory.php", COUNT_USER_FACTORY_PHP), + ]); + + let items = complete_at( + &backend, + &dir, + "src/test.php", + "count(3)->create();\n$users->\n", + 3, + 8, + ) + .await; + + let methods = method_names(&items); + assert!( + methods.contains(&"all"), + "the assigned variable should hold a Collection, got: {:?}", + methods + ); +} + +/// A factory written with Laravel's own `@extends Factory` +/// annotation resolves the same way as a convention-based one — and the +/// ambiguous `Collection|TModel` union is narrowed to the +/// branch the chain actually produces. +#[tokio::test] +async fn test_factory_extends_generic_narrows_ambiguous_union() { + let annotated_factory_php = "\ + */ class UserFactory extends Factory { public function definition(): array { return []; } } "; let (backend, dir) = make_workspace(&[ + ("src/Models/User.php", COUNT_USER_PHP), + ("database/factories/UserFactory.php", annotated_factory_php), + ]); + + let single = complete_at( + &backend, + &dir, + "src/test.php", + "create()->\n", + 2, + 28, + ) + .await; + let single = method_names(&single); + assert!( + single.contains(&"greet") && !single.contains(&"all"), + "an annotated factory without a count should build one User, got: {:?}", + single + ); + + let many = complete_at( + &backend, + &dir, + "src/test2.php", + "count(2)->create()->\n", + 2, + 38, + ) + .await; + let many = method_names(&many); + assert!( + many.contains(&"all") && !many.contains(&"greet"), + "an annotated factory with a count should build a Collection, got: {:?}", + many + ); +} + +/// A model with a custom Eloquent collection gets that collection back, +/// not the base `Illuminate\Database\Eloquent\Collection`. +#[tokio::test] +async fn test_factory_count_returns_custom_collection() { + let collection_php = "\ + + */ +class UserCollection extends Collection { + /** @return array */ + public function verified(): array { return []; } +} +"; + let user_php = "\ +count(3)->create()->\n", + 2, + 38, + ) + .await; + + let methods = method_names(&items); + assert!( + methods.contains(&"verified"), + "the model's custom collection should be used, got: {:?}", + methods + ); +} + +/// A factory that overrides `create()` itself keeps the type it declares, +/// count state or not. +#[tokio::test] +async fn test_factory_own_create_override_wins() { + let factory_php = "\ +count(3)->create() should still resolve to User let items = complete_at( &backend, &dir, @@ -8258,7 +8578,216 @@ class UserFactory extends Factory { let methods = method_names(&items); assert!( methods.contains(&"greet"), - "count()->create() chain should resolve back to User, got methods: {:?}", + "an overridden create() should keep its declared return type, got: {:?}", + methods + ); +} + +/// A `Factory` subclass with no matching model must not have its +/// `create()` reinterpreted — there is no model type to build from. +#[tokio::test] +async fn test_factory_without_a_model_is_left_alone() { + let factory_php = "\ +count(3)->\n", + 2, + 32, + ) + .await; + + let methods = method_names(&items); + assert!( + methods.contains(&"describe"), + "a model-less factory should still chain on itself, got: {:?}", + methods + ); +} + +/// `create()` on a class that has nothing to do with Eloquent factories +/// resolves from its own declaration, count-looking chain or not. +#[tokio::test] +async fn test_non_factory_create_is_untouched() { + let thing_php = "\ +count(3)->create()->\n", + 3, + 24, + ) + .await; + + let methods = method_names(&items); + assert!( + methods.contains(&"label"), + "a non-factory create() should keep its declared return type, got: {:?}", + methods + ); +} + +/// Without a `Factory` base class that declares `create()` there is no +/// call to type, so a count in the chain must not conjure a return type. +#[tokio::test] +async fn test_factory_without_an_inherited_create_resolves_nothing() { + let bare_factory_base_php = "\ + + */ +class UserFactory extends Factory { + public function definition(): array { return []; } +} +"; + + let (backend, dir) = create_psr4_workspace( + COMPOSER_JSON, + &[ + ( + "vendor/illuminate/Eloquent/Factories/Factory.php", + bare_factory_base_php, + ), + ("src/Models/User.php", user_php), + ("database/factories/UserFactory.php", factory_php), + ], + ); + + let items = complete_at( + &backend, + &dir, + "src/test.php", + "count(3)->create()->\n", + 3, + 24, + ) + .await; + + let methods = method_names(&items); + assert!( + !methods.contains(&"greet"), + "a create() that does not exist should resolve to nothing, got: {:?}", + methods + ); +} + +/// Without an Eloquent `Collection` class to resolve to, a counted chain +/// falls back to the single-model type rather than resolving to nothing. +#[tokio::test] +async fn test_factory_count_without_a_collection_class_falls_back() { + let model_php = "\ +count(3)->create()->\n", + 3, + 24, + ) + .await; + + let methods = method_names(&items); + assert!( + methods.contains(&"greet"), + "a missing Collection class should leave the model type in place, got: {:?}", methods ); } From c6fd06cd2795ee9d9a0f783aa5a3f282e83792e5 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Sun, 2 Aug 2026 12:33:08 +0600 Subject: [PATCH 2/3] chore: update changelog md file --- docs/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index a160ec51..9c14d8f7 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Eloquent morph map aliases.** A `Relation::morphMap(['post' => Post::class, …])` or `Relation::enforceMorphMap([…])` call in a service provider is now recovered, so the short aliases it registers behave like real symbols wherever they appear as string literals. Hover names the model an alias maps to and the file that registers it, go-to-definition offers both the registration and the model, and find-references links every usage back to the registration. The alias positions Eloquent actually resolves through the map are recognized: the registration's own keys, `Relation::getMorphedModel()`, `Model::getActualClassNameForMorph()`, and the `$types` argument of the `whereHasMorph()` family (including the `'*'` wildcard and class-name spellings, which are left alone). Laravel's list shorthand `Relation::morphMap([Post::class, …])` is understood too, keyed by each model's table name the way the framework derives it. When the project calls `enforceMorphMap()` (or `requireMorphMap()`) the map is the exhaustive set of morph types, so an alias that is not registered is flagged; without it the set stays open and nothing is reported. - **Workspace-wide diagnostics.** PHPantom now surfaces problems across the whole project, not just the files you have open. After startup and the full background index finish, diagnostics run in the background over every file and stream into the editor's problems panel as they're found, so issues in files you haven't opened yet are already visible when you navigate to them. Configured tools (PHPStan, PHPCS, Mago) also run once over the whole project afterwards, when the project has its own configuration file for that tool. Both passes are deliberately deferred until after startup so they never slow down the time it takes for the editor to become usable. Disable with `[diagnostics] workspace = false` (native pass) or `workspace-external = false` (external tools) in `.phpantom.toml`. - **Higher-order collection proxies.** `$users->map->email` is Laravel shorthand for `$users->map(fn ($u) => $u->email)`, and PHPantom now types it that way: the item type's members complete and hover through the proxy, and each one resolves to whatever the proxied collection method returns for it — `map` collects the member, `filter` and `each` keep the collection, `first` gives you one nullable item, `contains` a `bool`, `sum` a number. The result is an ordinary collection, so the chain continues from it. The proxy remembers which collection it came from, so a method that returns `static` stays on an Eloquent or application-defined collection, while mapping to a value that is not a model falls back to the base collection exactly as Eloquent does at runtime. Contributed by @shuvroroy (#314). +- **Model factories know how many models they build.** `User::factory()->create()` gives you a `User`, but the moment the chain sets a count it gives you a collection of them, and PHPantom now reads that off the call site. `count(3)`, `times(3)`, and an integer argument to `factory(3)` all switch `create()`, `createQuietly()`, and `make()` over to the collection the model actually builds — a custom Eloquent collection included — so the members that complete, hover, and resolve are the collection's, and iterating it or calling `first()` gets you back to the model. A `count(null)` clears the count again, and calls that pass a count for something else, like `hasPosts(3)`, leave the return type alone. A factory that writes its own `create()` keeps whatever it declares. Contributed by @shuvroroy (#315). - **Enum validation rules type their field.** A `validated()` array shape used to give up on an enum rule and call the field `mixed`, because the rule is an object (`new Enum(Role::class)`, `Rule::enum(Role::class)`) rather than a rule name. The field now takes the enum's backing type (`string` for a string-backed enum, `int` for an int-backed one), which is what the validated array actually holds, since it carries the raw input rather than the enum case. The enum is read the same way whichever spelling it has, including a fluent chain like `Rule::enum(Role::class)->only([…])`, and its class name is resolved against the imports of the file that declares the rules, so a `FormRequest`'s own `use` statements are what count. A pure enum has no raw scalar form, so those fields stay `mixed` rather than being guessed at. Contributed by @shuvroroy (#307). ### Changed From 5f12d2979d0ab782e26acafc92aa843b7bbcbbc5 Mon Sep 17 00:00:00 2001 From: Shuvro Roy Date: Sun, 2 Aug 2026 12:53:42 +0600 Subject: [PATCH 3/3] chore: test coverage improvement --- .../laravel/factory_count_tests.rs | 34 +++++++++++++++++++ tests/integration/completion_laravel.rs | 18 ++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/virtual_members/laravel/factory_count_tests.rs b/src/virtual_members/laravel/factory_count_tests.rs index 72ebc937..d936280a 100644 --- a/src/virtual_members/laravel/factory_count_tests.rs +++ b/src/virtual_members/laravel/factory_count_tests.rs @@ -174,6 +174,40 @@ fn a_new_expression_head_without_a_count_builds_one() { assert_eq!(count_of("new UserFactory()->state([])"), FactoryCount::One); } +/// The chain head can be something other than a static call — the legacy +/// global `factory()` helper, or an invoked callable. Neither says +/// anything about the count, whatever it was handed. +#[test] +fn a_non_static_chain_head_builds_one() { + assert_eq!(count_of("factory(3)"), FactoryCount::One); + assert_eq!(count_of("$makeFactory()"), FactoryCount::One); + assert_eq!(count_of("$makeFactory()->state([])"), FactoryCount::One); +} + +/// A chain long enough to hit the walk's depth bound gives up rather +/// than scanning forever, even though a count is set further down. +#[test] +fn a_pathological_chain_gives_up_on_the_count() { + let counted = |links: usize| { + let mut expr = String::from("User::factory()->count(3)"); + for _ in 0..links { + expr.push_str("->state([])"); + } + count_of(&expr) + }; + + assert_eq!( + counted(10), + FactoryCount::Many, + "an ordinary chain still finds the count" + ); + assert_eq!( + counted(200), + FactoryCount::One, + "past the bound the walk stops looking" + ); +} + // ── factory_model_type ────────────────────────────────────────────── #[test] diff --git a/tests/integration/completion_laravel.rs b/tests/integration/completion_laravel.rs index ebfef532..fd4486ef 100644 --- a/tests/integration/completion_laravel.rs +++ b/tests/integration/completion_laravel.rs @@ -8618,6 +8618,24 @@ class WidgetFactory extends Factory { "a model-less factory should still chain on itself, got: {:?}", methods ); + + // And a counted create() on it must not borrow some other model. + let created = complete_at( + &backend, + &dir, + "src/test2.php", + "count(3)->create()->\n", + 2, + 42, + ) + .await; + + let created = method_names(&created); + assert!( + !created.contains(&"greet"), + "a model-less factory must not resolve create() to another model, got: {:?}", + created + ); } /// `create()` on a class that has nothing to do with Eloquent factories