Rounding in NumberFormatter (#2399)

Fix #2385. NumberFormatter is using sprintf on a float, and is seeing inconsistent rounding as a result (it will also occasionally result in `-0`). Change to round the number before presenting it to sprintf.
This commit is contained in:
oleibman 2021-11-26 09:05:35 -08:00 committed by GitHub
parent a561385efb
commit 3257ae5c90
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 51 additions and 40 deletions

View File

@ -5995,36 +5995,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/FractionFormatter.php
-
message: "#^Cannot cast mixed to int\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
message: "#^Cannot cast mixed to string\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
message: "#^Parameter \\#1 \\$number of function abs expects float\\|int\\|string, mixed given\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
message: "#^Parameter \\#1 \\$number of function number_format expects float, mixed given\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
message: "#^Parameter \\#1 \\$x of function fmod expects float, mixed given\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
message: "#^Parameter \\#2 \\.\\.\\.\\$values of function sprintf expects bool\\|float\\|int\\|string\\|null, mixed given\\.$#"
count: 2
path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\PercentageFormatter\\:\\:format\\(\\) has parameter \\$value with no type specified\\.$#"
count: 1

View File

@ -33,6 +33,7 @@ class NumberFormatter
*/
private static function processComplexNumberFormatMask($number, string $mask): string
{
/** @var string */
$result = $number;
$maskingBlockCount = preg_match_all('/0+/', $mask, $maskingBlocks, PREG_OFFSET_CAPTURE);
@ -45,8 +46,10 @@ class NumberFormatter
$divisor = 10 ** $size;
$offset = $block[1];
$blockValue = sprintf("%0{$size}d", fmod($number, $divisor));
$number = floor($number / $divisor);
/** @var float */
$numberFloat = $number;
$blockValue = sprintf("%0{$size}d", fmod($numberFloat, $divisor));
$number = floor($numberFloat / $divisor);
$mask = substr_replace($mask, $blockValue, $offset, $size);
}
if ($number > 0) {
@ -64,7 +67,9 @@ class NumberFormatter
private static function complexNumberFormatMask($number, string $mask, bool $splitOnPoint = true): string
{
$sign = ($number < 0.0) ? '-' : '';
$number = (string) abs($number);
/** @var float */
$numberFloat = $number;
$number = (string) abs($numberFloat);
if ($splitOnPoint && strpos($mask, '.') !== false && strpos($number, '.') !== false) {
$numbers = explode('.', $number);
@ -88,6 +93,8 @@ class NumberFormatter
*/
private static function formatStraightNumericValue($value, string $format, array $matches, bool $useThousands): string
{
/** @var float */
$valueFloat = $value;
$left = $matches[1];
$dec = $matches[2];
$right = $matches[3];
@ -96,7 +103,7 @@ class NumberFormatter
$minWidth = strlen($left) + strlen($dec) + strlen($right);
if ($useThousands) {
$value = number_format(
$value,
$valueFloat,
strlen($right),
StringHelper::getDecimalSeparator(),
StringHelper::getThousandsSeparator()
@ -107,9 +114,9 @@ class NumberFormatter
if (preg_match('/[0#]E[+-]0/i', $format)) {
// Scientific format
return sprintf('%5.2E', $value);
return sprintf('%5.2E', $valueFloat);
} elseif (preg_match('/0([^\d\.]+)0/', $format) || substr_count($format, '.') > 1) {
if ($value == (int) $value && substr_count($format, '.') === 1) {
if ($value == (int) $valueFloat && substr_count($format, '.') === 1) {
$value *= 10 ** strlen(explode('.', $format)[1]);
}
@ -117,7 +124,9 @@ class NumberFormatter
}
$sprintf_pattern = "%0$minWidth." . strlen($right) . 'f';
$value = sprintf($sprintf_pattern, $value);
/** @var float */
$valueFloat = $value;
$value = sprintf($sprintf_pattern, round($valueFloat, strlen($right)));
return self::pregReplace(self::NUMBER_REGEX, $value, $format);
}
@ -196,15 +205,15 @@ class NumberFormatter
}
/**
* @param mixed $value
* @param array|string $value
*/
private static function makeString($value): string
{
return is_array($value) ? '' : (string) $value;
return is_array($value) ? '' : "$value";
}
private static function pregReplace(string $pattern, string $replacement, string $subject): string
{
return self::makeString(preg_replace($pattern, $replacement, $subject));
return self::makeString(preg_replace($pattern, $replacement, $subject) ?? '');
}
}

View File

@ -0,0 +1,32 @@
<?php
namespace PhpOffice\PhpSpreadsheetTests\Style;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PHPUnit\Framework\TestCase;
class NumberFormatRoundTest extends TestCase
{
public static function testRound(): void
{
// Inconsistent rounding due to letting sprintf do it rather than round.
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->getStyle('A1:H2')->getNumberFormat()->setFormatCode('0');
$sheet->getStyle('A3:H3')->getNumberFormat()->setFormatCode('0.0');
$sheet->fromArray(
[
[-3.5, -2.5, -1.5, -0.5, 0.5, 1.5, 2.5, 3.5],
[-3.1, -2.9, -1.4, -0.3, 0.7, 1.6, 2.4, 3.7],
[-3.15, -2.85, -1.43, -0.87, 0.72, 1.60, 2.45, 3.75],
]
);
$expected = [
[-4, -3, -2, -1, 1, 2, 3, 4],
[-3, -3, -1, 0, 1, 2, 2, 4],
[-3.2, -2.9, -1.4, -0.9, 0.7, 1.6, 2.5, 3.8],
];
self::assertEquals($expected, $sheet->toArray());
$spreadsheet->disconnectWorksheets();
}
}