From 29916394f8544d495240207fbec57ac5c9a616aa Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Fri, 12 Feb 2016 17:33:32 +0100 Subject: [PATCH 1/4] [TwigBridge] Symfony 3.1 forward compatibility --- src/Symfony/Bridge/Twig/Extension/YamlExtension.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bridge/Twig/Extension/YamlExtension.php b/src/Symfony/Bridge/Twig/Extension/YamlExtension.php index 7d330c4b89..2d46795b4a 100644 --- a/src/Symfony/Bridge/Twig/Extension/YamlExtension.php +++ b/src/Symfony/Bridge/Twig/Extension/YamlExtension.php @@ -12,6 +12,7 @@ namespace Symfony\Bridge\Twig\Extension; use Symfony\Component\Yaml\Dumper as YamlDumper; +use Symfony\Component\Yaml\Yaml; /** * Provides integration of the Yaml component with Twig. @@ -40,7 +41,7 @@ class YamlExtension extends \Twig_Extension } if (defined('Symfony\Component\Yaml\Yaml::DUMP_OBJECT')) { - $dumpObjects = (int) $dumpObjects; + return $dumper->dump($input, $inline, 0, is_bool($dumpObjects) ? Yaml::DUMP_OBJECT : 0); } return $dumper->dump($input, $inline, 0, false, $dumpObjects); From f005c80bc5244a4b4eceae263adc410355e6cbe9 Mon Sep 17 00:00:00 2001 From: Alessandro Chitolina Date: Tue, 22 Dec 2015 08:14:31 +0100 Subject: [PATCH 2/4] [Form] Fixed violation mapping if multiple forms are using the same (or part of the same) property path --- .../ViolationMapper/ViolationMapper.php | 57 +++++++------------ .../ViolationMapper/ViolationMapperTest.php | 21 +++++++ 2 files changed, 42 insertions(+), 36 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php index 7686809588..bef225c422 100644 --- a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php +++ b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php @@ -147,12 +147,9 @@ class ViolationMapper implements ViolationMapperInterface */ private function matchChild(FormInterface $form, PropertyPathIteratorInterface $it) { - // Remember at what property path underneath "data" - // we are looking. Check if there is a child with that - // path, otherwise increase path by one more piece + $target = null; $chunk = ''; - $foundChild = null; - $foundAtIndex = 0; + $foundAtIndex = null; // Construct mapping rules for the given form $rules = array(); @@ -164,17 +161,11 @@ class ViolationMapper implements ViolationMapperInterface } } - // Skip forms inheriting their parent data when iterating the children - $childIterator = new \RecursiveIteratorIterator( + $children = iterator_to_array(new \RecursiveIteratorIterator( new InheritDataAwareIterator($form) - ); - - // Make the path longer until we find a matching child - while (true) { - if (!$it->valid()) { - return; - } + )); + while ($it->valid()) { if ($it->isIndex()) { $chunk .= '['.$it->current().']'; } else { @@ -196,33 +187,27 @@ class ViolationMapper implements ViolationMapperInterface } } - // Test children unless we already found one - if (null === $foundChild) { - foreach ($childIterator as $child) { - /* @var FormInterface $child */ - $childPath = (string) $child->getPropertyPath(); - - // Child found, mark as return value - if ($chunk === $childPath) { - $foundChild = $child; - $foundAtIndex = $it->key(); - } + /** @var FormInterface $child */ + foreach ($children as $key => $child) { + $childPath = (string) $child->getPropertyPath(); + if ($childPath === $chunk) { + $target = $child; + $foundAtIndex = $it->key(); + } elseif (0 === strpos($childPath, $chunk)) { + continue; } + + unset($children[$key]); } - // Add element to the chunk $it->next(); - - // If we reached the end of the path or if there are no - // more matching mapping rules, return the found child - if (null !== $foundChild && (!$it->valid() || count($rules) === 0)) { - // Reset index in case we tried to find mapping - // rules further down the path - $it->seek($foundAtIndex); - - return $foundChild; - } } + + if (null !== $foundAtIndex) { + $it->seek($foundAtIndex); + } + + return $target; } /** diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php index e4e0f9cc48..f6ad34eccb 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php @@ -1474,4 +1474,25 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName.' should have an error, but has none'); } } + + public function testBacktrackIfSeveralSubFormsWithSamePropertyPath() + { + $violation = $this->getConstraintViolation('data.address[street]'); + $parent = $this->getForm('parent'); + $child1 = $this->getForm('subform1', 'address'); + $child2 = $this->getForm('subform2', 'address'); + $grandChild = $this->getForm('street'); + + $parent->add($child1); + $parent->add($child2); + $child2->add($grandChild); + + $this->mapper->mapViolation($violation, $parent); + + // The error occurred on the child of the second form with the same path + $this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one'); + $this->assertCount(0, $child1->getErrors(), $child1->getName().' should not have an error, but has one'); + $this->assertCount(0, $child2->getErrors(), $child2->getName().' should not have an error, but has one'); + $this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChild->getName().' should have an error, but has none'); + } } From 3c0615141eb7404ec82967e18b342a0a7ff28a29 Mon Sep 17 00:00:00 2001 From: Tomas Liubinas Date: Thu, 21 Jan 2016 20:00:49 +0200 Subject: [PATCH 3/4] Remove InputOption::VALUE_REQUIRED mode from $default parameter description as InputOption::setDefault() throws an exception only when called in InputOption::VALUE_NONE mode. In practice the $default value could still be accessed in InputOption::VALUE_REQUIRED mode in case InputOption was never set but accessed from InputDefinition::getOption() method --- src/Symfony/Component/Console/Command/Command.php | 2 +- src/Symfony/Component/Console/Input/InputOption.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Console/Command/Command.php b/src/Symfony/Component/Console/Command/Command.php index bd899158c4..3d3f2c14d7 100644 --- a/src/Symfony/Component/Console/Command/Command.php +++ b/src/Symfony/Component/Console/Command/Command.php @@ -370,7 +370,7 @@ class Command * @param string $shortcut The shortcut (can be null) * @param int $mode The option mode: One of the InputOption::VALUE_* constants * @param string $description A description text - * @param mixed $default The default value (must be null for InputOption::VALUE_REQUIRED or InputOption::VALUE_NONE) + * @param mixed $default The default value (must be null for InputOption::VALUE_NONE) * * @return Command The current instance */ diff --git a/src/Symfony/Component/Console/Input/InputOption.php b/src/Symfony/Component/Console/Input/InputOption.php index 167f19901a..26773ca599 100644 --- a/src/Symfony/Component/Console/Input/InputOption.php +++ b/src/Symfony/Component/Console/Input/InputOption.php @@ -36,7 +36,7 @@ class InputOption * @param string|array $shortcut The shortcuts, can be null, a string of shortcuts delimited by | or an array of shortcuts * @param int $mode The option mode: One of the VALUE_* constants * @param string $description A description text - * @param mixed $default The default value (must be null for self::VALUE_REQUIRED or self::VALUE_NONE) + * @param mixed $default The default value (must be null for self::VALUE_NONE) * * @throws \InvalidArgumentException If option mode is invalid or incompatible */ From 30388f17ffc6b02ca592662cedae86025cf1dd21 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Mon, 15 Feb 2016 18:40:28 +0100 Subject: [PATCH 4/4] [DependencyInjection] fix dumped YAML snytax --- .../DependencyInjection/Dumper/YamlDumper.php | 4 ++-- .../Tests/Dumper/YamlDumperTest.php | 15 +++++++++------ .../Tests/Fixtures/yaml/services10.yml | 2 +- .../Tests/Fixtures/yaml/services6.yml | 2 +- .../Tests/Fixtures/yaml/services9.yml | 8 ++++---- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php index eca056dea1..84ad6f872a 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php @@ -64,7 +64,7 @@ class YamlDumper extends Dumper $class = substr($class, 1); } - $code .= sprintf(" class: %s\n", $class); + $code .= sprintf(" class: %s\n", $this->dumper->dump($class)); } if (!$definition->isPublic()) { @@ -100,7 +100,7 @@ class YamlDumper extends Dumper } if ($definition->getFactoryClass()) { - $code .= sprintf(" factory_class: %s\n", $definition->getFactoryClass()); + $code .= sprintf(" factory_class: %s\n", $this->dumper->dump($definition->getFactoryClass())); } if ($definition->isLazy()) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php index f9747a7c2f..aec4109f24 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php @@ -13,6 +13,7 @@ namespace Symfony\Component\DependencyInjection\Tests\Dumper; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Dumper\YamlDumper; +use Symfony\Component\Yaml\Yaml; class YamlDumperTest extends \PHPUnit_Framework_TestCase { @@ -27,24 +28,21 @@ class YamlDumperTest extends \PHPUnit_Framework_TestCase { $dumper = new YamlDumper($container = new ContainerBuilder()); - $this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services1.yml', $dumper->dump(), '->dump() dumps an empty container as an empty YAML file'); - - $container = new ContainerBuilder(); - $dumper = new YamlDumper($container); + $this->assertEqualYamlStructure(self::$fixturesPath.'/yaml/services1.yml', $dumper->dump(), '->dump() dumps an empty container as an empty YAML file'); } public function testAddParameters() { $container = include self::$fixturesPath.'/containers/container8.php'; $dumper = new YamlDumper($container); - $this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services8.yml', $dumper->dump(), '->dump() dumps parameters'); + $this->assertEqualYamlStructure(self::$fixturesPath.'/yaml/services8.yml', $dumper->dump(), '->dump() dumps parameters'); } public function testAddService() { $container = include self::$fixturesPath.'/containers/container9.php'; $dumper = new YamlDumper($container); - $this->assertEquals(str_replace('%path%', self::$fixturesPath.DIRECTORY_SEPARATOR.'includes'.DIRECTORY_SEPARATOR, file_get_contents(self::$fixturesPath.'/yaml/services9.yml')), $dumper->dump(), '->dump() dumps services'); + $this->assertEqualYamlStructure(str_replace('%path%', self::$fixturesPath.DIRECTORY_SEPARATOR.'includes'.DIRECTORY_SEPARATOR, file_get_contents(self::$fixturesPath.'/yaml/services9.yml')), $dumper->dump(), '->dump() dumps services'); $dumper = new YamlDumper($container = new ContainerBuilder()); $container->register('foo', 'FooClass')->addArgument(new \stdClass()); @@ -56,4 +54,9 @@ class YamlDumperTest extends \PHPUnit_Framework_TestCase $this->assertEquals('Unable to dump a service container if a parameter is an object or a resource.', $e->getMessage(), '->dump() throws a RuntimeException if the container to be dumped has reference to objects or resources'); } } + + private function assertEqualYamlStructure($yaml, $expected, $message = '') + { + $this->assertEquals(Yaml::parse($expected), Yaml::parse($yaml), $message); + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services10.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services10.yml index f2f8d953d5..c66084cdbe 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services10.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services10.yml @@ -6,4 +6,4 @@ services: class: BAR project: - test: %project.parameter.foo% + test: '%project.parameter.foo%' diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml index b79697b6e9..4531cc57ee 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml @@ -5,7 +5,7 @@ services: scope.custom: { class: FooClass, scope: custom } scope.prototype: { class: FooClass, scope: prototype } constructor: { class: FooClass, factory_method: getInstance } - file: { class: FooClass, file: %path%/foo.php } + file: { class: FooClass, file: '%path%/foo.php' } arguments: { class: FooClass, arguments: [foo, '@foo', [true, false]] } configurator1: { class: FooClass, configurator: sc_configure } configurator2: { class: FooClass, configurator: ['@baz', configure] } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml index c6181ed549..ae673c1727 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml @@ -23,16 +23,16 @@ services: arguments: [foo, '@foo.baz', '%foo_bar%'] configurator: ['@foo.baz', configure] foo.baz: - class: %baz_class% - factory_class: %baz_class% + class: '%baz_class%' + factory_class: '%baz_class%' factory_method: getInstance configurator: ['%baz_class%', configureStatic1] foo_bar: - class: %foo_class% + class: '%foo_class%' scope: prototype method_call1: class: FooClass - file: %path%foo.php + file: '%path%foo.php' calls: - [setBar, ['@foo']] - [setBar, ['@?foo2']]