NumberFormatter parsing fails

[alexandrunastase]

Description

The following code:

<?php

$formatter = new NumberFormatter('de_DE', NumberFormatter::DECIMAL);
$result = $formatter->parse('100,01') 
// Expected: 100.01
// Actual: false

$result = $formatter->parse('1.100,01') 
// Expected: 1100.01
// Actual: 1.1

I created a docker-compose project that contains a phpunit test with multiple data that are working fine in PHP 8.1.3, but do not in 8.1.25 : https://github.com/alexandrunastase/reproduce-number-formatter-issue.

I also tried 8.2.12 and the issue is present there as well

Tried both de_DE and ro_RO locales and both are behaving the same.

PHP Version

PHP 8.1.25, PHP 8.2.12

Operating System

alpine

[devnexen]

works for me with 8.2.12 under macOs.

[nielsdos]

@alexandrunastase What version of libicu do you have installed on there? It's possible to check via the INTL_ICU_VERSION constant.

[alexandrunastase]

@devnexen Thanks for the feedback I updated the OS to alpine. Seems to be specific to this one

[damianwadley]

The 8.1.3 image is based on alpine 3.15 which uses ICU 69.1.
The 8.1.25 image is based on alpine 3.18 which uses ICU 73.2.

The difference is no doubt going to be because of the ICU versions, in that there was a change in either it or the underlying CLDR data, but tracking down exactly where and why is difficult.

[alexandrunastase]

@nielsdos I get 73.2 for the 8.1.25 and 69.1 for 8.1.3

[ramsey]

I don't think changes to the CLDR would cause these differences.

I've been digging through the ICU source to see what might have changed. Here are some potential culprits that are changes introduced in ICU 73.1 and 73.2. The rest of the code in these functions (in ICU) hasn't changed in years; most of the code hasn't changed in 20+ years.

(I'm not sure how these changes could cause the issue reported here, but since they're changes made in code that hasn't changed in many, many years, we shouldn't rule out these changes as impacting behavior.)

• The signatures of lots of functions (including unum_parseDouble(), which we use in this case) changed from UChar to char16_t.
  • See here: https://github.com/unicode-org/icu/blame/8d3d214ad7f76b7d0650f19a871a0e7d58a5986f/icu4c/source/i18n/unum.cpp#L433
  • And here: unicode-org/icu@2de88f9#diff-6ca0ceec708e4a3a67830d154fb6d4fb77479e20f86dd64f693134e43536a7afL417-R417
• They replaced NULL with nullptr throughout the code, which includes lines of code executed when calling unum_parseDouble().
  • See here: https://github.com/unicode-org/icu/blame/8d3d214ad7f76b7d0650f19a871a0e7d58a5986f/icu4c/source/i18n/fmtable.cpp#L507
  • And here: unicode-org/icu@2e0d30c#diff-ed7ebcc62a20814e17e3963eef255ae09f90582277b34d8a179ed4d43e944b50L507-R507

What's clear is the only way NumberFormatter::parse() returns false is if libicu reports an error. See here:

php-src/ext/intl/formatter/formatter_parse.c

Line 118 in ae44be1

INTL_METHOD_CHECK_STATUS( nfo, "Number parsing failed" );

This macro is defined over here, where you can see that it returns false if there's an ICU error:

php-src/ext/intl/intl_data.h

Lines 66 to 72 in ae44be1

	#define INTL_METHOD_CHECK_STATUS(obj, msg) \
	intl_error_set_code( NULL, INTL_DATA_ERROR_CODE((obj)) ); \
	if( U_FAILURE( INTL_DATA_ERROR_CODE((obj)) ) ) \
	{ \
	intl_errors_set_custom_msg( INTL_DATA_ERROR_P((obj)), msg, 0 ); \
	RETURN_FALSE; \
	}

[ramsey]

I submitted a PR to your repo that adds the ICU error message to the test failure output: alexandrunastase/reproduce-number-formatter-issue#1

PHPUnit 9.6.13 by Sebastian Bergmann and contributors.

FFFF                                                                4 / 4 (100%)

Time: 00:00.014, Memory: 6.00 MB

There were 4 failures:

1) NumberFormatterTest::testEuropeanFormat with data set #0 ('100,01', 100.01)
ICU error: (9) Number parsing failed: U_PARSE_ERROR
Failed asserting that false is identical to 100.01.

/app/tests/NumberFormatterTest.php:19

2) NumberFormatterTest::testEuropeanFormat with data set #1 ('1.100,01', 1100.01)
ICU error: (0) U_ZERO_ERROR
Failed asserting that 1.1 is identical to 1100.01.

/app/tests/NumberFormatterTest.php:19

3) NumberFormatterTest::testEuropeanFormat with data set #2 ('1.000.100,01', 1000100.01)
ICU error: (0) U_ZERO_ERROR
Failed asserting that 1.0 is identical to 1000100.01.

/app/tests/NumberFormatterTest.php:19

4) NumberFormatterTest::testEuropeanFormat with data set #3 ('10,00', 10)
ICU error: (9) Number parsing failed: U_PARSE_ERROR
Failed asserting that false is identical to 10.0.

/app/tests/NumberFormatterTest.php:19

FAILURES!
Tests: 4, Assertions: 4, Failures: 4.

[ramsey]

BTW, U_ZERO_ERROR means there was no ICU error. I realize that looks a bit odd in the output. 😄

[alexandrunastase]

I submitted a PR to your repo that adds the ICU error message to the test failure output: alexandrunastase/reproduce-number-formatter-issue#1

PHPUnit 9.6.13 by Sebastian Bergmann and contributors.

FFFF                                                                4 / 4 (100%)

Time: 00:00.014, Memory: 6.00 MB

There were 4 failures:

1) NumberFormatterTest::testEuropeanFormat with data set #0 ('100,01', 100.01)
ICU error: (9) Number parsing failed: U_PARSE_ERROR
Failed asserting that false is identical to 100.01.

/app/tests/NumberFormatterTest.php:19

2) NumberFormatterTest::testEuropeanFormat with data set #1 ('1.100,01', 1100.01)
ICU error: (0) U_ZERO_ERROR
Failed asserting that 1.1 is identical to 1100.01.

/app/tests/NumberFormatterTest.php:19

3) NumberFormatterTest::testEuropeanFormat with data set #2 ('1.000.100,01', 1000100.01)
ICU error: (0) U_ZERO_ERROR
Failed asserting that 1.0 is identical to 1000100.01.

/app/tests/NumberFormatterTest.php:19

4) NumberFormatterTest::testEuropeanFormat with data set #3 ('10,00', 10)
ICU error: (9) Number parsing failed: U_PARSE_ERROR
Failed asserting that false is identical to 10.0.

/app/tests/NumberFormatterTest.php:19

FAILURES!
Tests: 4, Assertions: 4, Failures: 4.

@ramsey PR is merged. Thanks for taking a look into this 👍