diff --git a/.roave-backward-compatibility-check.xml b/.roave-backward-compatibility-check.xml index d86f7d3a5..e8d21ed3f 100644 --- a/.roave-backward-compatibility-check.xml +++ b/.roave-backward-compatibility-check.xml @@ -11,5 +11,7 @@ #\[BC\] CHANGED: Method getIdentifierValues\(\) of class Zenstruck\\Foundry\\Persistence\\PersistenceStrategy became final# #"Zenstruck\\Foundry\\Test\\(CommonResetDatabase|CommonFactories)" could not be found in the located source(.*)# #Method Zenstruck\\Foundry\\ORM\\AbstractORMPersistenceStrategy\#withoutDoctrineEvents\(\) was removed# + #Method Zenstruck\\Foundry\\Factory::create(One|Many|Range|Sequence)\(\) was removed# + #Method Zenstruck\\Foundry\\Persistence\\PersistentProxyObjectFactory::createOne\(\) was removed# diff --git a/src/Factory.php b/src/Factory.php index 3a8b4eb35..1c7325f1e 100644 --- a/src/Factory.php +++ b/src/Factory.php @@ -26,9 +26,31 @@ * @phpstan-type Parameters = array * @phpstan-type Attributes = Parameters|callable(int):Parameters * @phpstan-type Sequence = iterable|callable(): iterable + * + * The create helpers go through __callStatic(). One block per reader: PHPStorm and PHPStan get + * the real signatures, Psalm gets plain ones because it reads the aliases above as class names + * inside a "@method" tag, and inlining them makes it drop the tags entirely. Its return types + * come from FixCreateHelpersReturnType anyway. + * + * @psalm-method static T createOne(array|callable $attributes = []) + * @psalm-method static list createMany(int $number, array|callable $attributes = []) + * @psalm-method static list createRange(int $min, int $max, array|callable $attributes = []) + * @psalm-method static list createSequence(iterable|callable $sequence) + * + * @method static T createOne(Attributes $attributes = []) + * @method static ($number is positive-int ? non-empty-list : list) createMany(int $number, Attributes $attributes = []) + * @method static ($min is positive-int ? non-empty-list : list) createRange(int $min, int $max, Attributes $attributes = []) + * @method static list createSequence(Sequence $sequence) + * + * @phpstan-method static T createOne(Attributes $attributes = []) + * @phpstan-method static ($number is positive-int ? non-empty-list : list) createMany(int $number, Attributes $attributes = []) + * @phpstan-method static ($min is positive-int ? non-empty-list : list) createRange(int $min, int $max, Attributes $attributes = []) + * @phpstan-method static list createSequence(Sequence $sequence) */ abstract class Factory { + private const CREATE_HELPERS = ['createOne', 'createMany', 'createRange', 'createSequence']; + /** @phpstan-var Attributes[] */ private array $attributes = []; @@ -68,7 +90,7 @@ final public static function new(array|callable $attributes = []): static * * @return T */ - public static function createOne(array|callable $attributes = []): mixed + private static function doCreateOne(array|callable $attributes = []): mixed { return static::new()->create($attributes); } @@ -81,7 +103,7 @@ public static function createOne(array|callable $attributes = []): mixed * @return list * @phpstan-return ($number is positive-int ? non-empty-list : list) */ - final public static function createMany(int $number, array|callable $attributes = []): array + private static function doCreateMany(int $number, array|callable $attributes = []): array { return static::new()->many($number)->create($attributes); } @@ -94,7 +116,7 @@ final public static function createMany(int $number, array|callable $attributes * @return list * @phpstan-return ($min is positive-int ? non-empty-list : list) */ - final public static function createRange(int $min, int $max, array|callable $attributes = []): array + private static function doCreateRange(int $min, int $max, array|callable $attributes = []): array { return static::new()->range($min, $max)->create($attributes); } @@ -104,11 +126,58 @@ final public static function createRange(int $min, int $max, array|callable $att * * @return list */ - final public static function createSequence(iterable|callable $sequence): array + private static function doCreateSequence(iterable|callable $sequence): array { return static::new()->sequence($sequence)->create(); } + /** + * @param array $arguments + */ + final public static function __callStatic(string $name, array $arguments): mixed + { + if (!\in_array($name, self::CREATE_HELPERS, true)) { + throw new \BadMethodCallException(\sprintf('Call to undefined method %s::%s().', static::class, $name)); + } + + return static::{'do'.\ucfirst($name)}(...$arguments); + } + + /** + * The create helpers are static, so calling one on an instance used to silently discard + * everything set on that instance: `Factory::new()->someState()->createMany(2)` created + * two objects without the state. Routing them through __callStatic makes the instance + * call land here instead, where the state can be honored. + * + * @param array $arguments + */ + final public function __call(string $name, array $arguments): mixed + { + if (!\in_array($name, self::CREATE_HELPERS, true)) { + throw new \BadMethodCallException(\sprintf('Call to undefined method %s::%s().', static::class, $name)); + } + + trigger_deprecation('zenstruck/foundry', '2.13', 'Calling "%s()" on a factory instance is deprecated and will throw in Foundry 3. Use "%s" instead.', $name, self::instanceEquivalent($name)); + + return match ($name) { + 'createOne' => $this->create(...$arguments), + 'createMany' => $this->many($arguments[0])->create($arguments[1] ?? []), + 'createRange' => $this->range($arguments[0], $arguments[1])->create($arguments[2] ?? []), + 'createSequence' => $this->sequence($arguments[0])->create(), + }; + } + + private static function instanceEquivalent(string $name): string + { + return match ($name) { + 'createOne' => 'create()', + 'createMany' => 'many()->create()', + 'createRange' => 'range()->create()', + 'createSequence' => 'sequence()->create()', + default => throw new \LogicException(\sprintf('Unhandled create helper "%s".', $name)), + }; + } + /** * @param array|callable $attributes Property values for the object(s) to create * diff --git a/src/Persistence/PersistentProxyObjectFactory.php b/src/Persistence/PersistentProxyObjectFactory.php index 2da446a60..084278656 100644 --- a/src/Persistence/PersistentProxyObjectFactory.php +++ b/src/Persistence/PersistentProxyObjectFactory.php @@ -47,15 +47,6 @@ final public function create(callable|array $attributes = []): object return proxy(parent::create($attributes)); // @phpstan-ignore function.unresolvableReturnType } - /** - * @return T|Proxy - * @phpstan-return T&Proxy - */ - final public static function createOne(array|callable $attributes = []): mixed - { - return proxy(parent::createOne($attributes)); // @phpstan-ignore function.unresolvableReturnType - } - /** * @return T|Proxy * @phpstan-return T&Proxy diff --git a/tests/Integration/Persistence/GenericFactoryTestCase.php b/tests/Integration/Persistence/GenericFactoryTestCase.php index 54b5a87e3..f8ce69819 100644 --- a/tests/Integration/Persistence/GenericFactoryTestCase.php +++ b/tests/Integration/Persistence/GenericFactoryTestCase.php @@ -196,7 +196,7 @@ public function repository_and_create_function(): void #[Test] public function create_many(): void { - $models = static::factory()->createMany(3, static fn(int $i) => ['prop1' => "value{$i}"]); + $models = static::factory()->many(3)->create(static fn(int $i) => ['prop1' => "value{$i}"]); static::factory()::repository()->assert()->count(3); diff --git a/tests/Unit/ObjectFactoryTest.php b/tests/Unit/ObjectFactoryTest.php index 629a499f9..51ba2a801 100644 --- a/tests/Unit/ObjectFactoryTest.php +++ b/tests/Unit/ObjectFactoryTest.php @@ -12,6 +12,8 @@ namespace Zenstruck\Foundry\Tests\Unit; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\IgnoreDeprecations; +use PHPUnit\Framework\Attributes\RequiresPhpunit; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; use Zenstruck\Foundry\Factory; @@ -525,4 +527,73 @@ public function as_data_provider(): void { $this->markTestIncomplete(); } + + /** + * @test + * + * @group legacy + * @requires PHPUnit >=11.0.0 + */ + #[Test] + #[IgnoreDeprecations] + #[RequiresPhpunit('>=11.0.0')] + public function create_helpers_called_on_an_instance_keep_the_state(): void + { + $factory = SimpleObjectFactory::new()->with(['prop1' => 'from-state']); + + self::assertSame('from-state', $factory->createOne()->prop1); + self::assertSame(['from-state', 'from-state'], \array_column($factory->createMany(2), 'prop1')); + self::assertSame(['from-state', 'from-state'], \array_column($factory->createRange(2, 2), 'prop1')); + self::assertSame(['from-state'], \array_column($factory->createSequence([[]]), 'prop1')); + } + + /** + * @test + * + * @group legacy + * @requires PHPUnit >=11.0.0 + */ + #[Test] + #[IgnoreDeprecations] + #[RequiresPhpunit('>=11.0.0')] + public function create_helpers_called_on_an_instance_are_deprecated(): void + { + $this->expectUserDeprecationMessageMatches('/Calling "createMany\(\)" on a factory instance is deprecated and will throw in Foundry 3\. Use "many\(\)->create\(\)" instead\./'); + + SimpleObjectFactory::new()->createMany(1); + } + + /** + * @test + */ + #[Test] + public function create_helpers_still_ignore_nothing_when_called_statically(): void + { + self::assertCount(2, SimpleObjectFactory::createMany(2, ['prop1' => 'static'])); + self::assertSame('static', SimpleObjectFactory::createOne(['prop1' => 'static'])->prop1); + } + + /** + * @test + */ + #[Test] + public function calling_an_undefined_method_statically_throws(): void + { + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage(\sprintf('Call to undefined method %s::doesNotExist().', SimpleObjectFactory::class)); + + SimpleObjectFactory::doesNotExist(); // @phpstan-ignore staticMethod.notFound + } + + /** + * @test + */ + #[Test] + public function calling_an_undefined_method_on_an_instance_throws(): void + { + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage(\sprintf('Call to undefined method %s::doesNotExist().', SimpleObjectFactory::class)); + + SimpleObjectFactory::new()->doesNotExist(); // @phpstan-ignore method.notFound + } } diff --git a/utils/psalm/FixCreateHelpersReturnType.php b/utils/psalm/FixCreateHelpersReturnType.php new file mode 100644 index 000000000..ebf471759 --- /dev/null +++ b/utils/psalm/FixCreateHelpersReturnType.php @@ -0,0 +1,140 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Zenstruck\Foundry\Psalm; + +use PhpParser\Node\Expr; +use PhpParser\Node\Expr\StaticCall; +use PhpParser\Node\Identifier; +use PhpParser\Node\Name; +use Psalm\Internal\Analyzer\ClassLikeAnalyzer; +use Psalm\Plugin\EventHandler\AfterExpressionAnalysisInterface; +use Psalm\Plugin\EventHandler\Event\AfterExpressionAnalysisEvent; +use Psalm\StatementsSource; +use Psalm\Type; +use Psalm\Type\Atomic\TIntRange; +use Psalm\Type\Atomic\TLiteralInt; +use Zenstruck\Foundry\Factory; +use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\Proxy; + +/** + * The create helpers are declared with "@method static" tags on Factory, and Psalm does not + * bind the class template when it resolves a pseudo method: it expands the return type with + * the declaring class as "self", so "T" stays "T:Zenstruck\Foundry\Factory as mixed". + * + * The return type is therefore rebuilt here, once the call has been analyzed. This also + * covers the proxy factories, which FixProxyFactoryMethodsReturnType cannot reach: it hooks + * AfterMethodCallAnalysisInterface, and that never fires for a magic static call. + */ +final class FixCreateHelpersReturnType implements AfterExpressionAnalysisInterface +{ + public static function afterExpressionAnalysis(AfterExpressionAnalysisEvent $event): ?bool + { + $expr = $event->getExpr(); + + if (!$expr instanceof StaticCall + || !$expr->name instanceof Identifier + || !$expr->class instanceof Name + ) { + return null; + } + + $method = \strtolower($expr->name->name); + + if (!\in_array($method, ['createone', 'createmany', 'createrange', 'createsequence'], true)) { + return null; + } + + $source = $event->getStatementsSource(); + $class = ClassLikeAnalyzer::getFQCLNFromNameObject($expr->class, $source->getAliases()); + $codebase = $event->getCodebase(); + + if (!$codebase->classExtends($class, Factory::class)) { + return null; + } + + $storage = $codebase->classlikes->getStorageFor($class); + + if (!$storage) { + return null; + } + + $isProxy = $codebase->classExtends($class, PersistentProxyObjectFactory::class); + $templateType = $storage->template_extended_params[$isProxy ? PersistentProxyObjectFactory::class : Factory::class]['T'] ?? null; + + if (!$templateType) { + return null; + } + + $type = $templateType->getId(); + + if ($isProxy) { + $proxyClass = Proxy::class; + $type = "{$type}&{$proxyClass}<{$type}>"; + } + + $source->getNodeTypeProvider()->setType( + $expr, + Type::parseString(self::returnType($method, $type, $expr, $source, $isProxy)), + ); + + return null; + } + + /** + * The list helpers are declared with a conditional return type, which Psalm does not + * evaluate for a pseudo method either, so the argument is read here. Proxy factories + * never had the conditional type, FixProxyFactoryMethodsReturnType always widened them + * to a plain list, and that is kept. + */ + private static function returnType( + string $method, + string $type, + StaticCall $expr, + StatementsSource $source, + bool $isProxy, + ): string { + if ('createone' === $method) { + return $type; + } + + if ('createsequence' === $method || $isProxy) { + return "list<{$type}>"; + } + + // createMany() is driven by its first argument, createRange() by its lowest bound + $argument = $expr->getArgs()[0]->value ?? null; + + return $argument && self::isPositiveInt($argument, $source) + ? "non-empty-list<{$type}>" + : "list<{$type}>"; + } + + private static function isPositiveInt(Expr $expr, StatementsSource $source): bool + { + $type = $source->getNodeTypeProvider()->getType($expr); + + if (!$type || !$type->isSingle()) { + return false; + } + + $atomic = $type->getSingleAtomic(); + + if ($atomic instanceof TLiteralInt) { + return $atomic->value > 0; + } + + return $atomic instanceof TIntRange && null !== $atomic->min_bound && $atomic->min_bound > 0; + } +} diff --git a/utils/psalm/FoundryPlugin.php b/utils/psalm/FoundryPlugin.php index 5dc972c69..1c6463773 100644 --- a/utils/psalm/FoundryPlugin.php +++ b/utils/psalm/FoundryPlugin.php @@ -20,5 +20,8 @@ public function __invoke(RegistrationInterface $registration, ?\SimpleXMLElement { \class_exists(FixProxyFactoryMethodsReturnType::class, true); $registration->registerHooksFromClass(FixProxyFactoryMethodsReturnType::class); + + \class_exists(FixCreateHelpersReturnType::class, true); + $registration->registerHooksFromClass(FixCreateHelpersReturnType::class); } }