From 86b850ebd8f7843a8a85cbe8a48953811c5ef32e Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 7 Apr 2018 16:29:23 +0200 Subject: [PATCH 1/3] [HttpKernel] Dont create mock cookie for new sessions in tests --- .../EventListener/AbstractTestSessionListener.php | 2 +- .../Tests/EventListener/TestSessionListenerTest.php | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php index 0a153dd943297..82061fd6ea0fc 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php @@ -69,7 +69,7 @@ public function onKernelResponse(FilterResponseEvent $event) $session->save(); } - if ($session instanceof Session ? !$session->isEmpty() || $session->getId() !== $this->sessionId : $wasStarted) { + if ($session instanceof Session ? !$session->isEmpty() || (null !== $this->sessionId && $session->getId() !== $this->sessionId) : $wasStarted) { $params = session_get_cookie_params(); $event->getResponse()->headers->setCookie(new Cookie($session->getName(), $session->getId(), 0 === $params['lifetime'] ? 0 : time() + $params['lifetime'], $params['path'], $params['domain'], $params['secure'], $params['httponly'])); $this->sessionId = $session->getId(); diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/TestSessionListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/TestSessionListenerTest.php index 0a2263d5a8e78..22a2b71239874 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/TestSessionListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/TestSessionListenerTest.php @@ -13,7 +13,6 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ServiceSubscriberInterface; -use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\GetResponseEvent; @@ -46,6 +45,9 @@ protected function setUp() { $this->listener = $this->getMockForAbstractClass('Symfony\Component\HttpKernel\EventListener\AbstractTestSessionListener'); $this->session = $this->getSession(); + $this->listener->expects($this->any()) + ->method('getSession') + ->will($this->returnValue($this->session)); } public function testShouldSaveMasterRequestSession() @@ -95,7 +97,7 @@ public function testEmptySessionWithNewSessionIdDoesSendCookie() $this->fixSessionId('456'); $kernel = $this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(); - $request = Request::create('/', 'GET', array(), array(new Cookie('MOCKSESSID', '123'))); + $request = Request::create('/', 'GET', array(), array('MOCKSESSID' => '123')); $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST); $this->listener->onKernelRequest($event); From 7f398117f490b724dcfedd3663c76dfc49553392 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 7 Apr 2018 15:42:06 +0200 Subject: [PATCH 2/3] [DI] Improve error message for non-autowirable scalar argument --- .../Compiler/AutowirePass.php | 5 ++++- .../Tests/Compiler/AutowirePassTest.php | 22 ++++++++++++++++++- .../Fixtures/includes/autowiring_classes.php | 2 +- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 8bfc3cd9d9677..803490cecd295 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -233,7 +233,10 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a if ($parameter->isOptional()) { continue; } - throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": argument "$%s" of method "%s()" must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method)); + $type = ProxyHelper::getTypeHint($reflectionMethod, $parameter, false); + $type = $type ? sprintf('is type-hinted "%s"', $type) : 'has no type-hint'; + + throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": argument "$%s" of method "%s()" %s, you should configure its value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method, $type)); } // specifically pass the default value diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index ee0b4baef8452..924a490942492 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -436,6 +436,7 @@ public function testSomeSpecificArgumentsAreSet() // args are: A, Foo, Dunglas ->setArguments(array( 1 => new Reference('foo'), + 3 => array('bar'), )); (new ResolveClassPass())->process($container); @@ -447,6 +448,7 @@ public function testSomeSpecificArgumentsAreSet() new TypedReference(A::class, A::class, MultipleArguments::class), new Reference('foo'), new TypedReference(Dunglas::class, Dunglas::class, MultipleArguments::class), + array('bar'), ), $definition->getArguments() ); @@ -454,12 +456,30 @@ public function testSomeSpecificArgumentsAreSet() /** * @expectedException \Symfony\Component\DependencyInjection\Exception\AutowiringFailedException - * @expectedExceptionMessage Cannot autowire service "arg_no_type_hint": argument "$foo" of method "Symfony\Component\DependencyInjection\Tests\Compiler\MultipleArguments::__construct()" must have a type-hint or be given a value explicitly. + * @expectedExceptionMessage Cannot autowire service "arg_no_type_hint": argument "$bar" of method "Symfony\Component\DependencyInjection\Tests\Compiler\MultipleArguments::__construct()" is type-hinted "array", you should configure its value explicitly. */ public function testScalarArgsCannotBeAutowired() { $container = new ContainerBuilder(); + $container->register(A::class); + $container->register(Dunglas::class); + $container->register('arg_no_type_hint', __NAMESPACE__.'\MultipleArguments') + ->setArguments(array(1 => 'foo')) + ->setAutowired(true); + + (new ResolveClassPass())->process($container); + (new AutowirePass())->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\AutowiringFailedException + * @expectedExceptionMessage Cannot autowire service "arg_no_type_hint": argument "$foo" of method "Symfony\Component\DependencyInjection\Tests\Compiler\MultipleArguments::__construct()" has no type-hint, you should configure its value explicitly. + */ + public function testNoTypeArgsCannotBeAutowired() + { + $container = new ContainerBuilder(); + $container->register(A::class); $container->register(Dunglas::class); $container->register('arg_no_type_hint', __NAMESPACE__.'\MultipleArguments') diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php index 990a873f34985..6f93957753a49 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php @@ -181,7 +181,7 @@ public function __construct(A $k) } class MultipleArguments { - public function __construct(A $k, $foo, Dunglas $dunglas) + public function __construct(A $k, $foo, Dunglas $dunglas, array $bar) { } } From 40bd8bd797ae8921e2a04f71a1ba7e1d3c0c29a3 Mon Sep 17 00:00:00 2001 From: Normunds Date: Mon, 9 Apr 2018 15:54:29 +0300 Subject: [PATCH 3/3] Add d-block to bootstrap 4 alerts --- .../Twig/Resources/views/Form/bootstrap_4_layout.html.twig | 2 +- .../Form/Tests/AbstractBootstrap4HorizontalLayoutTest.php | 2 +- .../Component/Form/Tests/AbstractBootstrap4LayoutTest.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig b/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig index df88a00011883..1c5c390954790 100644 --- a/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig +++ b/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig @@ -267,7 +267,7 @@ {% block form_errors -%} {%- if errors|length > 0 -%} - + {%- for error in errors -%} {{ 'Error'|trans({}, 'validators') }} {{ error.message }} diff --git a/src/Symfony/Component/Form/Tests/AbstractBootstrap4HorizontalLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractBootstrap4HorizontalLayoutTest.php index c906c6549ad2f..016792e0edaf6 100644 --- a/src/Symfony/Component/Form/Tests/AbstractBootstrap4HorizontalLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractBootstrap4HorizontalLayoutTest.php @@ -32,7 +32,7 @@ public function testRow() [ ./label[@for="name"] [ - ./span[@class="alert alert-danger"] + ./span[@class="alert alert-danger d-block"] [./span[@class="mb-0 d-block"] [./span[.="[trans]Error[/trans]"]] [./span[.="[trans]Error![/trans]"]] diff --git a/src/Symfony/Component/Form/Tests/AbstractBootstrap4LayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractBootstrap4LayoutTest.php index beabaa21cdb1a..2cadba0ba9e67 100644 --- a/src/Symfony/Component/Form/Tests/AbstractBootstrap4LayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractBootstrap4LayoutTest.php @@ -32,7 +32,7 @@ public function testRow() [ ./label[@for="name"] [ - ./span[@class="alert alert-danger"] + ./span[@class="alert alert-danger d-block"] [./span[@class="mb-0 d-block"] [./span[.="[trans]Error[/trans]"]] [./span[.="[trans]Error![/trans]"]] @@ -161,7 +161,7 @@ public function testErrors() $this->assertMatchesXpath($html, '/span - [@class="alert alert-danger"] + [@class="alert alert-danger d-block"] [ ./span[@class="mb-0 d-block"] [./span[.="[trans]Error[/trans]"]]