Skip to content

Commit

Permalink
refactor: deprecate config instantiator.without_constructor (#522)
Browse files Browse the repository at this point in the history
  • Loading branch information
nikophil authored Nov 24, 2023
1 parent a45d631 commit 563f4da
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 14 deletions.
11 changes: 8 additions & 3 deletions src/Bundle/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ public function getConfigTreeBuilder(): TreeBuilder
->addDefaultsIfNotSet()
->info('Configure the default instantiator used by your factories.')
->validate()
->ifTrue(static fn(array $v) => $v['service'] && $v['without_constructor'])
->thenInvalid('Cannot set "without_constructor" when using custom service.')
->ifTrue(static fn(array $v) => $v['service'] && (($v['without_constructor'] ?? false) || !($v['use_constructor'] ?? true)))
->thenInvalid('Cannot set "use_constructor: false" when using custom service.')
->end()
->validate()
->ifTrue(static fn(array $v) => $v['service'] && $v['allow_extra_attributes'])
Expand All @@ -76,8 +76,13 @@ public function getConfigTreeBuilder(): TreeBuilder
->end()
->children()
->booleanNode('without_constructor')
->defaultFalse()
->info('Whether or not to call an object\'s constructor during instantiation.')
->setDeprecated('zenstruck/foundry', '1.37.0', 'Configuration "zenstruck_foundry.instantiator.without_constructor" is deprecated and will be removed in 2.0. Use "zenstruck_foundry.instantiator.use_constructor" instead.')
->end()
->booleanNode('use_constructor')
->info('Use the constructor to instantiate objects.')
// default is set in ZenstruckFoundryExtension till we have deprecation layer
//->defaultTrue()
->end()
->booleanNode('allow_extra_attributes')
->defaultFalse()
Expand Down
11 changes: 9 additions & 2 deletions src/Bundle/DependencyInjection/ZenstruckFoundryExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,17 @@ private function configureDefaultInstantiator(array $config, ContainerBuilder $c

$definition = $container->getDefinition('.zenstruck_foundry.default_instantiator');

if ($config['without_constructor']) {
$definition->setFactory([Instantiator::class, 'withoutConstructor']);
if (isset($config['without_constructor'])
&& isset($config['use_constructor'])
&& $config['without_constructor'] === $config['use_constructor']
) {
throw new \InvalidArgumentException('Cannot set "without_constructor" and "use_constructor" to the same value.');
}

$withoutConstructor = $config['without_constructor'] ?? !($config['use_constructor'] ?? true);

$definition->setFactory([Instantiator::class, $withoutConstructor ? 'withoutConstructor' : 'withConstructor']);

if ($config['allow_extra_attributes']) {
$definition->addMethodCall('allowExtraAttributes');
}
Expand Down
10 changes: 5 additions & 5 deletions src/Instantiator.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ final class Instantiator
{
private static ?PropertyAccessor $propertyAccessor = null;

private bool $withoutConstructor = false;
private bool $useConstructor = true;

private bool $allowExtraAttributes = false;

Expand Down Expand Up @@ -108,7 +108,7 @@ public function __call(string $name, array $arguments): self

trigger_deprecation('zenstruck/foundry', '1.37.0', 'Calling instance method "%1$s::withoutConstructor()" is deprecated and will be removed in 2.0. Use static call instead: "%1$s::withoutConstructor()" instead.', static::class);

$this->withoutConstructor = true;
$this->useConstructor = false;

return $this;
}
Expand All @@ -121,7 +121,7 @@ public static function __callStatic(string $name, array $arguments): self

$instance = new self(calledInternally: true);

$instance->withoutConstructor = true;
$instance->useConstructor = false;

return $instance;
}
Expand Down Expand Up @@ -270,8 +270,8 @@ private function instantiate(string $class, array &$attributes): object
$class = new \ReflectionClass($class);
$constructor = $class->getConstructor();

if ($this->withoutConstructor || !$constructor || !$constructor->isPublic()) {
if (!$this->withoutConstructor && $constructor && !$constructor->isPublic()) {
if (!$this->useConstructor || !$constructor || !$constructor->isPublic()) {
if ($this->useConstructor && $constructor && !$constructor->isPublic()) {
trigger_deprecation('zenstruck\foundry', '1.37.0', 'Instantiator was created to instantiate "%s" by calling the constructor whereas the constructor is not public. This is deprecated and will throw an exception in Foundry 2.0. Use "%s::withoutConstructor()" instead or make constructor public.', $class->getName(), self::class);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public function custom_instantiator_config(): void
{
$this->load([
'instantiator' => [
'without_constructor' => true,
'use_constructor' => false,
'allow_extra_attributes' => true,
'always_force_properties' => true,
],
Expand All @@ -139,8 +139,39 @@ public function custom_instantiator_config(): void

// matthiasnoback/symfony-dependency-injection-test cannot assert if a service is created through a factory.
// so, we're checking that private property "Instantiator::$withoutConstructor" was set to true.
$withoutConstructor = \Closure::bind(static fn(Instantiator $instantiator) => $instantiator->withoutConstructor, null, Instantiator::class)($instantiator);
self::assertTrue($withoutConstructor);
$useConstructor = \Closure::bind(static fn(Instantiator $instantiator) => $instantiator->useConstructor, null, Instantiator::class)($instantiator);
self::assertFalse($useConstructor);
}

/**
* @test
* @group legacy
*/
public function can_configure_instantiator_without_constructor(): void
{
$this->load(['instantiator' => ['without_constructor' => true]]);

$instantiator = $this->container->get('.zenstruck_foundry.default_instantiator');

// matthiasnoback/symfony-dependency-injection-test cannot assert if a service is created through a factory.
// so, we're checking that private property "Instantiator::$withoutConstructor" was set to true.
$useConstructor = \Closure::bind(static fn(Instantiator $instantiator) => $instantiator->useConstructor, null, Instantiator::class)($instantiator);
self::assertFalse($useConstructor);
}

/**
* @test
* @group legacy
*/
public function throws_exception_when_instantiator_has_wrong_configuration(): void
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Cannot set "without_constructor" and "use_constructor" to the same value.');

$this->load(['instantiator' => [
'without_constructor' => true,
'use_constructor' => true,
]]);
}

/**
Expand Down Expand Up @@ -171,7 +202,7 @@ public function cannot_configure_allow_extra_attributes_if_using_custom_instanti
public function cannot_configure_without_constructor_if_using_custom_instantiator_service(): void
{
$this->expectException(InvalidConfigurationException::class);
$this->expectExceptionMessage('Invalid configuration for path "zenstruck_foundry.instantiator": Cannot set "without_constructor" when using custom service.');
$this->expectExceptionMessage('Invalid configuration for path "zenstruck_foundry.instantiator": Cannot set "use_constructor: false" when using custom service.');

$this->load(['instantiator' => ['service' => 'my_instantiator', 'without_constructor' => true]]);
}
Expand Down

0 comments on commit 563f4da

Please sign in to comment.