Skip to content

Commit 5a56807

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '4.4' into 5.0
* 4.4: [DI] Fix EnvVar not loaded when Loader requires an env var Fixed #34713 Move new messages to intl domain when possible [FrameworkBundle] Fix small typo in output comment chown and chgrp should also accept int as owner and group Revert "Fixed translations file dumper behavior" Fix RememberMe with null password [Validator] Fix plurals for sr_Latn (Serbian language written in latin script) validation messages Set booted flag to false when test kernel is unset [FrameworkBundle] remove messenger cache if not enabled [PhpUnitBridge][SymfonyTestsListenerTrait] Remove some unneeded code [HttpClient] Fix strict parsing of response status codes fix PHP const mapping keys using the inline notation [SecurityBundle] Drop duplicated code [FrameworkBundle] Make sure one can use fragments.hinclude_default_template Fix that no-cache requires positive validation with the origin, even for fresh responses Improve upgrading instructions for deprecated router options [DI] Suggest typed argument when binding fails with untyped argument
2 parents f76109a + 6faf589 commit 5a56807

File tree

4 files changed

+99
-18
lines changed

4 files changed

+99
-18
lines changed

‎Compiler/ResolveBindingsPass.php

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ protected function processValue($value, bool $isRoot = false)
114114
return parent::processValue($value, $isRoot);
115115
}
116116

117+
$bindingNames = [];
118+
117119
foreach ($bindings as $key => $binding) {
118120
list($bindingValue, $bindingId, $used, $bindingType, $file) = $binding->getValues();
119121
if ($used) {
@@ -123,7 +125,11 @@ protected function processValue($value, bool $isRoot = false)
123125
$this->unusedBindings[$bindingId] = [$key, $this->currentId, $bindingType, $file];
124126
}
125127

126-
if (preg_match('/^(?:(?:array|bool|float|int|string) )?\$/', $key)) {
128+
if (preg_match('/^(?:(?:array|bool|float|int|string|([^ $]++)) )\$/', $key, $m)) {
129+
$bindingNames[substr($key, \strlen($m[0]))] = $binding;
130+
}
131+
132+
if (!isset($m[1])) {
127133
continue;
128134
}
129135

@@ -184,11 +190,17 @@ protected function processValue($value, bool $isRoot = false)
184190
continue;
185191
}
186192

187-
if (!$typeHint || '\\' !== $typeHint[0] || !isset($bindings[$typeHint = substr($typeHint, 1)])) {
193+
if ($typeHint && '\\' === $typeHint[0] && isset($bindings[$typeHint = substr($typeHint, 1)])) {
194+
$arguments[$key] = $this->getBindingValue($bindings[$typeHint]);
195+
188196
continue;
189197
}
190198

191-
$arguments[$key] = $this->getBindingValue($bindings[$typeHint]);
199+
if (isset($bindingNames[$parameter->name])) {
200+
$bindingKey = array_search($binding, $bindings, true);
201+
$argumentType = substr($bindingKey, 0, strpos($bindingKey, ' '));
202+
$this->errorMessages[] = sprintf('Did you forget to add the type "%s" to argument "$%s" of method "%s::%s()"?', $argumentType, $parameter->name, $reflectionMethod->class, $reflectionMethod->name);
203+
}
192204
}
193205

194206
if ($arguments !== $call[1]) {

‎EnvVarProcessor.php

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ class EnvVarProcessor implements EnvVarProcessorInterface
3030
public function __construct(ContainerInterface $container, \Traversable $loaders = null)
3131
{
3232
$this->container = $container;
33-
$this->loaders = new \IteratorIterator($loaders ?? new \ArrayIterator());
34-
$this->loaders = $this->loaders->getInnerIterator();
33+
$this->loaders = $loaders ?? new \ArrayIterator();
3534
}
3635

3736
/**
@@ -141,20 +140,32 @@ public function getEnv(string $prefix, string $name, \Closure $getEnv)
141140
}
142141
}
143142

144-
$loaders = $this->loaders;
145-
$this->loaders = new \ArrayIterator();
146-
147-
try {
148-
while ((false === $env || null === $env) && $loaders->valid()) {
149-
$loader = $loaders->current();
150-
$loaders->next();
151-
$this->loadedVars[] = $vars = $loader->loadEnvVars();
152-
$env = $vars[$name] ?? false;
143+
if (false === $env || null === $env) {
144+
$loaders = $this->loaders;
145+
$this->loaders = new \ArrayIterator();
146+
147+
try {
148+
$i = 0;
149+
$ended = true;
150+
$count = $loaders instanceof \Countable ? $loaders->count() : 0;
151+
foreach ($loaders as $loader) {
152+
if (\count($this->loadedVars) > $i++) {
153+
continue;
154+
}
155+
$this->loadedVars[] = $vars = $loader->loadEnvVars();
156+
if (false !== $env = $vars[$name] ?? false) {
157+
$ended = false;
158+
break;
159+
}
160+
}
161+
if ($ended || $count === $i) {
162+
$loaders = $this->loaders;
163+
}
164+
} catch (ParameterCircularReferenceException $e) {
165+
// skip loaders that need an env var that is not defined
166+
} finally {
167+
$this->loaders = $loaders;
153168
}
154-
} catch (ParameterCircularReferenceException $e) {
155-
// skip loaders that need an env var that is not defined
156-
} finally {
157-
$this->loaders = $loaders;
158169
}
159170

160171
if (false === $env || null === $env) {

‎Tests/Compiler/ResolveBindingsPassTest.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\DependencyInjection\Compiler\ResolveBindingsPass;
2121
use Symfony\Component\DependencyInjection\ContainerBuilder;
2222
use Symfony\Component\DependencyInjection\Definition;
23+
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
2324
use Symfony\Component\DependencyInjection\Reference;
2425
use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass;
2526
use Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy;
@@ -169,4 +170,19 @@ public function testSyntheticServiceWithBind()
169170

170171
$this->assertSame([1 => 'bar'], $container->getDefinition(NamedArgumentsDummy::class)->getArguments());
171172
}
173+
174+
public function testEmptyBindingTypehint()
175+
{
176+
$this->expectException(InvalidArgumentException::class);
177+
$this->expectExceptionMessage('Did you forget to add the type "string" to argument "$apiKey" of method "Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy::__construct()"?');
178+
179+
$container = new ContainerBuilder();
180+
$bindings = [
181+
'string $apiKey' => new BoundArgument('foo'),
182+
];
183+
$definition = $container->register(NamedArgumentsDummy::class, NamedArgumentsDummy::class);
184+
$definition->setBindings($bindings);
185+
$pass = new ResolveBindingsPass();
186+
$pass->process($container);
187+
}
172188
}

‎Tests/EnvVarProcessorTest.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
namespace Symfony\Component\DependencyInjection\Tests;
44

55
use PHPUnit\Framework\TestCase;
6+
use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
67
use Symfony\Component\DependencyInjection\Container;
78
use Symfony\Component\DependencyInjection\ContainerBuilder;
89
use Symfony\Component\DependencyInjection\EnvVarLoaderInterface;
910
use Symfony\Component\DependencyInjection\EnvVarProcessor;
11+
use Symfony\Component\DependencyInjection\Exception\ParameterCircularReferenceException;
1012

1113
class EnvVarProcessorTest extends TestCase
1214
{
@@ -553,4 +555,44 @@ public function loadEnvVars(): array
553555
$result = $processor->getEnv('string', 'FOO_ENV_LOADER', function () {});
554556
$this->assertSame('123', $result); // check twice
555557
}
558+
559+
public function testCircularEnvLoader()
560+
{
561+
$container = new ContainerBuilder();
562+
$container->setParameter('env(FOO_CONTAINER)', 'foo');
563+
$container->compile();
564+
565+
$index = 0;
566+
$loaders = function () use (&$index) {
567+
if (0 === $index++) {
568+
throw new ParameterCircularReferenceException(['FOO_CONTAINER']);
569+
}
570+
571+
yield new class() implements EnvVarLoaderInterface {
572+
public function loadEnvVars(): array
573+
{
574+
return [
575+
'FOO_ENV_LOADER' => '123',
576+
];
577+
}
578+
};
579+
};
580+
581+
$processor = new EnvVarProcessor($container, new RewindableGenerator($loaders, 1));
582+
583+
$result = $processor->getEnv('string', 'FOO_CONTAINER', function () {});
584+
$this->assertSame('foo', $result);
585+
586+
$result = $processor->getEnv('string', 'FOO_ENV_LOADER', function () {});
587+
$this->assertSame('123', $result);
588+
589+
$result = $processor->getEnv('default', ':BAR_CONTAINER', function ($name) use ($processor) {
590+
$this->assertSame('BAR_CONTAINER', $name);
591+
592+
return $processor->getEnv('string', $name, function () {});
593+
});
594+
$this->assertNull($result);
595+
596+
$this->assertSame(2, $index);
597+
}
556598
}

0 commit comments

Comments
 (0)