8000 [#17878] Fixing a bug where scalar values caused invalid ordering · symfony/symfony@865f202 · GitHub
[go: up one dir, main page]

Skip to content

Commit 865f202

Browse files
committed
[#17878] Fixing a bug where scalar values caused invalid ordering
1 parent cf692a6 commit 865f202

File tree

2 files changed

+113
-1
lines changed

2 files changed

+113
-1
lines changed

src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ private function completeDefinition($id, Definition $definition)
7878

7979
try {
8080
if (!$typeHint = $parameter->getClass()) {
81+
// no default value? Then fail
82+
if (!$parameter->isOptional()) {
83+
throw new RuntimeException(sprintf('Unable to autowire argument index %s ($%s) for the service "%s". If this is an object, give it a type-hint. Otherwise, specify this argument\'s value explicitly.', $index, $parameter->name, $id));
84+
}
85+
86+
// specifically pass the default value
87+
$arguments[$index] = $parameter->getDefaultValue();
88+
8189
continue;
8290
}
8391

src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php

Lines changed: 105 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,6 @@ public function testSomeSpecificArgumentsAreSet()
302302
$pass->process($container);
303303

304304
$definition = $container->getDefinition('multiple');
305-
// takes advantage of Reference's __toString
306305
$this->assertEquals(
307306
array(
308307
new Reference('a'),
@@ -312,6 +311,92 @@ public function testSomeSpecificArgumentsAreSet()
312311
$definition->getArguments()
313312
);
314313
}
314+
315+
/**
316+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
317+
* @expectedExceptionMessage Unable to autowire argument index 1 ($foo) for the service "arg_no_type_hint". If this is an object, give it a type-hint. Otherwise, specify this argument's value explicitly.
318+
*/
319+
public function testScalarArgsCannotBeAutowired()
320+
{
321+
$container = new ContainerBuilder();
322+
323+
$container->register('a', __NAMESPACE__.'\A');
324+
$container->register('dunglas', __NAMESPACE__.'\Dunglas');
325+
$container->register('arg_no_type_hint', __NAMESPACE__.'\MultipleArguments')
326+
->setAutowired(true);
327+
328+
$pass = new AutowirePass();
329+
$pass->process($container);
330+
331+
$container->getDefinition('arg_no_type_hint');
332+
}
333+
334+
/**
335+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
336+
* @expectedExceptionMessage Unable to autowire argument index 1 ($foo) for the service "not_really_optional_scalar". If this is an object, give it a type-hint. Otherwise, specify this argument's value explicitly.
337+
*/
338+
public function testOptionalScalarNotReallyOptionalThrowException()
339+
{
340+
$container = new ContainerBuilder();
341+
342+
$container->register('a', __NAMESPACE__.'\A');
343+
$container->register('lille', __NAMESPACE__.'\Lille');
344+
$container->register('not_really_optional_scalar', __NAMESPACE__.'\MultipleArgumentsOptionalScalarNotReallyOptional')
345+
->setAutowired(true);
346+
347+
$pass = new AutowirePass();
348+
$pass->process($container);
349+
}
350+
351+
public function testOptionalScalarArgsDontMessUpOrder()
352+
{
353+
$container = new ContainerBuilder();
354+
355+
$container->register('a', __NAMESPACE__.'\A');
356+
$container->register('lille', __NAMESPACE__.'\Lille');
357+
$container->register('with_optional_scalar', __NAMESPACE__.'\MultipleArgumentsOptionalScalar')
358+
->setAutowired(true);
359+
360+
$pass = new AutowirePass();
361+
$pass->process($container);
362+
363+
$definition = $container->getDefinition('with_optional_scalar');
364+
$this->assertEquals(
365+
array(
366+
new Reference('a'),
367+
// use the default value
368+
'default_val',
369+
new Reference('lille'),
370+
),
371+
$definition->getArguments()
372+
);
373+
}
374+
375+
public function testOptionalScalarArgsNotPassedIfLast()
376+
{
377+
$container = new ContainerBuilder();
378+
379+
$container->register('a', __NAMESPACE__.'\A');
380+
$container->register('lille', __NAMESPACE__.'\Lille');
381+
$container->register('with_optional_scalar_last', __NAMESPACE__.'\MultipleArgumentsOptionalScalarLast')
382+
->setAutowired(true);
383+
384+
$pass = new AutowirePass();
385+
$pass->process($container);
386+
387+
$definition = $container->getDefinition('with_optional_scalar_last');
388+
$this->assertEquals(
389+
array(
390+
new Reference('a'),
391+
new Reference('lille'),
392+
// third arg shouldn't *need* to be passed
393+
// but that's hard to "pull of" with autowiring, so
394+
// this assumes passing the default val is ok
395+
'some_val',
396+
),
397+
$definition->getArguments()
398+
);
399+
}
315400
}
316401

317402
class Foo
@@ -441,4 +526,23 @@ class MultipleArguments
441526
public function __construct(A $k, $foo, Dunglas $dunglas)
442527
{
443528
}
529+
}
530+
531+
class MultipleArgumentsOptionalScalar
532+
{
533+
public function __construct(A $a, $foo = 'default_val', Lille $lille = null)
534+
{
535+
}
536+
}
537+
class MultipleArgumentsOptionalScalarLast
538+
{
539+
public function __construct(A $a, Lille $lille, $foo = 'some_val')
540+
{
541+
}
542+
}
543+
class MultipleArgumentsOptionalScalarNotReallyOptional
544+
{
545+
public function __construct(A $a, $foo = 'default_val', Lille $lille)
546+
{
547+
}
444548
}

0 commit comments

Comments
 (0)
0