merged branch stloyd/choicetype (PR #1371)

Commits
-------

7783a05 Removed unused code from DateType Additional tests for ChoiceType and DateType based code
cdd39ac Added ability to set "empty_value" for `DateTimeType`, `DateType` and `TimeType` Additional tests covering added code
af4a7d7 More tests and more compatible code, with some suggestions from @helmer
527b738 Test covered version of fix for issue #1336

Discussion
----------

[Form] Added ability to set "empty_value" for choice list

Hey,

This PR is similar to #1336, but this one is fully test covered and have few change in behavior:

- if choice field is not set as non-required, `empty_value` is not added automaticly,
- also `empty_value` is not set if field have option `multiple` or `expanded`,
- `empty_value` for `DateType` and `TimeType` can be set "global" or per field, i.e.:

```
$builder->add('date', 'choice', array('required' => false, 'empty_value' => array('day' => 'Choose day')));
```

- `DateType` and `TimeType` code was cleaned a bit,
- added missing option to set up choice list as required when using PHP templates

---------------------------------------------------------------------------

by stloyd at 2011/06/20 04:55:45 -0700

@fabpot I'm just not sure is that change with removing "auto-adding" of `empty_value` is good (probably BC)

---------------------------------------------------------------------------

by lenar at 2011/06/20 05:24:02 -0700

Now this is a really nice way to hijack work done by others. Really encourages newcomers. Gratz!

---------------------------------------------------------------------------

by fabpot at 2011/06/20 05:57:40 -0700

@lenar: if the code in this PR is yours (at least partly), I'm not going to merge it. @stloyd, can you clarify this issue with @lenar? Thanks.

---------------------------------------------------------------------------

by lenar at 2011/06/20 06:21:11 -0700

It's @helmer's mostly, not mine, but the issue stays.

---------------------------------------------------------------------------

by fabpot at 2011/06/20 06:26:15 -0700

No matter who the code belongs to, Git allows us to keep track of all contributors. So, we need to do our best to not loose any code ownership.

---------------------------------------------------------------------------

by helmer at 2011/06/20 06:58:03 -0700

I do not care much for ownership, just that this kind of cooperation (or lack thereof) is kind of exhausting. Closed #1336.

---------------------------------------------------------------------------

by stloyd at 2011/06/20 07:47:53 -0700

@fabpot, @lenar: This PR is inspired by #1336, made by @helmer, but after looking at his code and talking with him, we cant (IMO) get an consensus. So I wrote this PR as an another way to fix issue described in #1336.

__Summary__: I don't think this one is better than fix at #1336, it's more like another approach to fix that issue.

---------------------------------------------------------------------------

by helmer at 2011/06/20 08:15:59 -0700

@stloyd: I actually think your variant is better, so good job there, thanks.

It just ain't nice to:
1) Comment on my changes being useless due to lack of tests
2) Writing brand new testsuite from your perspective that "proves" my approach is "wrong" (while ignoring my answers, why I did something precisely like I did, which I did in sync with @fabpot comments on his first attempt to improve the issue)
3) Saying my PR is broken because your new tests against it fail
4) Changing functionality to "fix" something that was not really broken

Other than that, I wanted to contribute a few lines to improve something relatively simple, and it ended up in a huge mess with more lost hours than I planned to spend on it.

On the bright side, we ended up with something good (:

---------------------------------------------------------------------------

by stloyd at 2011/06/20 08:32:30 -0700

@helmer: 1) & 2) Sorry for that "bad language", but you get me wrong a bit. Tests was written for code in master (there was no problem to change them to work with your POV). 3) Same as before, you can adopt tests easily, but never mind. Maybe later we could cooperate better ;-)
About 4) I mentioned it in description of this PR and that was point I was disagreeing with you (also about how "default" options are adopted in fields) :-)

src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

@@ -71,6 +71,7 @@ class ChoiceType extends AbstractType
             ->setAttribute('multiple', $options['multiple'])
             ->setAttribute('expanded', $options['expanded'])
             ->setAttribute('required', $options['required'])
+            ->setAttribute('empty_value', $options['multiple'] || $options['expanded'] ? null : $options['empty_value'])
         ;
 
         if ($options['expanded']) {
@@ -106,7 +107,7 @@ class ChoiceType extends AbstractType
             ->set('preferred_choices', array_intersect_key($choices, $preferred))
             ->set('choices', array_diff_key($choices, $preferred))
             ->set('separator', '-------------------')
-            ->set('empty_value', !$form->getAttribute('multiple') && !$form->getAttribute('required') ? '' : null)
+            ->set('empty_value', $form->getAttribute('empty_value'))
         ;
 
         if ($view->get('multiple') && !$view->get('expanded')) {
@@ -132,6 +133,7 @@ class ChoiceType extends AbstractType
             'choices'           => array(),
             'preferred_choices' => array(),
             'empty_data'        => $multiple || $expanded ? array() : '',
+            'empty_value'       => ($multiple || $expanded) || !isset($options['empty_value']) ? null : '',
             'error_bubbling'    => false,
         );
     }

src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php

@@ -32,12 +32,14 @@ class DateTimeType extends AbstractType
             'years',
             'months',
             'days',
+            'empty_value',
         )));
         $timeOptions = array_intersect_key($options, array_flip(array(
             'hours',
             'minutes',
             'seconds',
             'with_seconds',
+            'empty_value',
         )));
 
         if (isset($options['date_widget'])) {
@@ -97,9 +99,9 @@ class DateTimeType extends AbstractType
     {
         return array(
             'input'         => 'datetime',
-            'with_seconds'  => false,
             'data_timezone' => null,
             'user_timezone' => null,
+            'empty_value'   => null,
             // Don't modify \DateTime classes by reference, we treat
             // them like immutable value objects
             'by_reference'  => false,

src/Symfony/Component/Form/Extension/Core/Type/DateType.php

@@ -41,32 +41,42 @@ class DateType extends AbstractType
             $builder->appendClientTransformer(new DateTimeToLocalizedStringTransformer($options['data_timezone'], $options['user_timezone'], $options['format'], \IntlDateFormatter::NONE));
         } else {
             $yearOptions = $monthOptions = $dayOptions = array();
-            $widget = $options['widget'];
 
-            if ($widget === 'choice') {
+            if ($options['widget'] === 'choice') {
+                if (is_array($options['empty_value'])) {
+                    $options['empty_value'] = array_merge(array('year' => null, 'month' => null, 'day' => null), $options['empty_value']);
+                } else {
+                    $options['empty_value'] = array('year' => $options['empty_value'], 'month' => $options['empty_value'], 'day' => $options['empty_value']);
+                }
+
                 // Only pass a subset of the options to children
                 $yearOptions = array(
                     'choice_list' => new PaddedChoiceList(
                         array_combine($options['years'], $options['years']), 4, '0', STR_PAD_LEFT
                     ),
+                    'empty_value' => $options['empty_value']['year'],
                 );
                 $monthOptions = array(
                     'choice_list' => new MonthChoiceList(
                         $formatter, $options['months']
                     ),
+                    'empty_value' => $options['empty_value']['month'],
                 );
                 $dayOptions = array(
                     'choice_list' => new PaddedChoiceList(
                         array_combine($options['days'], $options['days']), 2, '0', STR_PAD_LEFT
                     ),
+                    'empty_value' => $options['empty_value']['day'],
                 );
             }
 
             $builder
-                ->add('year', $widget, $yearOptions)
-                ->add('month', $widget, $monthOptions)
-                ->add('day', $widget, $dayOptions)
-                ->appendClientTransformer(new DateTimeToArrayTransformer($options['data_timezone'], $options['user_timezone'], array('year', 'month', 'day')))
+                ->add('year', $options['widget'], $yearOptions)
+                ->add('month', $options['widget'], $monthOptions)
+                ->add('day', $options['widget'], $dayOptions)
+                ->appendClientTransformer(new DateTimeToArrayTransformer(
+                    $options['data_timezone'], $options['user_timezone'], array('year', 'month', 'day')
+                ))
             ;
         }
 
@@ -98,7 +108,6 @@ class DateType extends AbstractType
         $view->set('widget', $form->getAttribute('widget'));
 
         if ($view->hasChildren()) {
-
             $pattern = $form->getAttribute('formatter')->getPattern();
 
             // set right order with respect to locale (e.g.: de_DE=dd.MM.yy; en_US=M/d/yy)
@@ -120,17 +129,18 @@ class DateType extends AbstractType
     public function getDefaultOptions(array $options)
     {
         return array(
-            'years'             => range(date('Y') - 5, date('Y') + 5),
-            'months'            => range(1, 12),
-            'days'              => range(1, 31),
-            'widget'            => 'choice',
-            'input'             => 'datetime',
-            'format'            => \IntlDateFormatter::MEDIUM,
-            'data_timezone'     => null,
-            'user_timezone'     => null,
+            'years'         => range(date('Y') - 5, date('Y') + 5),
+            'months'        => range(1, 12),
+            'days'          => range(1, 31),
+            'widget'        => 'choice',
+            'input'         => 'datetime',
+            'format'        => \IntlDateFormatter::MEDIUM,
+            'data_timezone' => null,
+            'user_timezone' => null,
+            'empty_value'   => null,
             // Don't modify \DateTime classes by reference, we treat
             // them like immutable value objects
-            'by_reference'      => false,
+            'by_reference'  => false,
         );
     }
 

src/Symfony/Component/Form/Extension/Core/Type/TimeType.php

@@ -28,30 +28,41 @@ class TimeType extends AbstractType
      */
     public function buildForm(FormBuilder $builder, array $options)
     {
-        $hourOptions = $minuteOptions = $secondOptions = array();
-        $parts = array('hour', 'minute');
-
         if ($options['widget'] === 'choice') {
-            $hourOptions['choice_list'] =  new PaddedChoiceList(
-                array_combine($options['hours'], $options['hours']), 2, '0', STR_PAD_LEFT
-            );
-            $minuteOptions['choice_list'] = new PaddedChoiceList(
-                array_combine($options['minutes'], $options['minutes']), 2, '0', STR_PAD_LEFT
-            );
+            if (is_array($options['empty_value'])) {
+                $options['empty_value'] = array_merge(array('hour' => null, 'minute' => null, 'second' => null), $options['empty_value']);
+            } else {
+                $options['empty_value'] = array('hour' => $options['empty_value'], 'minute' => $options['empty_value'], 'second' => $options['empty_value']);
+            }
+
+            $builder
+                ->add('hour', $options['widget'], array(
+                    'choice_list' => new PaddedChoiceList(
+                        array_combine($options['hours'], $options['hours']), 2, '0', STR_PAD_LEFT
+                    ),
+                    'empty_value' => $options['empty_value']['hour']
+                ))
+                ->add('minute', $options['widget'], array(
+                    'choice_list' => new PaddedChoiceList(
+                        array_combine($options['minutes'], $options['minutes']), 2, '0', STR_PAD_LEFT
+                    ),
+                    'empty_value' => $options['empty_value']['minute']
+                ))
+            ;
 
             if ($options['with_seconds']) {
-                $secondOptions['choice_list'] = new PaddedChoiceList(
-                    array_combine($options['seconds'], $options['seconds']), 2, '0', STR_PAD_LEFT
-                );
+                $builder->add('second', $options['widget'], array(
+                    'choice_list' => new PaddedChoiceList(
+                        array_combine($options['seconds'], $options['seconds']), 2, '0', STR_PAD_LEFT
+                    ),
+                    'empty_value' => $options['empty_value']['second']
+                ));
             }
         }
 
-        $builder->add('hour', $options['widget'], $hourOptions)
-            ->add('minute', $options['widget'], $minuteOptions);
-
+        $parts = array('hour', 'minute');
         if ($options['with_seconds']) {
             $parts[] = 'second';
-            $builder->add('second', $options['widget'], $secondOptions);
         }
 
         if ($options['input'] === 'string') {
@@ -97,17 +108,18 @@ class TimeType extends AbstractType
     public function getDefaultOptions(array $options)
     {
         return array(
-            'hours'             => range(0, 23),
-            'minutes'           => range(0, 59),
-            'seconds'           => range(0, 59),
-            'widget'            => 'choice',
-            'input'             => 'datetime',
-            'with_seconds'      => false,
-            'data_timezone'     => null,
-            'user_timezone'     => null,
+            'hours'         => range(0, 23),
+            'minutes'       => range(0, 59),
+            'seconds'       => range(0, 59),
+            'widget'        => 'choice',
+            'input'         => 'datetime',
+            'with_seconds'  => false,
+            'data_timezone' => null,
+            'user_timezone' => null,
+            'empty_value'   => null,
             // Don't modify \DateTime classes by reference, we treat
             // them like immutable value objects
-            'by_reference'      => false,
+            'by_reference'  => false,
         );
     }
 

tests/Symfony/Tests/Component/Form/AbstractLayoutTest.php

@@ -332,6 +332,7 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         $this->assertWidgetMatchesXpath($form->createView(), array(),
 '/select
     [@name="na&me"]
+    [@required="required"]
     [
         ./option[@value="&a"][@selected="selected"][.="Choice&A"]
         /following-sibling::option[@value="&b"][not(@selected)][.="Choice&B"]
@@ -354,6 +355,7 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         $this->assertWidgetMatchesXpath($form->createView(), array('separator' => '-- sep --'),
 '/select
     [@name="na&me"]
+    [@required="required"]
     [
         ./option[@value="&b"][not(@selected)][.="Choice&B"]
         /following-sibling::option[@disabled="disabled"][not(@selected)][.="-- sep --"]
@@ -377,8 +379,56 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         $this->assertWidgetMatchesXpath($form->createView(), array(),
 '/select
     [@name="na&me"]
+    [not(@required)]
     [
-        ./option[@value=""][.="[trans][/trans]"]
+        ./option[@value="&a"][@selected="selected"][.="Choice&A"]
+        /following-sibling::option[@value="&b"][not(@selected)][.="Choice&B"]
+    ]
+    [count(./option)=2]
+'
+        );
+    }
+
+    public function testSingleChoiceNonRequiredNoneSelected()
+    {
+        $form = $this->factory->createNamed('choice', 'na&me', null, array(
+            'property_path' => 'name',
+            'choices' => array('&a' => 'Choice&A', '&b' => 'Choice&B'),
+            'required' => false,
+            'multiple' => false,
+            'expanded' => false,
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/select
+    [@name="na&me"]
+    [not(@required)]
+    [
+        ./option[@value="&a"][not(@selected)][.="Choice&A"]
+        /following-sibling::option[@value="&b"][not(@selected)][.="Choice&B"]
+    ]
+    [count(./option)=2]
+'
+        );
+    }
+
+    public function testSingleChoiceWithNonRequiredEmptyValue()
+    {
+        $form = $this->factory->createNamed('choice', 'na&me', '&a', array(
+            'property_path' => 'name',
+            'choices' => array('&a' => 'Choice&A', '&b' => 'Choice&B'),
+            'multiple' => false,
+            'expanded' => false,
+            'required' => false,
+            'empty_value' => 'Select&Anything&Not&Me',
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/select
+    [@name="na&me"]
+    [not(@required)]
+    [
+        ./option[@value=""][not(@selected)][.="[trans]Select&Anything&Not&Me[/trans]"]
         /following-sibling::option[@value="&a"][@selected="selected"][.="Choice&A"]
         /following-sibling::option[@value="&b"][not(@selected)][.="Choice&B"]
     ]
@@ -388,6 +438,31 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
     }
 
     public function testSingleChoiceRequiredWithEmptyValue()
+    {
+        $form = $this->factory->createNamed('choice', 'na&me', '&a', array(
+            'property_path' => 'name',
+            'choices' => array('&a' => 'Choice&A', '&b' => 'Choice&B'),
+            'required' => true,
+            'multiple' => false,
+            'expanded' => false,
+            'empty_value' => 'Test&Me'
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/select
+    [@name="na&me"]
+    [@required="required"]
+    [
+        ./option[@value=""][.="[trans]Test&Me[/trans]"]
+        /following-sibling::option[@value="&a"][@selected="selected"][.="Choice&A"]
+        /following-sibling::option[@value="&b"][not(@selected)][.="Choice&B"]
+    ]
+    [count(./option)=3]
+'
+        );
+    }
+
+    public function testSingleChoiceRequiredWithEmptyValueViaView()
     {
         $form = $this->factory->createNamed('choice', 'na&me', '&a', array(
             'property_path' => 'name',
@@ -400,6 +475,7 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         $this->assertWidgetMatchesXpath($form->createView(), array('empty_value' => ''),
 '/select
     [@name="na&me"]
+    [@required="required"]
     [
         ./option[@value=""][.="[trans][/trans]"]
         /following-sibling::option[@value="&a"][@selected="selected"][.="Choice&A"]
@@ -463,6 +539,29 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testMultipleChoiceSkipEmptyValue()
+    {
+        $form = $this->factory->createNamed('choice', 'na&me', array('&a'), array(
+            'property_path' => 'name',
+            'choices' => array('&a' => 'Choice&A', '&b' => 'Choice&B'),
+            'multiple' => true,
+            'expanded' => false,
+            'empty_value' => 'Test&Me'
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/select
+    [@name="na&me[]"]
+    [@multiple="multiple"]
+    [
+        ./option[@value="&a"][@selected="selected"][.="Choice&A"]
+        /following-sibling::option[@value="&b"][not(@selected)][.="Choice&B"]
+    ]
+    [count(./option)=2]
+'
+        );
+    }
+
     public function testMultipleChoiceNonRequired()
     {
         $form = $this->factory->createNamed('choice', 'na&me', array('&a'), array(
@@ -508,6 +607,29 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testSingleChoiceExpandedSkipEmptyValue()
+    {
+        $form = $this->factory->createNamed('choice', 'na&me', '&a', array(
+            'property_path' => 'name',
+            'choices' => array('&a' => 'Choice&A', '&b' => 'Choice&B'),
+            'multiple' => false,
+            'expanded' => true,
+            'empty_value' => 'Test&Me'
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/div
+    [
+        ./input[@type="radio"][@name="na&me"][@id="na&me_&a"][@checked]
+        /following-sibling::label[@for="na&me_&a"][.="[trans]Choice&A[/trans]"]
+        /following-sibling::input[@type="radio"][@name="na&me"][@id="na&me_&b"][not(@checked)]
+        /following-sibling::label[@for="na&me_&b"][.="[trans]Choice&B[/trans]"]
+    ]
+    [count(./input)=2]
+'
+        );
+    }
+
     public function testSingleChoiceExpandedWithBooleanValue()
     {
         $form = $this->factory->createNamed('choice', 'na&me', true, array(
@@ -570,6 +692,24 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testCountryWithEmptyValue()
+    {
+        $form = $this->factory->createNamed('country', 'na&me', 'AT', array(
+            'property_path' => 'name',
+            'empty_value' => 'Select&Country',
+            'required' => false,
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/select
+    [@name="na&me"]
+    [./option[@value=""][not(@selected)][.="[trans]Select&Country[/trans]"]]
+    [./option[@value="AT"][@selected="selected"][.="Austria"]]
+    [count(./option)>201]
+'
+        );
+    }
+
     public function testCsrf()
     {
         $this->csrfProvider->expects($this->any())
@@ -628,6 +768,87 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testDateTimeWithEmptyValueGlobal()
+    {
+        $form = $this->factory->createNamed('datetime', 'na&me', null, array(
+            'property_path' => 'name',
+            'input' => 'string',
+            'empty_value' => 'Change&Me',
+            'required' => false,
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/div
+    [
+        ./div
+            [@id="na&me_date"]
+            [
+                ./select
+                    [@id="na&me_date_month"]
+                    [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+                /following-sibling::select
+                    [@id="na&me_date_day"]
+                    [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+                /following-sibling::select
+                    [@id="na&me_date_year"]
+                    [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+            ]
+        /following-sibling::div
+            [@id="na&me_time"]
+            [
+                ./select
+                    [@id="na&me_time_hour"]
+                    [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+                /following-sibling::select
+                    [@id="na&me_time_minute"]
+                    [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+            ]
+    ]
+    [count(.//select)=5]
+'
+        );
+    }
+    public function testDateTimeWithEmptyValueOnTime()
+    {
+        $form = $this->factory->createNamed('datetime', 'na&me', '2011-02-03', array(
+            'property_path' => 'name',
+            'input' => 'string',
+            'empty_value' => array('hour' => 'Change&Me', 'minute' => 'Change&Me'),
+            'required' => false,
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/div
+    [
+        ./div
+            [@id="na&me_date"]
+            [
+                ./select
+                    [@id="na&me_date_month"]
+                    [./option[@value="2"][@selected="selected"]]
+                /following-sibling::select
+                    [@id="na&me_date_day"]
+                    [./option[@value="3"][@selected="selected"]]
+                /following-sibling::select
+                    [@id="na&me_date_year"]
+                    [./option[@value="2011"][@selected="selected"]]
+            ]
+        /following-sibling::div
+            [@id="na&me_time"]
+            [
+                ./select
+                    [@id="na&me_time_hour"]
+                    [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+                /following-sibling::select
+                    [@id="na&me_time_minute"]
+                    [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+            ]
+    ]
+    [count(.//select)=5]
+'
+        );
+    }
+
     public function testDateTimeWithSeconds()
     {
         $form = $this->factory->createNamed('datetime', 'na&me', '2011-02-03 04:05:06', array(
@@ -697,6 +918,62 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testDateChoiceWithEmptyValueGlobal()
+    {
+        $form = $this->factory->createNamed('date', 'na&me', null, array(
+            'property_path' => 'name',
+            'input' => 'string',
+            'widget' => 'choice',
+            'empty_value' => 'Change&Me',
+            'required' => false,
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/div
+    [
+        ./select
+            [@id="na&me_month"]
+            [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+        /following-sibling::select
+            [@id="na&me_day"]
+            [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+        /following-sibling::select
+            [@id="na&me_year"]
+            [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+    ]
+    [count(./select)=3]
+'
+        );
+    }
+
+    public function testDateChoiceWithEmptyValueOnYear()
+    {
+        $form = $this->factory->createNamed('date', 'na&me', null, array(
+            'property_path' => 'name',
+            'input' => 'string',
+            'widget' => 'choice',
+            'required' => false,
+            'empty_value' => array('year' => 'Change&Me'),
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/div
+    [
+        ./select
+            [@id="na&me_month"]
+            [./option[@value="1"]]
+        /following-sibling::select
+            [@id="na&me_day"]
+            [./option[@value="1"]]
+        /following-sibling::select
+            [@id="na&me_year"]
+            [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+    ]
+    [count(./select)=3]
+'
+        );
+    }
+
     public function testDateText()
     {
         $form = $this->factory->createNamed('date', 'na&me', '2011-02-03', array(
@@ -743,6 +1020,61 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testBirthDay()
+    {
+        $form = $this->factory->createNamed('birthday', 'na&me', '2000-02-03', array(
+            'property_path' => 'name',
+            'input' => 'string',
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/div
+    [
+        ./select
+            [@id="na&me_month"]
+            [./option[@value="2"][@selected="selected"]]
+        /following-sibling::select
+            [@id="na&me_day"]
+            [./option[@value="3"][@selected="selected"]]
+        /following-sibling::select
+            [@id="na&me_year"]
+            [./option[@value="2000"][@selected="selected"]]
+    ]
+    [count(./select)=3]
+'
+        );
+    }
+
+    public function testBirthDayWithEmptyValue()
+    {
+        $form = $this->factory->createNamed('birthday', 'na&me', '1950-01-01', array(
+            'property_path' => 'name',
+            'input' => 'string',
+            'empty_value' => '',
+            'required' => false,
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/div
+    [
+        ./select
+            [@id="na&me_month"]
+            [./option[@value=""][.="[trans][/trans]"]]
+            [./option[@value="1"][@selected="selected"]]
+        /following-sibling::select
+            [@id="na&me_day"]
+            [./option[@value=""][.="[trans][/trans]"]]
+            [./option[@value="1"][@selected="selected"]]
+        /following-sibling::select
+            [@id="na&me_year"]
+            [./option[@value=""][.="[trans][/trans]"]]
+            [./option[@value="1950"][@selected="selected"]]
+    ]
+    [count(./select)=3]
+'
+        );
+    }
+
     public function testEmail()
     {
         $form = $this->factory->createNamed('email', 'na&me', 'foo&bar', array(
@@ -1097,20 +1429,75 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
             [@id="na&me_hour"]
             [@size="1"]
             [./option[@value="4"][@selected="selected"]]
+            [count(./option)>23]
         /following-sibling::select
             [@id="na&me_minute"]
             [@size="1"]
             [./option[@value="5"][@selected="selected"]]
+            [count(./option)>59]
         /following-sibling::select
             [@id="na&me_second"]
             [@size="1"]
             [./option[@value="6"][@selected="selected"]]
+            [count(./option)>59]
     ]
     [count(./select)=3]
 '
         );
     }
 
+    public function testTimeWithEmptyValueGlobal()
+    {
+        $form = $this->factory->createNamed('time', 'na&me', null, array(
+            'property_path' => 'name',
+            'input' => 'string',
+            'empty_value' => 'Change&Me',
+            'required' => false,
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/div
+    [
+        ./select
+            [@id="na&me_hour"]
+            [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+            [count(./option)>24]
+        /following-sibling::select
+            [@id="na&me_minute"]
+            [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+            [count(./option)>60]
+    ]
+    [count(./select)=2]
+'
+        );
+    }
+
+    public function testTimeWithEmptyValueOnYear()
+    {
+        $form = $this->factory->createNamed('time', 'na&me', null, array(
+            'property_path' => 'name',
+            'input' => 'string',
+            'required' => false,
+            'empty_value' => array('hour' => 'Change&Me'),
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/div
+    [
+        ./select
+            [@id="na&me_hour"]
+            [./option[@value=""][.="[trans]Change&Me[/trans]"]]
+            [count(./option)>24]
+        /following-sibling::select
+            [@id="na&me_minute"]
+            [./option[@value="1"]]
+            [count(./option)>59]
+    ]
+    [count(./select)=2]
+'
+        );
+    }
+
     public function testTimezone()
     {
         $form = $this->factory->createNamed('timezone', 'na&me', 'Europe/Vienna', array(
@@ -1120,6 +1507,7 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         $this->assertWidgetMatchesXpath($form->createView(), array(),
 '/select
     [@name="na&me"]
+    [@required="required"]
     [./optgroup
         [@label="Europe"]
         [./option[@value="Europe/Vienna"][@selected="selected"][.="Vienna"]]
@@ -1130,6 +1518,23 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testTimezoneWithEmptyValue()
+    {
+        $form = $this->factory->createNamed('timezone', 'na&me', null, array(
+            'property_path' => 'name',
+            'empty_value' => 'Select&Timezone',
+            'required' => false,
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/select
+    [./option[@value=""][.="[trans]Select&Timezone[/trans]"]]
+    [count(./optgroup)>10]
+    [count(.//option)>201]
+'
+        );
+    }
+
     public function testUrl()
     {
         $url = 'http://www.google.com?foo1=bar1&foo2=bar2';

tests/Symfony/Tests/Component/Form/Extension/Core/Type/ChoiceTypeTest.php

@@ -311,6 +311,27 @@ class ChoiceTypeTest extends TypeTestCase
         $this->factory->create('choice', 'name');
     }
 
+    public function testPassRequiredToView()
+    {
+        $form = $this->factory->create('choice', null, array(
+            'choices' => $this->choices,
+        ));
+        $view = $form->createView();
+
+        $this->assertTrue($view->get('required'));
+    }
+
+    public function testPassNonRequiredToView()
+    {
+        $form = $this->factory->create('choice', null, array(
+            'required' => false,
+            'choices' => $this->choices,
+        ));
+        $view = $form->createView();
+
+        $this->assertFalse($view->get('required'));
+    }
+
     public function testPassMultipleToView()
     {
         $form = $this->factory->create('choice', null, array(
@@ -333,6 +354,61 @@ class ChoiceTypeTest extends TypeTestCase
         $this->assertTrue($view->get('expanded'));
     }
 
+    public function testNotPassedEmptyValueToViewIsNull()
+    {
+        $form = $this->factory->create('choice', null, array(
+            'multiple' => false,
+            'choices' => $this->choices,
+        ));
+        $view = $form->createView();
+
+        $this->assertNull($view->get('empty_value'));
+    }
+
+    public function testPassEmptyValueToViewIsEmpty()
+    {
+        $form = $this->factory->create('choice', null, array(
+            'multiple' => false,
+            'required' => false,
+            'choices' => $this->choices,
+        ));
+        $view = $form->createView();
+
+        $this->assertEmpty($view->get('empty_value'));
+    }
+
+    /**
+     * @dataProvider getOptionsWithEmptyValue
+     */
+    public function testPassEmptyValueToView($multiple, $expanded, $required, $emptyValue, $viewValue)
+    {
+        $form = $this->factory->create('choice', null, array(
+            'multiple' => $multiple,
+            'expanded' => $expanded,
+            'required' => $required,
+            'empty_value' => $emptyValue,
+            'choices' => $this->choices,
+        ));
+        $view = $form->createView();
+
+        $this->assertEquals($viewValue, $view->get('empty_value'));
+    }
+
+    public function getOptionsWithEmptyValue()
+    {
+        return array(
+            array(false, false, false, 'foobar', 'foobar'),
+            array(true, false, false, 'foobar', null),
+            array(false, true, false, 'foobar', null),
+            array(false, false, true, 'foobar', 'foobar'),
+            array(false, false, true, '', ''),
+            array(false, false, true, null, null),
+            array(false, true, true, 'foobar', null),
+            array(true, true, false, 'foobar', null),
+            array(true, true, true, 'foobar', null),
+        );
+    }
+
     public function testPassChoicesToView()
     {
         $choices = array('a' => 'A', 'b' => 'B', 'c' => 'C', 'd' => 'D');

tests/Symfony/Tests/Component/Form/Extension/Core/Type/DateTypeTest.php

@@ -174,7 +174,7 @@ class DateTypeTest extends LocalizedTestCase
 
         $form->bind($text);
 
-        $this->assertSame(null, $form->getData());
+        $this->assertNull($form->getData());
         $this->assertEquals($text, $form->getClientData());
     }