money-bundle icon indicating copy to clipboard operation
money-bundle copied to clipboard

PHP 8 support

Open enumag opened this issue 4 years ago • 2 comments

Currently this bundle doesn't allow PHP 8. All required packages seem to already do so. Is there any blocker here?

enumag avatar Feb 22 '21 14:02 enumag

As I see, the phpunit is blocking right now. I've tried the upgrade to version 9.5.2 and found another bug (9 to be exact):

1) JK\MoneyBundle\Tests\Twig\MoneyExtensionTest::testMoneyFilter with data set #0 ('cs_CZ', 'CZK', 2, true, true, 1599, '15,99 Kč')
TypeError: NumberFormatter::setAttribute(): Argument #2 ($value) must be of type int|float, bool given

/app/Twig/MoneyExtension.php:43
/app/Tests/Twig/MoneyExtensionTest.php:54

Fix is rather easy in Twig/MoneyExtension.php:43

-        $noFormatter->setAttribute(NumberFormatter::GROUPING_USED, $groupingUsed);
+        $noFormatter->setAttribute(NumberFormatter::GROUPING_USED, $groupingUsed ? 1 : 0);

Another way would be changing values of consts to integers:

    const GROUPING_USED = true;
    const GROUPING_NONE = false;

The first option isn't a BC break.

I can prepare a PR if you want.

rzeczkowskip avatar Feb 22 '21 18:02 rzeczkowskip

Personally I'd use this but ofc I'm not the maintainer here... use whichever non-BC break method you like, I'm sure the PR will be appreciated.

$noFormatter->setAttribute(NumberFormatter::GROUPING_USED, (int) $groupingUsed);

enumag avatar Feb 22 '21 20:02 enumag