Skip to content

Commit

Permalink
bug #36578 [Form] deprecate not using a rounding mode (xabbuh)
Browse files Browse the repository at this point in the history
This PR was merged into the 5.1-dev branch.

Discussion
----------

[Form] deprecate not using a rounding mode

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | yes
| Tickets       |
| License       | MIT
| Doc PR        |

Commits
-------

25ba1a2 deprecate not using a rounding mode
  • Loading branch information
fabpot committed May 3, 2020
2 parents 669b7f1 + 25ba1a2 commit 3a6f8ca
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 39 deletions.
2 changes: 2 additions & 0 deletions UPGRADE-5.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ EventDispatcher
Form
----

* Not configuring the `rounding_mode` option of the `PercentType` is deprecated. It will default to `PercentToLocalizedStringTransformer::ROUND_HALF_UP` in Symfony 6.
* Not passing a rounding mode to the constructor of `PercentToLocalizedStringTransformer` is deprecated. It will default to `ROUND_HALF_UP` in Symfony 6.
* Implementing the `FormConfigInterface` without implementing the `getIsEmptyCallback()` method
is deprecated. The method will be added to the interface in 6.0.
* Implementing the `FormConfigBuilderInterface` without implementing the `setIsEmptyCallback()` method
Expand Down
2 changes: 2 additions & 0 deletions UPGRADE-6.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ EventDispatcher
Form
----

* The default value of the `rounding_mode` option of the `PercentType` has been changed to `PercentToLocalizedStringTransformer::ROUND_HALF_UP`.
* The default rounding mode of the `PercentToLocalizedStringTransformer` has been changed to `ROUND_HALF_UP`.
* Added the `getIsEmptyCallback()` method to the `FormConfigInterface`.
* Added the `setIsEmptyCallback()` method to the `FormConfigBuilderInterface`.
* Added argument `callable|null $filter` to `ChoiceListFactoryInterface::createListFromChoices()` and `createListFromLoader()`.
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/Form/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ CHANGELOG
5.1.0
-----

* Deprecated not configuring the `rounding_mode` option of the `PercentType`. It will default to `PercentToLocalizedStringTransformer::ROUND_HALF_UP` in Symfony 6.
* Deprecated not passing a rounding mode to the constructor of `PercentToLocalizedStringTransformer`. It will default to `ROUND_HALF_UP` in Symfony 6.
* Added `collection_entry` block prefix to `CollectionType` entries
* Added a `choice_filter` option to `ChoiceType`
* Added argument `callable|null $filter` to `ChoiceListFactoryInterface::createListFromChoices()` and `createListFromLoader()` - not defining them is deprecated.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ class PercentToLocalizedStringTransformer implements DataTransformerInterface
self::INTEGER,
];

protected $roundingMode;

private $roundingMode;
private $type;
private $scale;

Expand All @@ -93,7 +92,7 @@ class PercentToLocalizedStringTransformer implements DataTransformerInterface
*
* @throws UnexpectedTypeException if the given value of type is unknown
*/
public function __construct(int $scale = null, string $type = null, ?int $roundingMode = self::ROUND_HALF_UP)
public function __construct(int $scale = null, string $type = null, ?int $roundingMode = null)
{
if (null === $scale) {
$scale = 0;
Expand All @@ -103,8 +102,8 @@ public function __construct(int $scale = null, string $type = null, ?int $roundi
$type = self::FRACTIONAL;
}

if (null === $roundingMode) {
$roundingMode = self::ROUND_HALF_UP;
if (null === $roundingMode && (\func_num_args() < 4 || func_get_arg(3))) {
trigger_deprecation('symfony/form', '5.1', sprintf('Not passing a rounding mode to %s() is deprecated. Starting with Symfony 6.0 it will default to "%s::ROUND_HALF_UP".', __METHOD__, __CLASS__));
}

if (!\in_array($type, self::$types, true)) {
Expand Down Expand Up @@ -235,7 +234,10 @@ protected function getNumberFormatter()
$formatter = new \NumberFormatter(\Locale::getDefault(), \NumberFormatter::DECIMAL);

$formatter->setAttribute(\NumberFormatter::FRACTION_DIGITS, $this->scale);
$formatter->setAttribute(\NumberFormatter::ROUNDING_MODE, $this->roundingMode);

if (null !== $this->roundingMode) {
$formatter->setAttribute(\NumberFormatter::ROUNDING_MODE, $this->roundingMode);
}

return $formatter;
}
Expand Down
33 changes: 23 additions & 10 deletions src/Symfony/Component/Form/Extension/Core/Type/PercentType.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
namespace Symfony\Component\Form\Extension\Core\Type;

use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\DataTransformer\NumberToLocalizedStringTransformer;
use Symfony\Component\Form\Extension\Core\DataTransformer\PercentToLocalizedStringTransformer;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormView;
use Symfony\Component\OptionsResolver\Options;
use Symfony\Component\OptionsResolver\OptionsResolver;

class PercentType extends AbstractType
Expand All @@ -29,7 +29,8 @@ public function buildForm(FormBuilderInterface $builder, array $options)
$builder->addViewTransformer(new PercentToLocalizedStringTransformer(
$options['scale'],
$options['type'],
$options['rounding_mode']
$options['rounding_mode'],
false
));
}

Expand All @@ -48,7 +49,11 @@ public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults([
'scale' => 0,
'rounding_mode' => NumberToLocalizedStringTransformer::ROUND_HALF_UP,
'rounding_mode' => function (Options $options) {
trigger_deprecation('symfony/form', '5.1', sprintf('Not configuring the "rounding_mode" option is deprecated. It will default to "%s::ROUND_HALF_UP" in Symfony 6.0.', PercentToLocalizedStringTransformer::class));

return null;
},
'symbol' => '%',
'type' => 'fractional',
'compound' => false,
Expand All @@ -59,16 +64,24 @@ public function configureOptions(OptionsResolver $resolver)
'integer',
]);
$resolver->setAllowedValues('rounding_mode', [
NumberToLocalizedStringTransformer::ROUND_FLOOR,
NumberToLocalizedStringTransformer::ROUND_DOWN,
NumberToLocalizedStringTransformer::ROUND_HALF_DOWN,
NumberToLocalizedStringTransformer::ROUND_HALF_EVEN,
NumberToLocalizedStringTransformer::ROUND_HALF_UP,
NumberToLocalizedStringTransformer::ROUND_UP,
NumberToLocalizedStringTransformer::ROUND_CEILING,
null,
PercentToLocalizedStringTransformer::ROUND_FLOOR,
PercentToLocalizedStringTransformer::ROUND_DOWN,
PercentToLocalizedStringTransformer::ROUND_HALF_DOWN,
PercentToLocalizedStringTransformer::ROUND_HALF_EVEN,
PercentToLocalizedStringTransformer::ROUND_HALF_UP,
PercentToLocalizedStringTransformer::ROUND_UP,
PercentToLocalizedStringTransformer::ROUND_CEILING,
]);
$resolver->setAllowedTypes('scale', 'int');
$resolver->setAllowedTypes('symbol', ['bool', 'string']);
$resolver->setDeprecated('rounding_mode', 'symfony/form', '5.1', function (Options $options, $roundingMode) {
if (null === $roundingMode) {
return sprintf('Not configuring the "rounding_mode" option is deprecated. It will default to "%s::ROUND_HALF_UP" in Symfony 6.0.', PercentToLocalizedStringTransformer::class);
}

return '';
});
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@
namespace Symfony\Component\Form\Tests\Extension\Core\DataTransformer;

use PHPUnit\Framework\TestCase;
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
use Symfony\Component\Form\Extension\Core\DataTransformer\PercentToLocalizedStringTransformer;
use Symfony\Component\Intl\Util\IntlTestHelper;

class PercentToLocalizedStringTransformerTest extends TestCase
{
use ExpectDeprecationTrait;

private $defaultLocale;

protected function setUp(): void
Expand All @@ -32,7 +35,7 @@ protected function tearDown(): void

public function testTransform()
{
$transformer = new PercentToLocalizedStringTransformer();
$transformer = new PercentToLocalizedStringTransformer(null, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$this->assertEquals('10', $transformer->transform(0.1));
$this->assertEquals('15', $transformer->transform(0.15));
Expand All @@ -42,14 +45,14 @@ public function testTransform()

public function testTransformEmpty()
{
$transformer = new PercentToLocalizedStringTransformer();
$transformer = new PercentToLocalizedStringTransformer(null, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$this->assertEquals('', $transformer->transform(null));
}

public function testTransformWithInteger()
{
$transformer = new PercentToLocalizedStringTransformer(null, 'integer');
$transformer = new PercentToLocalizedStringTransformer(null, 'integer', PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$this->assertEquals('0', $transformer->transform(0.1));
$this->assertEquals('1', $transformer->transform(1));
Expand All @@ -64,14 +67,26 @@ public function testTransformWithScale()

\Locale::setDefault('de_AT');

$transformer = new PercentToLocalizedStringTransformer(2);
$transformer = new PercentToLocalizedStringTransformer(2, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$this->assertEquals('12,34', $transformer->transform(0.1234));
}

/**
* @group legacy
*/
public function testReverseTransformWithScaleAndRoundingDisabled()
{
$this->expectDeprecation('Since symfony/form 5.1: Not passing a rounding mode to Symfony\Component\Form\Extension\Core\DataTransformer\PercentToLocalizedStringTransformer::__construct() is deprecated. Starting with Symfony 6.0 it will default to Symfony\Component\Form\Extension\Core\DataTransformer\PercentToLocalizedStringTransformer::ROUND_HALF_UP.');

$transformer = new PercentToLocalizedStringTransformer(2, PercentToLocalizedStringTransformer::FRACTIONAL);

$this->assertEquals(0.0123456, $transformer->reverseTransform('1.23456'));
}

public function testReverseTransform()
{
$transformer = new PercentToLocalizedStringTransformer();
$transformer = new PercentToLocalizedStringTransformer(null, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$this->assertEquals(0.1, $transformer->reverseTransform('10'));
$this->assertEquals(0.15, $transformer->reverseTransform('15'));
Expand Down Expand Up @@ -184,14 +199,14 @@ public function testReverseTransformWithRounding($type, $scale, $input, $output,

public function testReverseTransformEmpty()
{
$transformer = new PercentToLocalizedStringTransformer();
$transformer = new PercentToLocalizedStringTransformer(null, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$this->assertNull($transformer->reverseTransform(''));
}

public function testReverseTransformWithInteger()
{
$transformer = new PercentToLocalizedStringTransformer(null, 'integer');
$transformer = new PercentToLocalizedStringTransformer(null, 'integer', PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$this->assertEquals(10, $transformer->reverseTransform('10'));
$this->assertEquals(15, $transformer->reverseTransform('15'));
Expand All @@ -206,14 +221,14 @@ public function testReverseTransformWithScale()

\Locale::setDefault('de_AT');

$transformer = new PercentToLocalizedStringTransformer(2);
$transformer = new PercentToLocalizedStringTransformer(2, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$this->assertEquals(0.1234, $transformer->reverseTransform('12,34'));
}

public function testTransformExpectsNumeric()
{
$transformer = new PercentToLocalizedStringTransformer();
$transformer = new PercentToLocalizedStringTransformer(null, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$this->expectException('Symfony\Component\Form\Exception\TransformationFailedException');

Expand All @@ -222,7 +237,7 @@ public function testTransformExpectsNumeric()

public function testReverseTransformExpectsString()
{
$transformer = new PercentToLocalizedStringTransformer();
$transformer = new PercentToLocalizedStringTransformer(null, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$this->expectException('Symfony\Component\Form\Exception\TransformationFailedException');

Expand All @@ -234,7 +249,7 @@ public function testDecimalSeparatorMayBeDotIfGroupingSeparatorIsNotDot()
IntlTestHelper::requireFullIntl($this, '4.8.1.1');

\Locale::setDefault('fr');
$transformer = new PercentToLocalizedStringTransformer(1, 'integer');
$transformer = new PercentToLocalizedStringTransformer(1, 'integer', PercentToLocalizedStringTransformer::ROUND_HALF_UP);

// completely valid format
$this->assertEquals(1234.5, $transformer->reverseTransform('1 234,5'));
Expand All @@ -253,7 +268,7 @@ public function testDecimalSeparatorMayNotBeDotIfGroupingSeparatorIsDot()

\Locale::setDefault('de_DE');

$transformer = new PercentToLocalizedStringTransformer(1, 'integer');
$transformer = new PercentToLocalizedStringTransformer(1, 'integer', PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$transformer->reverseTransform('1.234.5');
}
Expand All @@ -266,7 +281,7 @@ public function testDecimalSeparatorMayNotBeDotIfGroupingSeparatorIsDotWithNoGro

\Locale::setDefault('de_DE');

$transformer = new PercentToLocalizedStringTransformer(1, 'integer');
$transformer = new PercentToLocalizedStringTransformer(1, 'integer', PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$transformer->reverseTransform('1234.5');
}
Expand All @@ -277,7 +292,7 @@ public function testDecimalSeparatorMayBeDotIfGroupingSeparatorIsDotButNoGroupin
IntlTestHelper::requireFullIntl($this, false);

\Locale::setDefault('fr');
$transformer = new PercentToLocalizedStringTransformer(1, 'integer');
$transformer = new PercentToLocalizedStringTransformer(1, 'integer', PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$this->assertEquals(1234.5, $transformer->reverseTransform('1234,5'));
$this->assertEquals(1234.5, $transformer->reverseTransform('1234.5'));
Expand All @@ -289,7 +304,7 @@ public function testDecimalSeparatorMayBeCommaIfGroupingSeparatorIsNotComma()
IntlTestHelper::requireFullIntl($this, '4.8.1.1');

\Locale::setDefault('bg');
$transformer = new PercentToLocalizedStringTransformer(1, 'integer');
$transformer = new PercentToLocalizedStringTransformer(1, 'integer', PercentToLocalizedStringTransformer::ROUND_HALF_UP);

// completely valid format
$this->assertEquals(1234.5, $transformer->reverseTransform('1 234.5'));
Expand All @@ -305,7 +320,7 @@ public function testDecimalSeparatorMayNotBeCommaIfGroupingSeparatorIsComma()
$this->expectException('Symfony\Component\Form\Exception\TransformationFailedException');
IntlTestHelper::requireFullIntl($this, '4.8.1.1');

$transformer = new PercentToLocalizedStringTransformer(1, 'integer');
$transformer = new PercentToLocalizedStringTransformer(1, 'integer', PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$transformer->reverseTransform('1,234,5');
}
Expand All @@ -315,7 +330,7 @@ public function testDecimalSeparatorMayNotBeCommaIfGroupingSeparatorIsCommaWithN
$this->expectException('Symfony\Component\Form\Exception\TransformationFailedException');
IntlTestHelper::requireFullIntl($this, '4.8.1.1');

$transformer = new PercentToLocalizedStringTransformer(1, 'integer');
$transformer = new PercentToLocalizedStringTransformer(1, 'integer', PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$transformer->reverseTransform('1234,5');
}
Expand All @@ -328,7 +343,7 @@ public function testDecimalSeparatorMayBeCommaIfGroupingSeparatorIsCommaButNoGro

$transformer = $this->getMockBuilder('Symfony\Component\Form\Extension\Core\DataTransformer\PercentToLocalizedStringTransformer')
->setMethods(['getNumberFormatter'])
->setConstructorArgs([1, 'integer'])
->setConstructorArgs([1, 'integer', PercentToLocalizedStringTransformer::ROUND_HALF_UP])
->getMock();
$transformer->expects($this->any())
->method('getNumberFormatter')
Expand All @@ -341,7 +356,7 @@ public function testDecimalSeparatorMayBeCommaIfGroupingSeparatorIsCommaButNoGro
public function testReverseTransformDisallowsLeadingExtraCharacters()
{
$this->expectException('Symfony\Component\Form\Exception\TransformationFailedException');
$transformer = new PercentToLocalizedStringTransformer();
$transformer = new PercentToLocalizedStringTransformer(null, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$transformer->reverseTransform('foo123');
}
Expand All @@ -350,7 +365,7 @@ public function testReverseTransformDisallowsCenteredExtraCharacters()
{
$this->expectException('Symfony\Component\Form\Exception\TransformationFailedException');
$this->expectExceptionMessage('The number contains unrecognized characters: "foo3"');
$transformer = new PercentToLocalizedStringTransformer();
$transformer = new PercentToLocalizedStringTransformer(null, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$transformer->reverseTransform('12foo3');
}
Expand All @@ -367,7 +382,7 @@ public function testReverseTransformDisallowsCenteredExtraCharactersMultibyte()

\Locale::setDefault('ru');

$transformer = new PercentToLocalizedStringTransformer();
$transformer = new PercentToLocalizedStringTransformer(null, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$transformer->reverseTransform("12\xc2\xa0345,67foo8");
}
Expand All @@ -376,7 +391,7 @@ public function testReverseTransformDisallowsTrailingExtraCharacters()
{
$this->expectException('Symfony\Component\Form\Exception\TransformationFailedException');
$this->expectExceptionMessage('The number contains unrecognized characters: "foo"');
$transformer = new PercentToLocalizedStringTransformer();
$transformer = new PercentToLocalizedStringTransformer(null, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$transformer->reverseTransform('123foo');
}
Expand All @@ -393,7 +408,7 @@ public function testReverseTransformDisallowsTrailingExtraCharactersMultibyte()

\Locale::setDefault('ru');

$transformer = new PercentToLocalizedStringTransformer();
$transformer = new PercentToLocalizedStringTransformer(null, null, PercentToLocalizedStringTransformer::ROUND_HALF_UP);

$transformer->reverseTransform("12\xc2\xa0345,678foo");
}
Expand Down

0 comments on commit 3a6f8ca

Please sign in to comment.