Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[Form] Added option type checks in some FormTypes #12425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[Form] Added option type checks in some FormTypes #12425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions 2 UPGRADE-3.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,8 @@ UPGRADE FROM 2.x to 3.0
```php
echo $form->getErrors(true, false);
```
* The array type hints for `ChoiceList::initialize()` method's `$labels` and
`$preferredChoices` parameters were removed.

### FrameworkBundle

Expand Down
6 changes: 3 additions & 3 deletions 6 src/Symfony/Component/Form/Extension/Core/ChoiceList/ChoiceList.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class ChoiceList implements ChoiceListInterface
*
* @throws UnexpectedTypeException If the choices are not an array or \Traversable.
*/
public function __construct($choices, array $labels, array $preferredChoices = array())
public function __construct($choices, $labels, $preferredChoices = array())
{
if (!is_array($choices) && !$choices instanceof \Traversable) {
throw new UnexpectedTypeException($choices, 'array or \Traversable');
Expand All @@ -105,7 +105,7 @@ public function __construct($choices, array $labels, array $preferredChoices = a
* @param array $labels The labels belonging to the choices.
* @param array $preferredChoices The choices to display with priority.
*/
protected function initialize($choices, array $labels, array $preferredChoices)
protected function initialize($choices, $labels, $preferredChoices)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changing this signature is a BC break

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof is right. However, this class is not an API class, so technically this change is possible, as long as the change and the upgrade path is documented in the UPGRADE file (as per our BC policy). Can you add that information to the UPGRADE file please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@webmozart, sure, will do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a note in the UPGRADE file, it looks like this is the only remaining thing to do before merging.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabpot This file has been removed in 3.0.

This PR should be rebased before a new review IMHO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new PR on master is indeed needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will try to fix it up.

{
$this->choices = array();
$this->values = array();
Expand Down Expand Up @@ -271,7 +271,7 @@ public function getIndicesForValues(array $values)
* @throws InvalidArgumentException If the structures of the choices and labels array do not match.
* @throws InvalidConfigurationException If no valid value or index could be created for a choice.
*/
protected function addChoices(array &$bucketForPreferred, array &$bucketForRemaining, $choices, array $labels, array $preferredChoices)
protected function addChoices(array &$bucketForPreferred, array &$bucketForRemaining, $choices, $labels, $preferredChoices)
{
// Add choices to the nested buckets
foreach ($choices as $group => $choice) {
Expand Down
4 changes: 2 additions & 2 deletions 4 src/Symfony/Component/Form/Extension/Core/ChoiceList/ObjectChoiceList.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class ObjectChoiceList extends ChoiceList
* are generated instead.
* @param PropertyAccessorInterface $propertyAccessor The reflection graph for reading property paths.
*/
public function __construct($choices, $labelPath = null, array $preferredChoices = array(), $groupPath = null, $valuePath = null, PropertyAccessorInterface $propertyAccessor = null)
public function __construct($choices, $labelPath = null, $preferredChoices = array(), $groupPath = null, $valuePath = null, PropertyAccessorInterface $propertyAccessor = null)
{
$this->propertyAccessor = $propertyAccessor ?: PropertyAccess::createPropertyAccessor();
$this->labelPath = null !== $labelPath ? new PropertyPath($labelPath) : null;
Expand All @@ -112,7 +112,7 @@ public function __construct($choices, $labelPath = null, array $preferredChoices
* @throws InvalidArgumentException When passing a hierarchy of choices and using
* the "groupPath" option at the same time.
*/
protected function initialize($choices, array $labels, array $preferredChoices)
protected function initialize($choices, $labels, $preferredChoices)
{
if (null !== $this->groupPath) {
$groupedChoices = array();
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Component/Form/Extension/Core/ChoiceList/SimpleChoiceList.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public function getValuesForChoices(array $choices)
* @param array $labels Ignored.
* @param array $preferredChoices The preferred choices.
*/
protected function addChoices(array &$bucketForPreferred, array &$bucketForRemaining, $choices, array $labels, array $preferredChoices)
protected function addChoices(array &$bucketForPreferred, array &$bucketForRemaining, $choices, $labels, $preferredChoices)
{
// Add choices to the nested buckets
foreach ($choices as $choice => $label) {
Expand Down
2 changes: 2 additions & 0 deletions 2 src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ public function configureOptions(OptionsResolver $resolver)
'delete_empty' => false,
));

$resolver->setAllowedTypes('options', 'array');

$resolver->setNormalizer('options', $optionsNormalizer);
}

Expand Down
17 changes: 17 additions & 0 deletions 17 src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1671,4 +1671,21 @@ public function testInitializeWithDefaultObjectChoice()
// Trigger data initialization
$form->getViewData();
}

public function testChoicesOptionSupportsTraversable()
{
$this->factory->create('choice', null, array(
'choices' => new \ArrayObject($this->choices),
));
}

/**
* @expectedException \Symfony\Component\OptionsResolver\Exception\InvalidOptionsException
*/
public function testSetInvalidPreferredChoicesOption()
{
$this->factory->create('choice', null, array(
'preferred_choices' => 'bad value',
));
}
}
10 changes: 10 additions & 0 deletions 10 src/Symfony/Component/Form/Tests/Extension/Core/Type/CollectionTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,4 +274,14 @@ public function testPrototypeDefaultLabel()

$this->assertSame('__test__label__', $form->createView()->vars['prototype']->vars['label']);
}

/**
* @expectedException \Symfony\Component\OptionsResolver\Exception\InvalidOptionsException
*/
public function testSetInvalidOptions()
{
$this->factory->create('collection', null, array(
'options' => 'bad value',
));
}
}
10 changes: 10 additions & 0 deletions 10 src/Symfony/Component/Form/Tests/Extension/Core/Type/CountryTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,14 @@ public function testUnknownCountryIsNotIncluded()
}
}
}

/**
* @expectedException \Symfony\Component\OptionsResolver\Exception\InvalidOptionsException
*/
public function testSetInvalidChoices()
{
$this->factory->create('country', null, array(
'choices' => 'bad value',
));
}
}
10 changes: 10 additions & 0 deletions 10 src/Symfony/Component/Form/Tests/Extension/Core/Type/CurrencyTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,14 @@ public function testCurrenciesAreSelectable()
$this->assertContains(new ChoiceView('USD', 'USD', 'US Dollar'), $choices, '', false, false);
$this->assertContains(new ChoiceView('SIT', 'SIT', 'Slovenian Tolar'), $choices, '', false, false);
}

/**
* @expectedException \Symfony\Component\OptionsResolver\Exception\InvalidOptionsException
*/
public function testSetInvalidChoices()
{
$this->factory->create('currency', null, array(
'choices' => 'bad value',
));
}
}
10 changes: 10 additions & 0 deletions 10 src/Symfony/Component/Form/Tests/Extension/Core/Type/LanguageTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,14 @@ public function testMultipleLanguagesIsNotIncluded()

$this->assertNotContains(new ChoiceView('mul', 'mul', 'Mehrsprachig'), $choices, '', false, false);
}

/**
* @expectedException \Symfony\Component\OptionsResolver\Exception\InvalidOptionsException
*/
public function testSetInvalidChoices()
{
$this->factory->create('language', null, array(
'choices' => 'bad value',
));
}
}
10 changes: 10 additions & 0 deletions 10 src/Symfony/Component/Form/Tests/Extension/Core/Type/LocaleTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,14 @@ public function testLocalesAreSelectable()
$this->assertContains(new ChoiceView('en_GB', 'en_GB', 'English (United Kingdom)'), $choices, '', false, false);
$this->assertContains(new ChoiceView('zh_Hant_MO', 'zh_Hant_MO', 'Chinese (Traditional, Macau SAR China)'), $choices, '', false, false);
}

/**
* @expectedException \Symfony\Component\OptionsResolver\Exception\InvalidOptionsException
*/
public function testSetInvalidChoices()
{
$this->factory->create('locale', null, array(
'choices' => 'bad value',
));
}
}
10 changes: 10 additions & 0 deletions 10 src/Symfony/Component/Form/Tests/Extension/Core/Type/TimeTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,4 +731,14 @@ public function testThrowExceptionIfSecondsIsInvalid()
'seconds' => 'bad value',
));
}

/**
* @expectedException \Symfony\Component\OptionsResolver\Exception\InvalidOptionsException
*/
public function testThrowExceptionIfEmptyValueIsInvalid()
{
$this->factory->create('time', null, array(
'empty_value' => array(),
));
}
}
10 changes: 10 additions & 0 deletions 10 src/Symfony/Component/Form/Tests/Extension/Core/Type/TimezoneTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,14 @@ public function testTimezonesAreSelectable()
$this->assertArrayHasKey('America', $choices);
$this->assertContains(new ChoiceView('America/New_York', 'America/New_York', 'New York'), $choices['America'], '', false, false);
}

/**
* @expectedException \Symfony\Component\OptionsResolver\Exception\InvalidOptionsException
*/
public function testSetInvalidChoices()
{
$this->factory->create('timezone', null, array(
'choices' => 'bad value',
));
}
}
Morty Proxy This is a proxified and sanitized view of the page, visit original site.