From ef39b704ccbdd6680c71d2d3841c0a8e6277d3ce Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Fri, 24 Mar 2017 10:02:28 +0100 Subject: [PATCH 01/17] [Form] Improve the exceptions when trying to get the data in a PRE_SET_DATA listener and the data has not already been set --- src/Symfony/Component/Form/Form.php | 12 +++++ .../Component/Form/Tests/SimpleFormTest.php | 46 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 54b9b39ebd..87cd15bf47 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -408,6 +408,10 @@ class Form implements \IteratorAggregate, FormInterface } if (!$this->defaultDataSet) { + if ($this->lockSetData) { + throw new RuntimeException('A cycle was detected. Listeners to the PRE_SET_DATA event must not call getData() if the form data has not already been set. You should call getData() on the FormEvent object instead.'); + } + $this->setData($this->config->getData()); } @@ -428,6 +432,10 @@ class Form implements \IteratorAggregate, FormInterface } if (!$this->defaultDataSet) { + if ($this->lockSetData) { + throw new RuntimeException('A cycle was detected. Listeners to the PRE_SET_DATA event must not call getNormData() if the form data has not already been set.'); + } + $this->setData($this->config->getData()); } @@ -448,6 +456,10 @@ class Form implements \IteratorAggregate, FormInterface } if (!$this->defaultDataSet) { + if ($this->lockSetData) { + throw new RuntimeException('A cycle was detected. Listeners to the PRE_SET_DATA event must not call getViewData() if the form data has not already been set.'); + } + $this->setData($this->config->getData()); } diff --git a/src/Symfony/Component/Form/Tests/SimpleFormTest.php b/src/Symfony/Component/Form/Tests/SimpleFormTest.php index 6223d3b39f..62c94c7490 100644 --- a/src/Symfony/Component/Form/Tests/SimpleFormTest.php +++ b/src/Symfony/Component/Form/Tests/SimpleFormTest.php @@ -903,6 +903,7 @@ class SimpleFormTest extends AbstractFormTest /** * @expectedException \Symfony\Component\Form\Exception\RuntimeException + * @expectedExceptionMessage A cycle was detected. Listeners to the PRE_SET_DATA event must not call setData(). You should call setData() on the FormEvent object instead. */ public function testSetDataCannotInvokeItself() { @@ -1084,6 +1085,51 @@ class SimpleFormTest extends AbstractFormTest $fooType->setDefaultOptions($resolver); } + /** + * @expectedException \Symfony\Component\Form\Exception\RuntimeException + * @expectedExceptionMessage A cycle was detected. Listeners to the PRE_SET_DATA event must not call getData() if the form data has not already been set. You should call getData() on the FormEvent object instead. + */ + public function testCannotCallGetDataInPreSetDataListenerIfDataHasNotAlreadyBeenSet() + { + $config = new FormConfigBuilder('name', 'stdClass', $this->dispatcher); + $config->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) { + $event->getForm()->getData(); + }); + $form = new Form($config); + + $form->setData('foo'); + } + + /** + * @expectedException \Symfony\Component\Form\Exception\RuntimeException + * @expectedExceptionMessage A cycle was detected. Listeners to the PRE_SET_DATA event must not call getNormData() if the form data has not already been set. + */ + public function testCannotCallGetNormDataInPreSetDataListener() + { + $config = new FormConfigBuilder('name', 'stdClass', $this->dispatcher); + $config->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) { + $event->getForm()->getNormData(); + }); + $form = new Form($config); + + $form->setData('foo'); + } + + /** + * @expectedException \Symfony\Component\Form\Exception\RuntimeException + * @expectedExceptionMessage A cycle was detected. Listeners to the PRE_SET_DATA event must not call getViewData() if the form data has not already been set. + */ + public function testCannotCallGetViewDataInPreSetDataListener() + { + $config = new FormConfigBuilder('name', 'stdClass', $this->dispatcher); + $config->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) { + $event->getForm()->getViewData(); + }); + $form = new Form($config); + + $form->setData('foo'); + } + protected function createForm() { return $this->getBuilder()->getForm(); From 3fe419cf66a8a9f033db54186896b142c5cbcada Mon Sep 17 00:00:00 2001 From: Julien Falque Date: Tue, 28 Mar 2017 08:19:43 +0200 Subject: [PATCH 02/17] Disable color support detection for tests --- src/Symfony/Component/Console/Tester/CommandTester.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Symfony/Component/Console/Tester/CommandTester.php b/src/Symfony/Component/Console/Tester/CommandTester.php index f95298bc90..609f46a654 100644 --- a/src/Symfony/Component/Console/Tester/CommandTester.php +++ b/src/Symfony/Component/Console/Tester/CommandTester.php @@ -70,9 +70,7 @@ class CommandTester } $this->output = new StreamOutput(fopen('php://memory', 'w', false)); - if (isset($options['decorated'])) { - $this->output->setDecorated($options['decorated']); - } + $this->output->setDecorated(isset($options['decorated']) ? $options['decorated'] : false); if (isset($options['verbosity'])) { $this->output->setVerbosity($options['verbosity']); } From d64679014b5eb6b36594f5d7062ad07f9ef198ab Mon Sep 17 00:00:00 2001 From: Niels Keurentjes Date: Sat, 25 Mar 2017 00:33:11 +0100 Subject: [PATCH 03/17] [WebProfilerBundle] Normalize whitespace in exceptions passed in headers If an exception was thrown with line separators in its message the WebProfiler would cause an exception by passing it through unsanitized into the X-Debug-Error HTTP header. This commit fixes that by replacing all whitespace sequences with a single space in the header. --- .../EventListener/WebDebugToolbarListener.php | 2 +- .../WebDebugToolbarListenerTest.php | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/WebProfilerBundle/EventListener/WebDebugToolbarListener.php b/src/Symfony/Bundle/WebProfilerBundle/EventListener/WebDebugToolbarListener.php index 71c5090fc8..09d2b9ba92 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/EventListener/WebDebugToolbarListener.php +++ b/src/Symfony/Bundle/WebProfilerBundle/EventListener/WebDebugToolbarListener.php @@ -68,7 +68,7 @@ class WebDebugToolbarListener implements EventSubscriberInterface $this->urlGenerator->generate('_profiler', array('token' => $response->headers->get('X-Debug-Token'))) ); } catch (\Exception $e) { - $response->headers->set('X-Debug-Error', get_class($e).': '.$e->getMessage()); + $response->headers->set('X-Debug-Error', get_class($e).': '.preg_replace('/\s+/', ' ', $e->getMessage())); } } diff --git a/src/Symfony/Bundle/WebProfilerBundle/Tests/EventListener/WebDebugToolbarListenerTest.php b/src/Symfony/Bundle/WebProfilerBundle/Tests/EventListener/WebDebugToolbarListenerTest.php index 446aefb793..a121035b7d 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Tests/EventListener/WebDebugToolbarListenerTest.php +++ b/src/Symfony/Bundle/WebProfilerBundle/Tests/EventListener/WebDebugToolbarListenerTest.php @@ -228,6 +228,27 @@ class WebDebugToolbarListenerTest extends \PHPUnit_Framework_TestCase $this->assertEquals('Exception: foo', $response->headers->get('X-Debug-Error')); } + public function testThrowingErrorCleanup() + { + $response = new Response(); + $response->headers->set('X-Debug-Token', 'xxxxxxxx'); + + $urlGenerator = $this->getUrlGeneratorMock(); + $urlGenerator + ->expects($this->once()) + ->method('generate') + ->with('_profiler', array('token' => 'xxxxxxxx')) + ->will($this->throwException(new \Exception("This\nmultiline\r\ntabbed text should\tcome out\r on\n \ta single plain\r\nline"))) + ; + + $event = new FilterResponseEvent($this->getKernelMock(), $this->getRequestMock(), HttpKernelInterface::MASTER_REQUEST, $response); + + $listener = new WebDebugToolbarListener($this->getTwigMock(), false, WebDebugToolbarListener::ENABLED, 'bottom', $urlGenerator); + $listener->onKernelResponse($event); + + $this->assertEquals('Exception: This multiline tabbed text should come out on a single plain line', $response->headers->get('X-Debug-Error')); + } + protected function getRequestMock($isXmlHttpRequest = false, $requestFormat = 'html', $hasSession = true) { $request = $this->getMock( From 6ed9d0e4c17d75550f8c26d16967c42e2ebd308c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 28 Mar 2017 23:38:21 +0200 Subject: [PATCH 04/17] Fix @param in PHPDoc Errors reported by Sami API Doc generator on branch 3.2 ERROR: The "factory" @param tag variable name is wrong (should be "objectLoader") on "Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader::__construct" in src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php:68 ERROR: The "objectLoader" @param tag variable name is wrong (should be "factory") on "Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader::__construct" in src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php:68 ERROR: "7" @param tags are expected but only "6" found on "Symfony\Bundle\WebProfilerBundle\Controller\ProfilerController::__construct" in src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php:50 ERROR: "3" @param tags are expected but only "2" found on "Symfony\Component\Asset\PathPackage::__construct" in src/Symfony/Component/Asset/PathPackage.php:35 ERROR: "2" @param tags are expected but only "1" found on "Symfony\Component\Cache\Adapter\PhpArrayAdapter::create" in src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php:64 ERROR: "3" @param tags are expected but only "1" found on "Symfony\Component\Cache\Adapter\RedisAdapter::__construct" in src/Symfony/Component/Cache/Adapter/RedisAdapter.php:39 ERROR: The "format" @param tag variable name is wrong (should be "fileLinkFormat") on "Symfony\Component\Debug\ExceptionHandler::setFileLinkFormat" in src/Symfony/Component/Debug/ExceptionHandler.php:90 ERROR: "2" @param tags are expected but only "3" found on "Symfony\Component\DependencyInjection\Compiler\Compiler::addPass" in src/Symfony/Component/DependencyInjection/Compiler/Compiler.php:73 ERROR: "2" @param tags are expected but only "3" found on "Symfony\Component\DependencyInjection\Compiler\PassConfig::addPass" in src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php:97 ERROR: "2" @param tags are expected but only "3" found on "Symfony\Component\DependencyInjection\ContainerBuilder::addCompilerPass" in src/Symfony/Component/DependencyInjection/ContainerBuilder.php:311 ERROR: "2" @param tags are expected but only "3" found on "Symfony\Component\DependencyInjection\LazyProxy\PhpDumper\DumperInterface::getProxyFactoryCode" in src/Symfony/Component/DependencyInjection/LazyProxy/PhpDumper/DumperInterface.php:41 ERROR: "0" @param tags are expected but only "1" found on "Symfony\Component\HttpFoundation\Request::isMethodSafe" in src/Symfony/Component/HttpFoundation/Request.php:1458 ERROR: "5" @param tags are expected but only "6" found on "Symfony\Component\Serializer\Normalizer\AbstractNormalizer::instantiateObject" in src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php:291 --- .../Form/ChoiceList/DoctrineChoiceLoader.php | 2 +- .../Controller/ProfilerController.php | 13 +++++++------ src/Symfony/Component/Asset/PathPackage.php | 1 + .../Component/Cache/Adapter/PhpArrayAdapter.php | 3 ++- .../Component/Cache/Adapter/RedisAdapter.php | 4 +++- src/Symfony/Component/Debug/ExceptionHandler.php | 2 +- 6 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php index 9477d82655..f199f16265 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php @@ -61,9 +61,9 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface * loaded objects * @param IdReader $idReader The reader for the object * IDs. + * @param null|EntityLoaderInterface $objectLoader The objects loader * @param ChoiceListFactoryInterface $factory The factory for creating * the loaded choice list - * @param null|EntityLoaderInterface $objectLoader The objects loader */ public function __construct($manager, $class, $idReader = null, $objectLoader = null, $factory = null) { diff --git a/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php b/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php index d811421b00..33092d3646 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php +++ b/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php @@ -40,12 +40,13 @@ class ProfilerController /** * Constructor. * - * @param UrlGeneratorInterface $generator The URL Generator - * @param Profiler $profiler The profiler - * @param \Twig_Environment $twig The twig environment - * @param array $templates The templates - * @param string $toolbarPosition The toolbar position (top, bottom, normal, or null -- use the configuration) - * @param string $baseDir The project root directory + * @param UrlGeneratorInterface $generator The URL Generator + * @param Profiler $profiler The profiler + * @param \Twig_Environment $twig The twig environment + * @param array $templates The templates + * @param string $toolbarPosition The toolbar position (top, bottom, normal, or null -- use the configuration) + * @param ContentSecurityPolicyHandler $cspHandler The Content-Security-Policy handler + * @param string $baseDir The project root directory */ public function __construct(UrlGeneratorInterface $generator, Profiler $profiler = null, \Twig_Environment $twig, array $templates, $toolbarPosition = 'bottom', ContentSecurityPolicyHandler $cspHandler = null, $baseDir = null) { diff --git a/src/Symfony/Component/Asset/PathPackage.php b/src/Symfony/Component/Asset/PathPackage.php index 906879f8b0..a8785c53f4 100644 --- a/src/Symfony/Component/Asset/PathPackage.php +++ b/src/Symfony/Component/Asset/PathPackage.php @@ -31,6 +31,7 @@ class PathPackage extends Package /** * @param string $basePath The base path to be prepended to relative paths * @param VersionStrategyInterface $versionStrategy The version strategy + * @param ContextInterface|null $context The context */ public function __construct($basePath, VersionStrategyInterface $versionStrategy, ContextInterface $context = null) { diff --git a/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php b/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php index 4b1552e1d1..ad287576f5 100644 --- a/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php @@ -57,7 +57,8 @@ class PhpArrayAdapter implements AdapterInterface * stores arrays in its latest versions. This factory method decorates the given * fallback pool with this adapter only if the current PHP version is supported. * - * @param string $file The PHP file were values are cached + * @param string $file The PHP file were values are cached + * @param CacheItemPoolInterface $fallbackPool Fallback for old PHP versions or opcache disabled * * @return CacheItemPoolInterface */ diff --git a/src/Symfony/Component/Cache/Adapter/RedisAdapter.php b/src/Symfony/Component/Cache/Adapter/RedisAdapter.php index 51a35c6f71..5cae772ef7 100644 --- a/src/Symfony/Component/Cache/Adapter/RedisAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/RedisAdapter.php @@ -34,7 +34,9 @@ class RedisAdapter extends AbstractAdapter private $redis; /** - * @param \Redis|\RedisArray|\RedisCluster|\Predis\Client $redisClient + * @param \Redis|\RedisArray|\RedisCluster|\Predis\Client $redisClient The redis client + * @param string $namespace The default namespace + * @param integer $defaultLifetime The default lifetime */ public function __construct($redisClient, $namespace = '', $defaultLifetime = 0) { diff --git a/src/Symfony/Component/Debug/ExceptionHandler.php b/src/Symfony/Component/Debug/ExceptionHandler.php index 2fdd8456fd..f2ae4b102e 100644 --- a/src/Symfony/Component/Debug/ExceptionHandler.php +++ b/src/Symfony/Component/Debug/ExceptionHandler.php @@ -83,7 +83,7 @@ class ExceptionHandler /** * Sets the format for links to source files. * - * @param string|FileLinkFormatter $format The format for links to source files + * @param string|FileLinkFormatter $fileLinkFormat The format for links to source files * * @return string The previous file link format */ From 17f1f079b2cec445e1e10b4f9dc936534b8b029d Mon Sep 17 00:00:00 2001 From: Roland Franssen Date: Mon, 27 Mar 2017 22:18:06 +0200 Subject: [PATCH 05/17] [Console] Revised exception rendering --- src/Symfony/Component/Console/Application.php | 13 ++++++------- .../Component/Console/Tests/ApplicationTest.php | 16 ++++++++++++++++ .../application_renderexception_escapeslines.txt | 9 +++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) create mode 100644 src/Symfony/Component/Console/Tests/Fixtures/application_renderexception_escapeslines.txt diff --git a/src/Symfony/Component/Console/Application.php b/src/Symfony/Component/Console/Application.php index 32bd8373db..bc5e9ee66c 100644 --- a/src/Symfony/Component/Console/Application.php +++ b/src/Symfony/Component/Console/Application.php @@ -650,12 +650,11 @@ class Application if (defined('HHVM_VERSION') && $width > 1 << 31) { $width = 1 << 31; } - $formatter = $output->getFormatter(); $lines = array(); - foreach (preg_split('/\r?\n/', OutputFormatter::escape($e->getMessage())) as $line) { + foreach (preg_split('/\r?\n/', $e->getMessage()) as $line) { foreach ($this->splitStringByWidth($line, $width - 4) as $line) { // pre-format lines to get the right string length - $lineLength = $this->stringWidth(preg_replace('/\[[^m]*m/', '', $formatter->format($line))) + 4; + $lineLength = $this->stringWidth($line) + 4; $lines[] = array($line, $lineLength); $len = max($lineLength, $len); @@ -663,15 +662,15 @@ class Application } $messages = array(); - $messages[] = $emptyLine = $formatter->format(sprintf('%s', str_repeat(' ', $len))); - $messages[] = $formatter->format(sprintf('%s%s', $title, str_repeat(' ', max(0, $len - $this->stringWidth($title))))); + $messages[] = $emptyLine = sprintf('%s', str_repeat(' ', $len)); + $messages[] = sprintf('%s%s', $title, str_repeat(' ', max(0, $len - $this->stringWidth($title)))); foreach ($lines as $line) { - $messages[] = $formatter->format(sprintf(' %s %s', $line[0], str_repeat(' ', $len - $line[1]))); + $messages[] = sprintf(' %s %s', OutputFormatter::escape($line[0]), str_repeat(' ', $len - $line[1])); } $messages[] = $emptyLine; $messages[] = ''; - $output->writeln($messages, OutputInterface::OUTPUT_RAW); + $output->writeln($messages); if (OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) { $output->writeln('Exception trace:'); diff --git a/src/Symfony/Component/Console/Tests/ApplicationTest.php b/src/Symfony/Component/Console/Tests/ApplicationTest.php index 895fdb9261..df6d1976e7 100644 --- a/src/Symfony/Component/Console/Tests/ApplicationTest.php +++ b/src/Symfony/Component/Console/Tests/ApplicationTest.php @@ -591,6 +591,22 @@ class ApplicationTest extends TestCase $this->assertStringEqualsFile(self::$fixturesPath.'/application_renderexception_doublewidth2.txt', $tester->getDisplay(true), '->renderException() wraps messages when they are bigger than the terminal'); } + public function testRenderExceptionEscapesLines() + { + $application = $this->getMockBuilder('Symfony\Component\Console\Application')->setMethods(array('getTerminalWidth'))->getMock(); + $application->setAutoExit(false); + $application->expects($this->any()) + ->method('getTerminalWidth') + ->will($this->returnValue(22)); + $application->register('foo')->setCode(function () { + throw new \Exception('dont break here !'); + }); + $tester = new ApplicationTester($application); + + $tester->run(array('command' => 'foo'), array('decorated' => false)); + $this->assertStringEqualsFile(self::$fixturesPath.'/application_renderexception_escapeslines.txt', $tester->getDisplay(true), '->renderException() escapes lines containing formatting'); + } + public function testRun() { $application = new Application(); diff --git a/src/Symfony/Component/Console/Tests/Fixtures/application_renderexception_escapeslines.txt b/src/Symfony/Component/Console/Tests/Fixtures/application_renderexception_escapeslines.txt new file mode 100644 index 0000000000..cf79b37a92 --- /dev/null +++ b/src/Symfony/Component/Console/Tests/Fixtures/application_renderexception_escapeslines.txt @@ -0,0 +1,9 @@ + + + [Exception] + dont break here < + info>! + + +foo + From 53ecf8393e976dd1585e9230e0cd4d1997d31760 Mon Sep 17 00:00:00 2001 From: Roland Franssen Date: Wed, 29 Mar 2017 15:52:26 +0200 Subject: [PATCH 06/17] [Console] Fix table cell styling --- .../Component/Console/Helper/Helper.php | 7 ++++- .../Component/Console/Helper/Table.php | 11 ++++--- .../Console/Tests/Helper/TableTest.php | 29 +++++++++++++++++++ 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Console/Helper/Helper.php b/src/Symfony/Component/Console/Helper/Helper.php index 90979d5b9d..b429e630c7 100644 --- a/src/Symfony/Component/Console/Helper/Helper.php +++ b/src/Symfony/Component/Console/Helper/Helper.php @@ -109,6 +109,11 @@ abstract class Helper implements HelperInterface } public static function strlenWithoutDecoration(OutputFormatterInterface $formatter, $string) + { + return self::strlen(self::removeDecoration($formatter, $string)); + } + + public static function removeDecoration(OutputFormatterInterface $formatter, $string) { $isDecorated = $formatter->isDecorated(); $formatter->setDecorated(false); @@ -118,6 +123,6 @@ abstract class Helper implements HelperInterface $string = preg_replace("/\033\[[^m]*m/", '', $string); $formatter->setDecorated($isDecorated); - return self::strlen($string); + return $string; } } diff --git a/src/Symfony/Component/Console/Helper/Table.php b/src/Symfony/Component/Console/Helper/Table.php index 8728e33d83..11bed884c2 100644 --- a/src/Symfony/Component/Console/Helper/Table.php +++ b/src/Symfony/Component/Console/Helper/Table.php @@ -341,7 +341,7 @@ class Table if (!strstr($cell, "\n")) { continue; } - $lines = explode("\n", $cell); + $lines = explode("\n", str_replace("\n", "\n", $cell)); foreach ($lines as $lineKey => $line) { if ($cell instanceof TableCell) { $line = new TableCell($line, array('colspan' => $cell->getColspan())); @@ -382,7 +382,7 @@ class Table $nbLines = $cell->getRowspan() - 1; $lines = array($cell); if (strstr($cell, "\n")) { - $lines = explode("\n", $cell); + $lines = explode("\n", str_replace("\n", "\n", $cell)); $nbLines = count($lines) > $nbLines ? substr_count($cell, "\n") : $nbLines; $rows[$line][$column] = new TableCell($lines[0], array('colspan' => $cell->getColspan())); @@ -514,6 +514,8 @@ class Table return $this->columnWidths[$column]; } + $lengths = array(); + foreach (array_merge($this->headers, $this->rows) as $row) { if ($row instanceof TableSeparator) { continue; @@ -521,9 +523,10 @@ class Table foreach ($row as $i => $cell) { if ($cell instanceof TableCell) { - $textLength = Helper::strlenWithoutDecoration($this->output->getFormatter(), $cell); + $textContent = Helper::removeDecoration($this->output->getFormatter(), $cell); + $textLength = Helper::strlen($textContent); if ($textLength > 0) { - $contentColumns = str_split($cell, ceil($textLength / $cell->getColspan())); + $contentColumns = str_split($textContent, ceil($textLength / $cell->getColspan())); foreach ($contentColumns as $position => $content) { $row[$i + $position] = $content; } diff --git a/src/Symfony/Component/Console/Tests/Helper/TableTest.php b/src/Symfony/Component/Console/Tests/Helper/TableTest.php index 5fe6b64560..0e984324e7 100644 --- a/src/Symfony/Component/Console/Tests/Helper/TableTest.php +++ b/src/Symfony/Component/Console/Tests/Helper/TableTest.php @@ -511,6 +511,35 @@ TABLE | Dante Alighieri | J. R. R. Tolkien | J. R. R | +-----------------+------------------+---------+ +TABLE + , + true, + ), + 'Row with formatted cells containing a newline' => array( + array(), + array( + array( + new TableCell('Dont break'."\n".'here', array('colspan' => 2)), + ), + new TableSeparator(), + array( + 'foo', + new TableCell('Dont break'."\n".'here', array('rowspan' => 2)), + ), + array( + 'bar', + ), + ), + 'default', + <<<'TABLE' ++-------+------------+ +| Dont break | +| here | ++-------+------------+ +| foo | Dont break | +| bar | here | ++-------+------------+ + TABLE , true, From dc55db2a9db59dacfca599ec5efc3b5e985767ba Mon Sep 17 00:00:00 2001 From: Philipp Kretzschmar Date: Wed, 27 Jul 2016 18:41:51 +0200 Subject: [PATCH 07/17] add expression text to SyntaxError fixes #19445 --- .../Component/ExpressionLanguage/Lexer.php | 5 +-- .../ExpressionLanguage/SyntaxError.php | 10 ++++-- .../ExpressionLanguage/Tests/LexerTest.php | 33 +++++++++++++++++-- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/ExpressionLanguage/Lexer.php b/src/Symfony/Component/ExpressionLanguage/Lexer.php index 26bb51d42e..3d3aba25ad 100644 --- a/src/Symfony/Component/ExpressionLanguage/Lexer.php +++ b/src/Symfony/Component/ExpressionLanguage/Lexer.php @@ -87,7 +87,8 @@ class Lexer $cursor += strlen($match[0]); } else { // unlexable - throw new SyntaxError(sprintf('Unexpected character "%s"', $expression[$cursor]), $cursor); + $message = sprintf('Unexpected character "%s"', $expression[$cursor]); + throw new SyntaxError($message, $cursor, $expression); } } @@ -95,7 +96,7 @@ class Lexer if (!empty($brackets)) { list($expect, $cur) = array_pop($brackets); - throw new SyntaxError(sprintf('Unclosed "%s"', $expect), $cur); + throw new SyntaxError(sprintf('Unclosed "%s"', $expect), $cur, $expression); } return new TokenStream($tokens); diff --git a/src/Symfony/Component/ExpressionLanguage/SyntaxError.php b/src/Symfony/Component/ExpressionLanguage/SyntaxError.php index d149c00768..c30f6e62f7 100644 --- a/src/Symfony/Component/ExpressionLanguage/SyntaxError.php +++ b/src/Symfony/Component/ExpressionLanguage/SyntaxError.php @@ -13,8 +13,14 @@ namespace Symfony\Component\ExpressionLanguage; class SyntaxError extends \LogicException { - public function __construct($message, $cursor = 0) + public function __construct($message, $cursor = 0, $expression = '') { - parent::__construct(sprintf('%s around position %d.', $message, $cursor)); + $message = sprintf('%s around position %d', $message, $cursor); + if ($expression) { + $message = sprintf('%s for expression "%s"', $message, $expression); + } + $message .= '.'; + + parent::__construct($message); } } diff --git a/src/Symfony/Component/ExpressionLanguage/Tests/LexerTest.php b/src/Symfony/Component/ExpressionLanguage/Tests/LexerTest.php index 4292c22359..a28537493d 100644 --- a/src/Symfony/Component/ExpressionLanguage/Tests/LexerTest.php +++ b/src/Symfony/Component/ExpressionLanguage/Tests/LexerTest.php @@ -18,14 +18,43 @@ use Symfony\Component\ExpressionLanguage\TokenStream; class LexerTest extends TestCase { + /** + * @var Lexer + */ + private $lexer; + + protected function setUp() + { + $this->lexer = new Lexer(); + } + /** * @dataProvider getTokenizeData */ public function testTokenize($tokens, $expression) { $tokens[] = new Token('end of expression', null, strlen($expression) + 1); - $lexer = new Lexer(); - $this->assertEquals(new TokenStream($tokens), $lexer->tokenize($expression)); + $this->assertEquals(new TokenStream($tokens), $this->lexer->tokenize($expression)); + } + + /** + * @expectedException Symfony\Component\ExpressionLanguage\SyntaxError + * @expectedExceptionMessage Unexpected character "'" around position 33 for expression "service(faulty.expression.example').dummyMethod()". + */ + public function testTokenizeThrowsErrorWithMessage() + { + $expression = "service(faulty.expression.example').dummyMethod()"; + $this->lexer->tokenize($expression); + } + + /** + * @expectedException Symfony\Component\ExpressionLanguage\SyntaxError + * @expectedExceptionMessage Unclosed "(" around position 7 for expression "service(unclosed.expression.dummyMethod()". + */ + public function testTokenizeThrowsErrorOnUnclosedBrace() + { + $expression = 'service(unclosed.expression.dummyMethod()'; + $this->lexer->tokenize($expression); } public function getTokenizeData() From fb4d8de5e4c99d074d6b8f67f9251671014a931b Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Fri, 31 Mar 2017 10:23:48 +0200 Subject: [PATCH 08/17] Avoid forcing to define the choices_as_values option when using choice_loader When using the choice loader, choices are ignored entirely. Forcing the dev to add the choices_as_values just to avoid the deprecation warning (and then to remove the option again at some point in 3.x due to deprecation) is a bad developer experience. --- src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index 26f7c80f17..61907067f5 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -357,7 +357,7 @@ class ChoiceType extends AbstractType }; $choicesAsValuesNormalizer = function (Options $options, $choicesAsValues) use ($that) { - if (true !== $choicesAsValues) { + if (true !== $choicesAsValues && null === $options['choice_loader']) { @trigger_error(sprintf('The value "false" for the "choices_as_values" option of the "%s" form type (%s) is deprecated since version 2.8 and will not be supported anymore in 3.0. Set this option to "true" and flip the contents of the "choices" option instead.', $that->getName(), __CLASS__), E_USER_DEPRECATED); } From 071548090bbd098347729ade45c020c5cc7bfcef Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Fri, 31 Mar 2017 14:24:41 +0200 Subject: [PATCH 09/17] Fix tests expecting a valid date --- .../Component/Form/Tests/Extension/Core/Type/DateTypeTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTypeTest.php index d907c9c3d1..05fc63f055 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTypeTest.php @@ -598,7 +598,7 @@ class DateTypeTest extends BaseTypeTest )); $form->submit(array( - 'day' => '0', + 'day' => '1', 'month' => '6', 'year' => '2010', )); From 9a1915f88bf3ffc6b40a6342ff604c5eff368223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Ostroluck=C3=BD?= Date: Sat, 1 Apr 2017 00:21:40 +0200 Subject: [PATCH 10/17] [EventDispatcher] Remove unneded count() --- src/Symfony/Component/EventDispatcher/EventDispatcher.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/EventDispatcher/EventDispatcher.php b/src/Symfony/Component/EventDispatcher/EventDispatcher.php index 87aca2d480..d54ecad6bd 100644 --- a/src/Symfony/Component/EventDispatcher/EventDispatcher.php +++ b/src/Symfony/Component/EventDispatcher/EventDispatcher.php @@ -80,7 +80,7 @@ class EventDispatcher implements EventDispatcherInterface */ public function hasListeners($eventName = null) { - return (bool) count($this->getListeners($eventName)); + return (bool) $this->getListeners($eventName); } /** From 75d5cb1bad123378a0b2202647dd172609a7a541 Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Sun, 2 Apr 2017 15:23:09 +0200 Subject: [PATCH 11/17] Disable resource tracking if the config component is missing --- .../Component/DependencyInjection/ContainerBuilder.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 496adb9426..5bafa94398 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -26,6 +26,7 @@ use Symfony\Component\Config\Resource\FileResource; use Symfony\Component\Config\Resource\ResourceInterface; use Symfony\Component\DependencyInjection\LazyProxy\Instantiator\InstantiatorInterface; use Symfony\Component\DependencyInjection\LazyProxy\Instantiator\RealServiceInstantiator; +use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; use Symfony\Component\ExpressionLanguage\Expression; use Symfony\Component\ExpressionLanguage\ExpressionFunctionProviderInterface; @@ -73,7 +74,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface */ private $compiler; - private $trackResources = true; + private $trackResources; /** * @var InstantiatorInterface|null @@ -90,6 +91,13 @@ class ContainerBuilder extends Container implements TaggedContainerInterface */ private $expressionLanguageProviders = array(); + public function __construct(ParameterBagInterface $parameterBag = null) + { + parent::__construct($parameterBag); + + $this->trackResources = interface_exists('Symfony\Component\Config\Resource\ResourceInterface'); + } + /** * Sets the track resources flag. * From 0f623f422019fb450feb2fda0c1aac91b9c6d162 Mon Sep 17 00:00:00 2001 From: Dariusz Ruminski Date: Fri, 31 Mar 2017 11:34:31 +0200 Subject: [PATCH 12/17] CS: Remove invisible chars --- .../DataTransformer/PercentToLocalizedStringTransformer.php | 2 +- .../Component/Form/Extension/Validator/ValidatorExtension.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Core/DataTransformer/PercentToLocalizedStringTransformer.php b/src/Symfony/Component/Form/Extension/Core/DataTransformer/PercentToLocalizedStringTransformer.php index 88531feaa2..7fc191a054 100644 --- a/src/Symfony/Component/Form/Extension/Core/DataTransformer/PercentToLocalizedStringTransformer.php +++ b/src/Symfony/Component/Form/Extension/Core/DataTransformer/PercentToLocalizedStringTransformer.php @@ -120,7 +120,7 @@ class PercentToLocalizedStringTransformer implements DataTransformerInterface $formatter = $this->getNumberFormatter(); // replace normal spaces so that the formatter can read them - $value = $formatter->parse(str_replace(' ', ' ', $value)); + $value = $formatter->parse(str_replace(' ', "\xc2\xa0", $value)); if (intl_is_failure($formatter->getErrorCode())) { throw new TransformationFailedException($formatter->getErrorMessage()); diff --git a/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php b/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php index e7ed95c459..5548493e8c 100644 --- a/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php +++ b/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php @@ -59,7 +59,7 @@ class ValidatorExtension extends AbstractExtension public function loadTypeGuesser() { - // 2.5 API + // 2.5 API if ($this->validator instanceof ValidatorInterface) { return new ValidatorTypeGuesser($this->validator); } From 7cd744133d74a976f69a33d0dbc1d5983dc30642 Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Fri, 31 Mar 2017 10:01:13 +0200 Subject: [PATCH 13/17] Complete the injection of the expression in all syntax errors --- .../Component/ExpressionLanguage/Lexer.php | 9 ++++---- .../Component/ExpressionLanguage/Parser.php | 14 ++++++------- .../ExpressionLanguage/SyntaxError.php | 2 +- .../ExpressionLanguage/Tests/LexerTest.php | 10 ++++----- .../ExpressionLanguage/Tests/ParserTest.php | 4 ++-- .../ExpressionLanguage/TokenStream.php | 21 +++++++++++++++---- 6 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/Symfony/Component/ExpressionLanguage/Lexer.php b/src/Symfony/Component/ExpressionLanguage/Lexer.php index 3d3aba25ad..8c10b72d86 100644 --- a/src/Symfony/Component/ExpressionLanguage/Lexer.php +++ b/src/Symfony/Component/ExpressionLanguage/Lexer.php @@ -59,12 +59,12 @@ class Lexer } elseif (false !== strpos(')]}', $expression[$cursor])) { // closing bracket if (empty($brackets)) { - throw new SyntaxError(sprintf('Unexpected "%s"', $expression[$cursor]), $cursor); + throw new SyntaxError(sprintf('Unexpected "%s"', $expression[$cursor]), $cursor, $expression); } list($expect, $cur) = array_pop($brackets); if ($expression[$cursor] != strtr($expect, '([{', ')]}')) { - throw new SyntaxError(sprintf('Unclosed "%s"', $expect), $cur); + throw new SyntaxError(sprintf('Unclosed "%s"', $expect), $cur, $expression); } $tokens[] = new Token(Token::PUNCTUATION_TYPE, $expression[$cursor], $cursor + 1); @@ -87,8 +87,7 @@ class Lexer $cursor += strlen($match[0]); } else { // unlexable - $message = sprintf('Unexpected character "%s"', $expression[$cursor]); - throw new SyntaxError($message, $cursor, $expression); + throw new SyntaxError(sprintf('Unexpected character "%s"', $expression[$cursor]), $cursor, $expression); } } @@ -99,6 +98,6 @@ class Lexer throw new SyntaxError(sprintf('Unclosed "%s"', $expect), $cur, $expression); } - return new TokenStream($tokens); + return new TokenStream($tokens, $expression); } } diff --git a/src/Symfony/Component/ExpressionLanguage/Parser.php b/src/Symfony/Component/ExpressionLanguage/Parser.php index f121ad9a9c..6f90451522 100644 --- a/src/Symfony/Component/ExpressionLanguage/Parser.php +++ b/src/Symfony/Component/ExpressionLanguage/Parser.php @@ -99,7 +99,7 @@ class Parser $node = $this->parseExpression(); if (!$stream->isEOF()) { - throw new SyntaxError(sprintf('Unexpected token "%s" of value "%s"', $stream->current->type, $stream->current->value), $stream->current->cursor); + throw new SyntaxError(sprintf('Unexpected token "%s" of value "%s"', $stream->current->type, $stream->current->value), $stream->current->cursor, $stream->getExpression()); } return $node; @@ -195,13 +195,13 @@ class Parser default: if ('(' === $this->stream->current->value) { if (false === isset($this->functions[$token->value])) { - throw new SyntaxError(sprintf('The function "%s" does not exist', $token->value), $token->cursor); + throw new SyntaxError(sprintf('The function "%s" does not exist', $token->value), $token->cursor, $this->stream->getExpression()); } $node = new Node\FunctionNode($token->value, $this->parseArguments()); } else { if (!in_array($token->value, $this->names, true)) { - throw new SyntaxError(sprintf('Variable "%s" is not valid', $token->value), $token->cursor); + throw new SyntaxError(sprintf('Variable "%s" is not valid', $token->value), $token->cursor, $this->stream->getExpression()); } // is the name used in the compiled code different @@ -227,7 +227,7 @@ class Parser } elseif ($token->test(Token::PUNCTUATION_TYPE, '{')) { $node = $this->parseHashExpression(); } else { - throw new SyntaxError(sprintf('Unexpected token "%s" of value "%s"', $token->type, $token->value), $token->cursor); + throw new SyntaxError(sprintf('Unexpected token "%s" of value "%s"', $token->type, $token->value), $token->cursor, $this->stream->getExpression()); } } @@ -289,7 +289,7 @@ class Parser } else { $current = $this->stream->current; - throw new SyntaxError(sprintf('A hash key must be a quoted string, a number, a name, or an expression enclosed in parentheses (unexpected token "%s" of value "%s"', $current->type, $current->value), $current->cursor); + throw new SyntaxError(sprintf('A hash key must be a quoted string, a number, a name, or an expression enclosed in parentheses (unexpected token "%s" of value "%s"', $current->type, $current->value), $current->cursor, $this->stream->getExpression()); } $this->stream->expect(Token::PUNCTUATION_TYPE, ':', 'A hash key must be followed by a colon (:)'); @@ -327,7 +327,7 @@ class Parser // As a result, if $token is NOT an operator OR $token->value is NOT a valid property or method name, an exception shall be thrown. ($token->type !== Token::OPERATOR_TYPE || !preg_match('/[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*/A', $token->value)) ) { - throw new SyntaxError('Expected name', $token->cursor); + throw new SyntaxError('Expected name', $token->cursor, $this->stream->getExpression()); } $arg = new Node\ConstantNode($token->value); @@ -345,7 +345,7 @@ class Parser $node = new Node\GetAttrNode($node, $arg, $arguments, $type); } elseif ('[' === $token->value) { if ($node instanceof Node\GetAttrNode && Node\GetAttrNode::METHOD_CALL === $node->attributes['type'] && PHP_VERSION_ID < 50400) { - throw new SyntaxError('Array calls on a method call is only supported on PHP 5.4+', $token->cursor); + throw new SyntaxError('Array calls on a method call is only supported on PHP 5.4+', $token->cursor, $this->stream->getExpression()); } $this->stream->next(); diff --git a/src/Symfony/Component/ExpressionLanguage/SyntaxError.php b/src/Symfony/Component/ExpressionLanguage/SyntaxError.php index c30f6e62f7..9373e9980b 100644 --- a/src/Symfony/Component/ExpressionLanguage/SyntaxError.php +++ b/src/Symfony/Component/ExpressionLanguage/SyntaxError.php @@ -17,7 +17,7 @@ class SyntaxError extends \LogicException { $message = sprintf('%s around position %d', $message, $cursor); if ($expression) { - $message = sprintf('%s for expression "%s"', $message, $expression); + $message = sprintf('%s for expression `%s`', $message, $expression); } $message .= '.'; diff --git a/src/Symfony/Component/ExpressionLanguage/Tests/LexerTest.php b/src/Symfony/Component/ExpressionLanguage/Tests/LexerTest.php index a28537493d..87c16f707b 100644 --- a/src/Symfony/Component/ExpressionLanguage/Tests/LexerTest.php +++ b/src/Symfony/Component/ExpressionLanguage/Tests/LexerTest.php @@ -34,12 +34,12 @@ class LexerTest extends TestCase public function testTokenize($tokens, $expression) { $tokens[] = new Token('end of expression', null, strlen($expression) + 1); - $this->assertEquals(new TokenStream($tokens), $this->lexer->tokenize($expression)); + $this->assertEquals(new TokenStream($tokens, $expression), $this->lexer->tokenize($expression)); } /** - * @expectedException Symfony\Component\ExpressionLanguage\SyntaxError - * @expectedExceptionMessage Unexpected character "'" around position 33 for expression "service(faulty.expression.example').dummyMethod()". + * @expectedException \Symfony\Component\ExpressionLanguage\SyntaxError + * @expectedExceptionMessage Unexpected character "'" around position 33 for expression `service(faulty.expression.example').dummyMethod()`. */ public function testTokenizeThrowsErrorWithMessage() { @@ -48,8 +48,8 @@ class LexerTest extends TestCase } /** - * @expectedException Symfony\Component\ExpressionLanguage\SyntaxError - * @expectedExceptionMessage Unclosed "(" around position 7 for expression "service(unclosed.expression.dummyMethod()". + * @expectedException \Symfony\Component\ExpressionLanguage\SyntaxError + * @expectedExceptionMessage Unclosed "(" around position 7 for expression `service(unclosed.expression.dummyMethod()`. */ public function testTokenizeThrowsErrorOnUnclosedBrace() { diff --git a/src/Symfony/Component/ExpressionLanguage/Tests/ParserTest.php b/src/Symfony/Component/ExpressionLanguage/Tests/ParserTest.php index 8996d5bfa0..b4cfff6d1e 100644 --- a/src/Symfony/Component/ExpressionLanguage/Tests/ParserTest.php +++ b/src/Symfony/Component/ExpressionLanguage/Tests/ParserTest.php @@ -20,7 +20,7 @@ class ParserTest extends TestCase { /** * @expectedException \Symfony\Component\ExpressionLanguage\SyntaxError - * @expectedExceptionMessage Variable "foo" is not valid around position 1. + * @expectedExceptionMessage Variable "foo" is not valid around position 1 for expression `foo`. */ public function testParseWithInvalidName() { @@ -31,7 +31,7 @@ class ParserTest extends TestCase /** * @expectedException \Symfony\Component\ExpressionLanguage\SyntaxError - * @expectedExceptionMessage Variable "foo" is not valid around position 1. + * @expectedExceptionMessage Variable "foo" is not valid around position 1 for expression `foo`. */ public function testParseWithZeroInNames() { diff --git a/src/Symfony/Component/ExpressionLanguage/TokenStream.php b/src/Symfony/Component/ExpressionLanguage/TokenStream.php index 6c4af745b2..3c22fc1d46 100644 --- a/src/Symfony/Component/ExpressionLanguage/TokenStream.php +++ b/src/Symfony/Component/ExpressionLanguage/TokenStream.php @@ -22,16 +22,19 @@ class TokenStream private $tokens; private $position = 0; + private $expression; /** * Constructor. * - * @param array $tokens An array of tokens + * @param array $tokens An array of tokens + * @param string $expression */ - public function __construct(array $tokens) + public function __construct(array $tokens, $expression = '') { $this->tokens = $tokens; $this->current = $tokens[0]; + $this->expression = $expression; } /** @@ -50,7 +53,7 @@ class TokenStream public function next() { if (!isset($this->tokens[$this->position])) { - throw new SyntaxError('Unexpected end of expression', $this->current->cursor); + throw new SyntaxError('Unexpected end of expression', $this->current->cursor, $this->expression); } ++$this->position; @@ -69,7 +72,7 @@ class TokenStream { $token = $this->current; if (!$token->test($type, $value)) { - throw new SyntaxError(sprintf('%sUnexpected token "%s" of value "%s" ("%s" expected%s)', $message ? $message.'. ' : '', $token->type, $token->value, $type, $value ? sprintf(' with value "%s"', $value) : ''), $token->cursor); + throw new SyntaxError(sprintf('%sUnexpected token "%s" of value "%s" ("%s" expected%s)', $message ? $message.'. ' : '', $token->type, $token->value, $type, $value ? sprintf(' with value "%s"', $value) : ''), $token->cursor, $this->expression); } $this->next(); } @@ -83,4 +86,14 @@ class TokenStream { return $this->current->type === Token::EOF_TYPE; } + + /** + * @internal + * + * @return string + */ + public function getExpression() + { + return $this->expression; + } } From 74ee588924cf2e88a5c64d48d6e6ea1283b77bdc Mon Sep 17 00:00:00 2001 From: Arthur de Moulins Date: Mon, 3 Apr 2017 11:21:18 +0200 Subject: [PATCH 14/17] support nullable array or collection --- .../Component/PropertyInfo/Extractor/PhpDocExtractor.php | 6 +++--- .../PropertyInfo/Tests/Extractors/PhpDocExtractorTest.php | 1 + .../Tests/Extractors/ReflectionExtractorTest.php | 1 + .../Component/PropertyInfo/Tests/Fixtures/Dummy.php | 7 +++++++ 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php b/src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php index 73e8ad3748..b6b39cc0fc 100644 --- a/src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php +++ b/src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php @@ -242,7 +242,7 @@ class PhpDocExtractor implements PropertyDescriptionExtractorInterface, Property * @param string $ucFirstProperty * @param int $type * - * @return DocBlock|null + * @return array|null */ private function getDocBlockFromMethod($class, $ucFirstProperty, $type) { @@ -337,10 +337,10 @@ class PhpDocExtractor implements PropertyDescriptionExtractorInterface, Property $collectionValueType = null; } else { $collectionKeyType = new Type(Type::BUILTIN_TYPE_INT); - $collectionValueType = new Type($phpType, false, $class); + $collectionValueType = new Type($phpType, $nullable, $class); } - return new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, $collectionKeyType, $collectionValueType); + return new Type(Type::BUILTIN_TYPE_ARRAY, $nullable, null, true, $collectionKeyType, $collectionValueType); } return new Type($phpType, $nullable, $class); diff --git a/src/Symfony/Component/PropertyInfo/Tests/Extractors/PhpDocExtractorTest.php b/src/Symfony/Component/PropertyInfo/Tests/Extractors/PhpDocExtractorTest.php index 1043997482..6477ba1967 100644 --- a/src/Symfony/Component/PropertyInfo/Tests/Extractors/PhpDocExtractorTest.php +++ b/src/Symfony/Component/PropertyInfo/Tests/Extractors/PhpDocExtractorTest.php @@ -68,6 +68,7 @@ class PhpDocExtractorTest extends TestCase array('d', array(new Type(Type::BUILTIN_TYPE_BOOL)), null, null), array('e', array(new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, new Type(Type::BUILTIN_TYPE_INT), new Type(Type::BUILTIN_TYPE_RESOURCE))), null, null), array('f', array(new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, new Type(Type::BUILTIN_TYPE_INT), new Type(Type::BUILTIN_TYPE_OBJECT, false, 'DateTime'))), null, null), + array('g', array(new Type(Type::BUILTIN_TYPE_ARRAY, true, null, true)), 'Nullable array.', null), array('donotexist', null, null, null), array('staticGetter', null, null, null), array('staticSetter', null, null, null), diff --git a/src/Symfony/Component/PropertyInfo/Tests/Extractors/ReflectionExtractorTest.php b/src/Symfony/Component/PropertyInfo/Tests/Extractors/ReflectionExtractorTest.php index 7818d0dd40..6af92affe0 100644 --- a/src/Symfony/Component/PropertyInfo/Tests/Extractors/ReflectionExtractorTest.php +++ b/src/Symfony/Component/PropertyInfo/Tests/Extractors/ReflectionExtractorTest.php @@ -38,6 +38,7 @@ class ReflectionExtractorTest extends TestCase 'parent', 'collection', 'B', + 'g', 'foo', 'foo2', 'foo3', diff --git a/src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php b/src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php index 96cb87db4a..12065b18b6 100644 --- a/src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php +++ b/src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php @@ -51,6 +51,13 @@ class Dummy extends ParentDummy */ public $B; + /** + * Nullable array. + * + * @var array|null + */ + public $g; + public static function getStatic() { } From 8c8b181f063b30b9d4df17fe6b1e9c05516525eb Mon Sep 17 00:00:00 2001 From: Dany Maillard Date: Mon, 3 Apr 2017 10:42:32 +0200 Subject: [PATCH 15/17] Lighten tests output by removing composer suggestions --- .travis.yml | 6 +++--- appveyor.yml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 088de9fec4..7205200d85 100644 --- a/.travis.yml +++ b/.travis.yml @@ -79,7 +79,7 @@ install: - export COMPOSER_ROOT_VERSION=$SYMFONY_VERSION.x-dev - if [[ ! $skip && $deps ]]; then export SYMFONY_DEPRECATIONS_HELPER=weak; fi - if [[ ! $skip && $deps ]]; then mv composer.json.phpunit composer.json; fi - - if [[ ! $skip ]]; then composer update; fi + - if [[ ! $skip ]]; then composer update --no-suggest; fi - if [[ ! $skip ]]; then ./phpunit install; fi - if [[ ! $skip && ! $PHP = hhvm* ]]; then php -i; else hhvm --php -r 'print_r($_SERVER);print_r(ini_get_all());'; fi @@ -90,5 +90,5 @@ script: - if [[ ! $deps && ! $PHP = hhvm* ]]; then echo -e "\\nRunning tests requiring tty"; $PHPUNIT --group tty; fi - if [[ ! $deps && $PHP = hhvm* ]]; then $PHPUNIT --exclude-group benchmark,intl-data; fi - if [[ ! $deps && $PHP = ${MIN_PHP%.*} ]]; then echo -e "1\\n0" | xargs -I{} sh -c 'echo "\\nPHP --enable-sigchild enhanced={}" && ENHANCE_SIGCHLD={} php-$MIN_PHP/sapi/cli/php .phpunit/phpunit-4.8/phpunit --colors=always src/Symfony/Component/Process/'; fi - - if [[ $deps = high ]]; then echo "$COMPONENTS" | parallel --gnu -j10% 'cd {}; composer update --no-progress --ansi; $PHPUNIT --exclude-group tty,benchmark,intl-data'$LEGACY"$REPORT"; fi - - if [[ $deps = low ]]; then echo "$COMPONENTS" | parallel --gnu -j10% 'cd {}; composer update --no-progress --ansi --prefer-lowest --prefer-stable; $PHPUNIT --exclude-group tty,benchmark,intl-data'"$REPORT"; fi + - if [[ $deps = high ]]; then echo "$COMPONENTS" | parallel --gnu -j10% 'cd {}; composer update --no-progress --no-suggest --ansi; $PHPUNIT --exclude-group tty,benchmark,intl-data'$LEGACY"$REPORT"; fi + - if [[ $deps = low ]]; then echo "$COMPONENTS" | parallel --gnu -j10% 'cd {}; composer update --no-progress --no-suggest --ansi --prefer-lowest --prefer-stable; $PHPUNIT --exclude-group tty,benchmark,intl-data'"$REPORT"; fi diff --git a/appveyor.yml b/appveyor.yml index 5f3953ed8a..86a943c77c 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -53,7 +53,7 @@ install: - copy /Y .composer\* %APPDATA%\Composer\ - php .github/build-packages.php "HEAD^" src\Symfony\Bridge\PhpUnit - IF %APPVEYOR_REPO_BRANCH%==master (SET COMPOSER_ROOT_VERSION=dev-master) ELSE (SET COMPOSER_ROOT_VERSION=%APPVEYOR_REPO_BRANCH%.x-dev) - - php composer.phar update --no-progress --ansi + - php composer.phar update --no-progress --no-suggest --ansi - php phpunit install test_script: From 992c6775340b53ab0e2f756ac512929581135305 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 3 Apr 2017 17:01:14 +0200 Subject: [PATCH 16/17] [DI] Don't use auto-registered services to populate type-candidates --- .../Compiler/AutowirePass.php | 56 +++++++++++-------- .../Tests/Compiler/AutowirePassTest.php | 21 ------- 2 files changed, 32 insertions(+), 45 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 9454d1bd4b..445a544f41 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -28,7 +28,7 @@ class AutowirePass implements CompilerPassInterface private $definedTypes = array(); private $types; private $notGuessableTypes = array(); - private $usedTypes = array(); + private $autowired = array(); /** * {@inheritdoc} @@ -45,15 +45,6 @@ class AutowirePass implements CompilerPassInterface $this->completeDefinition($id, $definition); } } - - foreach ($this->usedTypes as $type => $id) { - if (isset($this->usedTypes[$type]) && isset($this->notGuessableTypes[$type])) { - $classOrInterface = class_exists($type) ? 'class' : 'interface'; - $matchingServices = implode(', ', $this->types[$type]); - - throw new RuntimeException(sprintf('Unable to autowire argument of type "%s" for the service "%s". Multiple services exist for this %s (%s).', $type, $id, $classOrInterface, $matchingServices)); - } - } } catch (\Exception $e) { } catch (\Throwable $e) { } @@ -66,7 +57,7 @@ class AutowirePass implements CompilerPassInterface $this->definedTypes = array(); $this->types = null; $this->notGuessableTypes = array(); - $this->usedTypes = array(); + $this->autowired = array(); if (isset($e)) { throw $e; @@ -92,47 +83,56 @@ class AutowirePass implements CompilerPassInterface if (!$constructor = $reflectionClass->getConstructor()) { return; } + $parameters = $constructor->getParameters(); + if (method_exists('ReflectionMethod', 'isVariadic') && $constructor->isVariadic()) { + array_pop($parameters); + } $arguments = $definition->getArguments(); - foreach ($constructor->getParameters() as $index => $parameter) { + foreach ($parameters as $index => $parameter) { if (array_key_exists($index, $arguments) && '' !== $arguments[$index]) { continue; } try { if (!$typeHint = $parameter->getClass()) { + if (isset($arguments[$index])) { + continue; + } + // no default value? Then fail if (!$parameter->isOptional()) { throw new RuntimeException(sprintf('Unable to autowire argument index %d ($%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)); } - if (!array_key_exists($index, $arguments)) { - // specifically pass the default value - $arguments[$index] = $parameter->getDefaultValue(); - } + // specifically pass the default value + $arguments[$index] = $parameter->getDefaultValue(); continue; } + if (isset($this->autowired[$typeHint->name])) { + return $this->autowired[$typeHint->name] ? new Reference($this->autowired[$typeHint->name]) : null; + } + if (null === $this->types) { $this->populateAvailableTypes(); } if (isset($this->types[$typeHint->name]) && !isset($this->notGuessableTypes[$typeHint->name])) { $value = new Reference($this->types[$typeHint->name]); - $this->usedTypes[$typeHint->name] = $id; } else { try { $value = $this->createAutowiredDefinition($typeHint, $id); - $this->usedTypes[$typeHint->name] = $id; } catch (RuntimeException $e) { - if ($parameter->allowsNull()) { - $value = null; - } elseif ($parameter->isDefaultValueAvailable()) { + if ($parameter->isDefaultValueAvailable()) { $value = $parameter->getDefaultValue(); + } elseif ($parameter->allowsNull()) { + $value = null; } else { throw $e; } + $this->autowired[$typeHint->name] = false; } } } catch (\ReflectionException $e) { @@ -148,6 +148,16 @@ class AutowirePass implements CompilerPassInterface $arguments[$index] = $value; } + if ($parameters && !isset($arguments[++$index])) { + while (0 <= --$index) { + $parameter = $parameters[$index]; + if (!$parameter->isDefaultValueAvailable() || $parameter->getDefaultValue() !== $arguments[$index]) { + break; + } + unset($arguments[$index]); + } + } + // it's possible index 1 was set, then index 0, then 2, etc // make sure that we re-order so they're injected as expected ksort($arguments); @@ -252,13 +262,11 @@ class AutowirePass implements CompilerPassInterface throw new RuntimeException(sprintf('Unable to autowire argument of type "%s" for the service "%s". No services were found matching this %s and it cannot be auto-registered.', $typeHint->name, $id, $classOrInterface)); } - $argumentId = sprintf('autowired.%s', $typeHint->name); + $this->autowired[$typeHint->name] = $argumentId = sprintf('autowired.%s', $typeHint->name); $argumentDefinition = $this->container->register($argumentId, $typeHint->name); $argumentDefinition->setPublic(false); - $this->populateAvailableType($argumentId, $argumentDefinition); - try { $this->completeDefinition($argumentId, $argumentDefinition); } catch (RuntimeException $e) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index 047801ceba..9a41fa2ee0 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -422,10 +422,6 @@ class AutowirePassTest extends TestCase array( new Reference('a'), new Reference('lille'), - // third arg shouldn't *need* to be passed - // but that's hard to "pull of" with autowiring, so - // this assumes passing the default val is ok - 'some_val', ), $definition->getArguments() ); @@ -462,23 +458,6 @@ class AutowirePassTest extends TestCase $this->assertEquals(array(new Reference('a'), '', new Reference('lille')), $container->getDefinition('foo')->getArguments()); } - /** - * @dataProvider provideAutodiscoveredAutowiringOrder - * - * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException - * @expectedExceptionMEssage Unable to autowire argument of type "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" for the service "a". Multiple services exist for this interface (autowired.Symfony\Component\DependencyInjection\Tests\Compiler\CollisionA, autowired.Symfony\Component\DependencyInjection\Tests\Compiler\CollisionB). - */ - public function testAutodiscoveredAutowiringOrder($class) - { - $container = new ContainerBuilder(); - - $container->register('a', __NAMESPACE__.'\\'.$class) - ->setAutowired(true); - - $pass = new AutowirePass(); - $pass->process($container); - } - public function provideAutodiscoveredAutowiringOrder() { return array( From 9b601633a724bf50d8479bf70e46bd33dc19ae5c Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 3 Apr 2017 20:00:43 +0200 Subject: [PATCH 17/17] [DI] Autowiring and factories are incompatible with each others --- .../Compiler/AutowirePass.php | 4 ++++ .../Tests/Compiler/AutowirePassTest.php | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 9454d1bd4b..8acf2856d5 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -83,6 +83,10 @@ class AutowirePass implements CompilerPassInterface */ private function completeDefinition($id, Definition $definition) { + if ($definition->getFactory() || $definition->getFactoryClass(false) || $definition->getFactoryService(false)) { + throw new RuntimeException(sprintf('Service "%s" can use either autowiring or a factory, not both.', $id)); + } + if (!$reflectionClass = $this->getReflectionClass($id, $definition)) { return; } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index 047801ceba..6c17fcd66b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -486,6 +486,22 @@ class AutowirePassTest extends TestCase array('CannotBeAutowiredReverseOrder'), ); } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException + * @expectedExceptionMessage Service "a" can use either autowiring or a factory, not both. + */ + public function testWithFactory() + { + $container = new ContainerBuilder(); + + $container->register('a', __NAMESPACE__.'\A') + ->setFactory('foo') + ->setAutowired(true); + + $pass = new AutowirePass(); + $pass->process($container); + } } class Foo