From 88ef52d27230c0b06d8b2e2e3e38c4934894e634 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Tue, 7 Feb 2012 10:51:21 +0100 Subject: [PATCH] [Form] Improved FormType::getDefaultOptions() to see default options defined in parent types In turn, FormType::getParent() does not see default options anymore. --- CHANGELOG-2.1.md | 2 + UPGRADE-2.1.md | 19 ++++++++ .../Doctrine/Form/Type/DoctrineType.php | 1 - .../Form/Extension/Core/Type/ChoiceType.php | 4 +- .../Form/Extension/Core/Type/DateTimeType.php | 2 +- .../Form/Extension/Core/Type/DateType.php | 2 +- .../Form/Extension/Core/Type/TimeType.php | 2 +- .../Form/Extension/Core/Type/TimezoneType.php | 2 +- src/Symfony/Component/Form/FormFactory.php | 47 ++++++++++++++----- 9 files changed, 62 insertions(+), 19 deletions(-) diff --git a/CHANGELOG-2.1.md b/CHANGELOG-2.1.md index 7f44c792e27b..02b8140b6975 100644 --- a/CHANGELOG-2.1.md +++ b/CHANGELOG-2.1.md @@ -202,6 +202,8 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c * added options "add_method" and "remove_method" to collection and choice type * forms now don't create an empty object anymore if they are completely empty and not required. The empty value for such forms is null. + * FormType::getDefaultOptions() now sees default options defined by parent types + * [BC BREAK] FormType::getParent() does not see default options anymore ### HttpFoundation diff --git a/UPGRADE-2.1.md b/UPGRADE-2.1.md index 1e166543348a..c47d5f977c3e 100644 --- a/UPGRADE-2.1.md +++ b/UPGRADE-2.1.md @@ -229,3 +229,22 @@ UPGRADE FROM 2.0 to 2.1 return false; } } + +* The options passed to `getParent` of the form types don't contain default + options anymore + + You should check with `isset` if options exist before checking their value. + + Before: + + public function getParent() + { + return 'single_text' === $options['widget'] ? 'text' : 'choice'; + } + + After: + + public function getParent() + { + return isset($options['widget']) && 'single_text' === $options['widget'] ? 'text' : 'choice'; + } diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php index 1c634fb5b086..2b9161477e24 100644 --- a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php +++ b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php @@ -50,7 +50,6 @@ public function getDefaultOptions(array $options) 'property' => null, 'query_builder' => null, 'loader' => null, - 'choices' => null, 'group_by' => null, ); diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index ff4a4b5ee9bd..447ae6a1c811 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -149,7 +149,7 @@ public function getDefaultOptions(array $options) 'multiple' => false, 'expanded' => false, 'choice_list' => null, - 'choices' => array(), + 'choices' => null, 'preferred_choices' => array(), 'value_strategy' => ChoiceList::GENERATE, 'index_strategy' => ChoiceList::GENERATE, @@ -166,7 +166,7 @@ public function getDefaultOptions(array $options) */ public function getParent(array $options) { - return $options['expanded'] ? 'form' : 'field'; + return isset($options['expanded']) && $options['expanded'] ? 'form' : 'field'; } /** diff --git a/src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php b/src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php index 1c63d216419b..7906a2ba22bc 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php @@ -200,7 +200,7 @@ public function getAllowedOptionValues(array $options) */ public function getParent(array $options) { - return 'single_text' === $options['widget'] ? 'field' : 'form'; + return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form'; } /** diff --git a/src/Symfony/Component/Form/Extension/Core/Type/DateType.php b/src/Symfony/Component/Form/Extension/Core/Type/DateType.php index b98a222e3e70..b8411fd6a3b5 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/DateType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/DateType.php @@ -211,7 +211,7 @@ public function getAllowedOptionValues(array $options) */ public function getParent(array $options) { - return 'single_text' === $options['widget'] ? 'field' : 'form'; + return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form'; } /** diff --git a/src/Symfony/Component/Form/Extension/Core/Type/TimeType.php b/src/Symfony/Component/Form/Extension/Core/Type/TimeType.php index 7bfe50dc6b81..81465cb54714 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/TimeType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/TimeType.php @@ -184,7 +184,7 @@ public function getAllowedOptionValues(array $options) */ public function getParent(array $options) { - return 'single_text' === $options['widget'] ? 'field' : 'form'; + return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form'; } /** diff --git a/src/Symfony/Component/Form/Extension/Core/Type/TimezoneType.php b/src/Symfony/Component/Form/Extension/Core/Type/TimezoneType.php index ef308faeeb50..8d1f74b3eb23 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/TimezoneType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/TimezoneType.php @@ -31,7 +31,7 @@ public function getDefaultOptions(array $options) 'value_strategy' => ChoiceList::COPY_CHOICE, ); - if (!isset($options['choice_list']) && !isset($options['choices'])) { + if (empty($options['choice_list']) && empty($options['choices'])) { $defaultOptions['choices'] = self::getTimezones(); } diff --git a/src/Symfony/Component/Form/FormFactory.php b/src/Symfony/Component/Form/FormFactory.php index 1f658303e852..e82e0567ed7c 100644 --- a/src/Symfony/Component/Form/FormFactory.php +++ b/src/Symfony/Component/Form/FormFactory.php @@ -213,16 +213,20 @@ public function createBuilder($type, $data = null, array $options = array(), For */ public function createNamedBuilder($type, $name, $data = null, array $options = array(), FormBuilder $parent = null) { - $builder = null; - $types = array(); - $knownOptions = array(); - $passedOptions = array_keys($options); - $optionValues = array(); - if (!array_key_exists('data', $options)) { $options['data'] = $data; } + $builder = null; + $types = array(); + $defaultOptions = array(); + $optionValues = array(); + $passedOptions = $options; + + // Bottom-up determination of the type hierarchy + // Start with the actual type and look for the parent type + // The complete hierarchy is saved in $types, the first entry being + // the root and the last entry being the leaf (the concrete type) while (null !== $type) { if ($type instanceof FormTypeInterface) { if ($type->getName() == $type->getParent($options)) { @@ -236,7 +240,23 @@ public function createNamedBuilder($type, $name, $data = null, array $options = throw new UnexpectedTypeException($type, 'string or Symfony\Component\Form\FormTypeInterface'); } - $defaultOptions = $type->getDefaultOptions($options); + array_unshift($types, $type); + + // getParent() cannot see default options set by this type nor + // default options set by parent types + // As a result, the options always have to be checked for + // existence with isset() before using them in this method. + $type = $type->getParent($options); + } + + // Top-down determination of the options and default options + foreach ($types as $type) { + // Merge the default options of all types to an array of default + // options. Default options of children override default options + // of parents. + // Default options of ancestors are already visible in the $options + // array passed to the following methods. + $defaultOptions = array_replace($defaultOptions, $type->getDefaultOptions($options)); $optionValues = array_merge_recursive($optionValues, $type->getAllowedOptionValues($options)); foreach ($type->getExtensions() as $typeExtension) { @@ -244,20 +264,23 @@ public function createNamedBuilder($type, $name, $data = null, array $options = $optionValues = array_merge_recursive($optionValues, $typeExtension->getAllowedOptionValues($options)); } - $options = array_replace($defaultOptions, $options); - $knownOptions = array_merge($knownOptions, array_keys($defaultOptions)); - array_unshift($types, $type); - $type = $type->getParent($options); + // In each turn, the options are replaced by the combination of + // the currently known default options and the passed options. + // It is important to merge with $passedOptions and not with + // $options, otherwise default options of parents would override + // default options of child types. + $options = array_replace($defaultOptions, $passedOptions); } $type = end($types); + $knownOptions = array_keys($defaultOptions); $diff = array_diff(self::$requiredOptions, $knownOptions); if (count($diff) > 0) { throw new TypeDefinitionException(sprintf('Type "%s" should support the option(s) "%s"', $type->getName(), implode('", "', $diff))); } - $diff = array_diff($passedOptions, $knownOptions); + $diff = array_diff(array_keys($passedOptions), $knownOptions); if (count($diff) > 1) { throw new CreationException(sprintf('The options "%s" do not exist. Known options are: "%s"', implode('", "', $diff), implode('", "', $knownOptions)));