Skip to content

Commit

Permalink
[DependencyInjection] fix support for "new" in initializers on PHP 8.1
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolas-grekas committed Oct 4, 2021
1 parent 75dd709 commit 480eb71
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 17 deletions.
55 changes: 47 additions & 8 deletions Compiler/AutowirePass.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class AutowirePass extends AbstractRecursivePass
private $decoratedClass;
private $decoratedId;
private $methodCalls;
private $defaultArgument;
private $getPreviousValue;
private $decoratedMethodIndex;
private $decoratedMethodArgumentIndex;
Expand All @@ -42,6 +43,10 @@ class AutowirePass extends AbstractRecursivePass
public function __construct(bool $throwOnAutowireException = true)
{
$this->throwOnAutowiringException = $throwOnAutowireException;
$this->defaultArgument = new class() {
public $value;
public $names;
};
}

/**
Expand All @@ -56,6 +61,7 @@ public function process(ContainerBuilder $container)
$this->decoratedClass = null;
$this->decoratedId = null;
$this->methodCalls = null;
$this->defaultArgument->names = null;
$this->getPreviousValue = null;
$this->decoratedMethodIndex = null;
$this->decoratedMethodArgumentIndex = null;
Expand Down Expand Up @@ -150,8 +156,9 @@ private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot):
$this->decoratedClass = $this->container->findDefinition($this->decoratedId)->getClass();
}

$patchedIndexes = [];

foreach ($this->methodCalls as $i => $call) {
$this->decoratedMethodIndex = $i;
[$method, $arguments] = $call;

if ($method instanceof \ReflectionFunctionAbstract) {
Expand All @@ -168,11 +175,37 @@ private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot):
}
}

$arguments = $this->autowireMethod($reflectionMethod, $arguments);
$arguments = $this->autowireMethod($reflectionMethod, $arguments, $i);

if ($arguments !== $call[1]) {
$this->methodCalls[$i][1] = $arguments;
$patchedIndexes[] = $i;
}
}

// use named arguments to skip complex default values
foreach ($patchedIndexes as $i) {
$namedArguments = null;
$arguments = $this->methodCalls[$i][1];

foreach ($arguments as $j => $value) {
if ($namedArguments && !$value instanceof $this->defaultArgument) {
unset($arguments[$j]);
$arguments[$namedArguments[$j]] = $value;
}
if ($namedArguments || !$value instanceof $this->defaultArgument) {
continue;
}

if (\PHP_VERSION_ID >= 80100 && (\is_array($value->value) ? $value->value : \is_object($value->value))) {
unset($arguments[$j]);
$namedArguments = $value->names;
} else {
$arguments[$j] = $value->value;
}
}

$this->methodCalls[$i][1] = $arguments;
}

return $this->methodCalls;
Expand All @@ -185,16 +218,19 @@ private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot):
*
* @throws AutowiringFailedException
*/
private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, array $arguments): array
private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, array $arguments, int $methodIndex): array
{
$class = $reflectionMethod instanceof \ReflectionMethod ? $reflectionMethod->class : $this->currentId;
$method = $reflectionMethod->name;
$parameters = $reflectionMethod->getParameters();
if ($reflectionMethod->isVariadic()) {
array_pop($parameters);
}
$this->defaultArgument->names = new \ArrayObject();

foreach ($parameters as $index => $parameter) {
$this->defaultArgument->names[$index] = $parameter->name;

if (\array_key_exists($index, $arguments) && '' !== $arguments[$index]) {
continue;
}
Expand All @@ -212,7 +248,8 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
// be false when isOptional() returns true. If the
// argument *is* optional, allow it to be missing
if ($parameter->isOptional()) {
continue;
--$index;
break;
}
$type = ProxyHelper::getTypeHint($reflectionMethod, $parameter, false);
$type = $type ? sprintf('is type-hinted "%s"', ltrim($type, '\\')) : 'has no type-hint';
Expand All @@ -221,7 +258,8 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
}

// specifically pass the default value
$arguments[$index] = $parameter->getDefaultValue();
$arguments[$index] = clone $this->defaultArgument;
$arguments[$index]->value = $parameter->getDefaultValue();

continue;
}
Expand All @@ -231,7 +269,8 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
$failureMessage = $this->createTypeNotFoundMessageCallback($ref, sprintf('argument "$%s" of method "%s()"', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));

if ($parameter->isDefaultValueAvailable()) {
$value = $parameter->getDefaultValue();
$value = clone $this->defaultArgument;
$value->value = $parameter->getDefaultValue();
} elseif (!$parameter->allowsNull()) {
throw new AutowiringFailedException($this->currentId, $failureMessage);
}
Expand All @@ -252,6 +291,7 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
} else {
$arguments[$index] = new TypedReference($this->decoratedId, $this->decoratedClass);
$this->getPreviousValue = $getValue;
$this->decoratedMethodIndex = $methodIndex;
$this->decoratedMethodArgumentIndex = $index;

continue;
Expand All @@ -263,8 +303,7 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a

if ($parameters && !isset($arguments[++$index])) {
while (0 <= --$index) {
$parameter = $parameters[$index];
if (!$parameter->isDefaultValueAvailable() || $parameter->getDefaultValue() !== $arguments[$index]) {
if (!$arguments[$index] instanceof $this->defaultArgument) {
break;
}
unset($arguments[$index]);
Expand Down
32 changes: 32 additions & 0 deletions Compiler/CheckArgumentsValidityPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ protected function processValue($value, $isRoot = false)
}

$i = 0;
$hasNamedArgs = false;
foreach ($value->getArguments() as $k => $v) {
if (\PHP_VERSION_ID >= 80000 && preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/', $k)) {
$hasNamedArgs = true;
continue;
}

if ($k !== $i++) {
if (!\is_int($k)) {
$msg = sprintf('Invalid constructor argument for service "%s": integer expected but found string "%s". Check your service definition.', $this->currentId, $k);
Expand All @@ -57,11 +63,27 @@ protected function processValue($value, $isRoot = false)
throw new RuntimeException($msg);
}
}

if ($hasNamedArgs) {
$msg = sprintf('Invalid constructor argument for service "%s": cannot use positional argument after named argument. Check your service definition.', $this->currentId);
$value->addError($msg);
if ($this->throwExceptions) {
throw new RuntimeException($msg);
}

break;
}
}

foreach ($value->getMethodCalls() as $methodCall) {
$i = 0;
$hasNamedArgs = false;
foreach ($methodCall[1] as $k => $v) {
if (\PHP_VERSION_ID >= 80000 && preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/', $k)) {
$hasNamedArgs = true;
continue;
}

if ($k !== $i++) {
if (!\is_int($k)) {
$msg = sprintf('Invalid argument for method call "%s" of service "%s": integer expected but found string "%s". Check your service definition.', $methodCall[0], $this->currentId, $k);
Expand All @@ -79,6 +101,16 @@ protected function processValue($value, $isRoot = false)
throw new RuntimeException($msg);
}
}

if ($hasNamedArgs) {
$msg = sprintf('Invalid argument for method call "%s" of service "%s": cannot use positional argument after named argument. Check your service definition.', $methodCall[0], $this->currentId);
$value->addError($msg);
if ($this->throwExceptions) {
throw new RuntimeException($msg);
}

break;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion Compiler/InlineServiceDefinitionsPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public function process(ContainerBuilder $container)
protected function processValue($value, $isRoot = false)
{
if ($value instanceof ArgumentInterface) {
// Reference found in ArgumentInterface::getValues() are not inlineable
// References found in ArgumentInterface::getValues() are not inlineable
return $value;
}

Expand Down
8 changes: 4 additions & 4 deletions Dumper/PhpDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -714,8 +714,8 @@ private function addServiceMethodCalls(Definition $definition, string $variableN
$calls = '';
foreach ($definition->getMethodCalls() as $k => $call) {
$arguments = [];
foreach ($call[1] as $value) {
$arguments[] = $this->dumpValue($value);
foreach ($call[1] as $i => $value) {
$arguments[] = (\is_string($i) ? $i.': ' : '').$this->dumpValue($value);
}

$witherAssignation = '';
Expand Down Expand Up @@ -1080,8 +1080,8 @@ private function addNewInstance(Definition $definition, string $return = '', str
}

$arguments = [];
foreach ($definition->getArguments() as $value) {
$arguments[] = $this->dumpValue($value);
foreach ($definition->getArguments() as $i => $value) {
$arguments[] = (\is_string($i) ? $i.': ' : '').$this->dumpValue($value);
}

if (null !== $definition->getFactory()) {
Expand Down
8 changes: 4 additions & 4 deletions Tests/Compiler/CheckArgumentsValidityPassTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,22 @@ public function testProcess()
*/
public function testException(array $arguments, array $methodCalls)
{
$this->expectException(RuntimeException::class);
$container = new ContainerBuilder();
$definition = $container->register('foo');
$definition->setArguments($arguments);
$definition->setMethodCalls($methodCalls);

$pass = new CheckArgumentsValidityPass();
$this->expectException(RuntimeException::class);
$pass->process($container);
}

public function definitionProvider()
{
return [
[[null, 'a' => 'a'], []],
[['a' => 'a', null], []],
[[1 => 1], []],
[[], [['baz', [null, 'a' => 'a']]]],
[[], [['baz', ['a' => 'a', null]]]],
[[], [['baz', [1 => 1]]]],
];
}
Expand All @@ -70,7 +70,7 @@ public function testNoException()
{
$container = new ContainerBuilder();
$definition = $container->register('foo');
$definition->setArguments([null, 'a' => 'a']);
$definition->setArguments(['a' => 'a', null]);

$pass = new CheckArgumentsValidityPass(false);
$pass->process($container);
Expand Down
19 changes: 19 additions & 0 deletions Tests/Dumper/PhpDumperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
use Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition;
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithEnumAttribute;
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooUnitEnum;
use Symfony\Component\DependencyInjection\Tests\Fixtures\NewInInitializer;
use Symfony\Component\DependencyInjection\Tests\Fixtures\ScalarFactory;
use Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator;
use Symfony\Component\DependencyInjection\Tests\Fixtures\TestDefinition1;
Expand Down Expand Up @@ -1187,6 +1188,24 @@ public function testDumpHandlesObjectClassNames()
$this->assertInstanceOf(\stdClass::class, $container->get('bar'));
}

/**
* @requires PHP 8.1
*/
public function testNewInInitializer()
{
$container = new ContainerBuilder();
$container
->register('foo', NewInInitializer::class)
->setPublic(true)
->setAutowired(true)
->setArguments(['$bar' => 234]);

$container->compile();

$dumper = new PhpDumper($container);
$this->assertStringEqualsFile(self::$fixturesPath.'/php/services_new_in_initializer.php', $dumper->dump());
}

/**
* @requires PHP 8.1
*/
Expand Down
10 changes: 10 additions & 0 deletions Tests/Fixtures/NewInInitializer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

namespace Symfony\Component\DependencyInjection\Tests\Fixtures;

class NewInInitializer
{
public function __construct($foo = new \stdClass(), $bar = 123)
{
}
}
59 changes: 59 additions & 0 deletions Tests/Fixtures/php/services_new_in_initializer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php

use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Exception\LogicException;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;

/**
* This class has been auto-generated
* by the Symfony Dependency Injection Component.
*
* @final
*/
class ProjectServiceContainer extends Container
{
private $parameters = [];

public function __construct()
{
$this->services = $this->privates = [];
$this->methodMap = [
'foo' => 'getFooService',
];

$this->aliases = [];
}

public function compile(): void
{
throw new LogicException('You cannot compile a dumped container that was already compiled.');
}

public function isCompiled(): bool
{
return true;
}

public function getRemovedIds(): array
{
return [
'Psr\\Container\\ContainerInterface' => true,
'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
];
}

/**
* Gets the public 'foo' shared autowired service.
*
* @return \Symfony\Component\DependencyInjection\Tests\Fixtures\NewInInitializer
*/
protected function getFooService()
{
return $this->services['foo'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\NewInInitializer(bar: 234);
}
}

0 comments on commit 480eb71

Please sign in to comment.