Fix #76093, part 2: Format currency strings w/o loss of precision

cscott · lucaswerkmeister · commit f2175dcfde38 · 2023-10-13T19:45:25.000+02:00
This extends the argument types of NumberFormat::formatCurrency() to
include string, in the same way the earlier patch extended
NumberFormat::format() to allow a string.  This allows formatting
values which can't be represented precisely as a double or long
integer, using the "decimal number" type of the icu library.
diff --git a/ext/intl/formatter/formatter.stub.php b/ext/intl/formatter/formatter.stub.php
@@ -427,7 +427,7 @@ public function parse(string $string, int $type = NumberFormatter::TYPE_DOUBLE,
      * @tentative-return-type
      * @alias numfmt_format_currency
      */
-    public function formatCurrency(float $amount, string $currency): string|false {}
+    public function formatCurrency(int|float|string $amount, string $currency): string|false {}
 
     /**
      * @param string $currency
diff --git a/ext/intl/formatter/formatter_arginfo.h b/ext/intl/formatter/formatter_arginfo.h
diff --git a/ext/intl/formatter/formatter_format.c b/ext/intl/formatter/formatter_format.c
@@ -174,7 +174,7 @@ PHP_FUNCTION( numfmt_format )
 /* {{{ Format a number as currency. */
 PHP_FUNCTION( numfmt_format_currency )
 {
-	double     number;
+	zval      *number;
 	UChar      format_buf[32];
 	UChar*     formatted     = format_buf;
 	int32_t    formatted_len = USIZE(format_buf);
@@ -184,11 +184,20 @@ PHP_FUNCTION( numfmt_format_currency )
 	int32_t    scurrency_len = 0;
 	FORMATTER_METHOD_INIT_VARS;
 
+	object = getThis();
+
 	/* Parse parameters. */
-	if( zend_parse_method_parameters( ZEND_NUM_ARGS(), getThis(), "Ods",
-		&object, NumberFormatter_ce_ptr,  &number, &currency, &currency_len ) == FAILURE )
-	{
-		RETURN_THROWS();
+	if (object) {
+		ZEND_PARSE_PARAMETERS_START(2, 2)
+			Z_PARAM_STR_OR_NUMBER(number)
+			Z_PARAM_STRING(currency, currency_len)
+		ZEND_PARSE_PARAMETERS_END();
+	} else {
+		ZEND_PARSE_PARAMETERS_START(3, 3)
+			Z_PARAM_OBJECT_OF_CLASS(object, NumberFormatter_ce_ptr)
+			Z_PARAM_STR_OR_NUMBER(number)
+			Z_PARAM_STRING(currency, currency_len)
+		ZEND_PARSE_PARAMETERS_END();
 	}
 
 	/* Fetch the object. */
@@ -197,9 +206,16 @@ PHP_FUNCTION( numfmt_format_currency )
 	/* Convert currency to UTF-16. */
 	intl_convert_utf8_to_utf16(&scurrency, &scurrency_len, currency, currency_len, &INTL_DATA_ERROR_CODE(nfo));
 	INTL_METHOD_CHECK_STATUS( nfo, "Currency conversion to UTF-16 failed" );
+	unum_setTextAttribute(FORMATTER_OBJECT(nfo), UNUM_CURRENCY_CODE, scurrency, scurrency_len, &INTL_DATA_ERROR_CODE(nfo));
+	INTL_METHOD_CHECK_STATUS( nfo, "Setting currency code failed" );
 
 	/* Format the number using a fixed-length buffer. */
-	formatted_len = unum_formatDoubleCurrency(FORMATTER_OBJECT(nfo), number, scurrency, formatted, formatted_len, NULL, &INTL_DATA_ERROR_CODE(nfo));
+	if (Z_TYPE_P(number) == IS_STRING) {
+		formatted_len = unum_formatDecimal(FORMATTER_OBJECT(nfo), Z_STRVAL_P(number), Z_STRLEN_P(number), formatted, formatted_len, NULL, &INTL_DATA_ERROR_CODE(nfo));
+	} else {
+		convert_to_double(number);
+		formatted_len = unum_formatDoubleCurrency(FORMATTER_OBJECT(nfo), Z_DVAL_P(number), scurrency, formatted, formatted_len, NULL, &INTL_DATA_ERROR_CODE(nfo));
+	}
 
 	/* If the buffer turned out to be too small
 	 * then allocate another buffer dynamically
@@ -208,7 +224,11 @@ PHP_FUNCTION( numfmt_format_currency )
 	if (INTL_DATA_ERROR_CODE(nfo) == U_BUFFER_OVERFLOW_ERROR) {
 		intl_error_reset(INTL_DATA_ERROR_P(nfo));
 		formatted = eumalloc(formatted_len);
-		unum_formatDoubleCurrency(FORMATTER_OBJECT(nfo), number, scurrency, formatted, formatted_len, NULL, &INTL_DATA_ERROR_CODE(nfo));
+		if (Z_TYPE_P(number) == IS_STRING) {
+			unum_formatDecimal(FORMATTER_OBJECT(nfo), Z_STRVAL_P(number), Z_STRLEN_P(number), formatted, formatted_len, NULL, &INTL_DATA_ERROR_CODE(nfo));
+		} else {
+			unum_formatDoubleCurrency(FORMATTER_OBJECT(nfo), Z_DVAL_P(number), scurrency, formatted, formatted_len, NULL, &INTL_DATA_ERROR_CODE(nfo));
+		}
 	}
 
 	if( U_FAILURE( INTL_DATA_ERROR_CODE((nfo)) ) ) {
diff --git a/ext/intl/php_intl.stub.php b/ext/intl/php_intl.stub.php
@@ -395,7 +395,7 @@ function numfmt_format(NumberFormatter $formatter, int|float|string $num, int $t
 /** @param int $offset */
 function numfmt_parse(NumberFormatter $formatter, string $string, int $type = NumberFormatter::TYPE_DOUBLE, &$offset = null): int|float|false {}
 
-function numfmt_format_currency(NumberFormatter $formatter, float $amount, string $currency): string|false {}
+function numfmt_format_currency(NumberFormatter $formatter, int|float|string $amount, string $currency): string|false {}
 
 /**
  * @param string $currency
diff --git a/ext/intl/php_intl_arginfo.h b/ext/intl/php_intl_arginfo.h
diff --git a/ext/intl/tests/bug76093.phpt b/ext/intl/tests/bug76093.phpt
@@ -7,6 +7,7 @@ Bug #76093 (NumberFormatter::format loses precision)
 
 # See also https://phabricator.wikimedia.org/T268456
 $x = new NumberFormatter('en_US', NumberFormatter::DECIMAL);
+$x2 = new NumberFormatter('en_US', NumberFormatter::CURRENCY);
 foreach ([
     '999999999999999999', # Fits in signed 64-bit integer
     '9999999999999999999', # Does not fit in signed 64-bit integer
@@ -25,6 +26,9 @@ foreach ([
             'int64' => $x->format($value, NumberFormatter::TYPE_INT64),
             'double' => $x->format($value, NumberFormatter::TYPE_DOUBLE),
             'decimal' => $x->format($value, NumberFormatter::TYPE_DECIMAL),
+            # formatCurrency requires the NumberFormatter to be created with
+            # the CURRENCY or CURRENCY_ACCOUNTING style.
+            'currency' => $x2->formatCurrency($value, 'USD'),
         ]);
     } catch (TypeError $ex) {
         echo $ex->getMessage(), PHP_EOL;
@@ -33,7 +37,7 @@ foreach ([
 
 ?>
 --EXPECTF--
-array(5) {
+array(6) {
   ["input"]=>
   string(18) "999999999999999999"
   ["default"]=>
@@ -44,8 +48,10 @@ array(5) {
   string(25) "1,000,000,000,000,000,000"
   ["decimal"]=>
   string(23) "999,999,999,999,999,999"
+  ["currency"]=>
+  string(27) "$999,999,999,999,999,999.00"
 }
-array(5) {
+array(6) {
   ["input"]=>
   string(19) "9999999999999999999"
   ["default"]=>
@@ -56,8 +62,10 @@ array(5) {
   string(26) "10,000,000,000,000,000,000"
   ["decimal"]=>
   string(25) "9,999,999,999,999,999,999"
+  ["currency"]=>
+  string(29) "$9,999,999,999,999,999,999.00"
 }
-array(5) {
+array(6) {
   ["input"]=>
   float(1.0E+19)
   ["default"]=>
@@ -68,4 +76,6 @@ array(5) {
   string(26) "10,000,000,000,000,000,000"
   ["decimal"]=>
   string(26) "10,000,000,000,000,000,000"
+  ["currency"]=>
+  string(30) "$10,000,000,000,000,000,000.00"
 }
diff --git a/ext/intl/tests/bug78912.phpt b/ext/intl/tests/bug78912.phpt
@@ -10,6 +10,13 @@ if (version_compare(INTL_ICU_VERSION, '53.0') < 0) die('skip for ICU >= 53.0');
 <?php
 $nf = new NumberFormatter('en_US', NumberFormatter::CURRENCY_ACCOUNTING);
 var_dump($nf->formatCurrency(-12345.67, 'USD'));
+# Also some tests for bug #76093 while we're at it.
+var_dump($nf->formatCurrency(9999999999999999999, 'USD')); # gets rounded
+var_dump($nf->formatCurrency('-12345.67', 'USD'));
+var_dump($nf->formatCurrency('9999999999999999999', 'USD')); # not rounded!
 ?>
 --EXPECT--
 string(12) "($12,345.67)"
+string(30) "$10,000,000,000,000,000,000.00"
+string(12) "($12,345.67)"
+string(29) "$9,999,999,999,999,999,999.00"
