Fraction Formatting (#2254)

* Fraction Formatting See issue #2253. User's analysis was correct - leading zeros in the decimal portion were being stripped out, so 0.0625 and 0.625 were being treated the same. As it turns out, integers also aren't handled well (`0 0/1` anyone?). The latter problem had been hidden because caller tested for integer first and skipped call if true; but FractionFormatter::format is public and should work correctly regardless. All Phpstan baseline entries for FractionFormatter and NumberFormatter are eliminated. New test data is added; no need for changes to test code. * Scrutinizer Ensure result is string.
PHPOffice · Aug 18, 2021 · 710f9f1 · 710f9f1
1 parent d007634
commit 710f9f1
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 121 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -4590,101 +4590,6 @@ parameters:
 			count: 1
 			path: src/PhpSpreadsheet/Style/NumberFormat/Formatter.php
 
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\FractionFormatter\\:\\:format\\(\\) has parameter \\$value with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/FractionFormatter.php
-
-		-
-			message: "#^Parameter \\#1 \\$str of function trim expects string, float given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/FractionFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:mergeComplexNumberFormatMasks\\(\\) has parameter \\$masks with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:mergeComplexNumberFormatMasks\\(\\) has parameter \\$numbers with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:processComplexNumberFormatMask\\(\\) has parameter \\$mask with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:processComplexNumberFormatMask\\(\\) has parameter \\$number with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:complexNumberFormatMask\\(\\) has parameter \\$mask with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:complexNumberFormatMask\\(\\) has parameter \\$number with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:complexNumberFormatMask\\(\\) has parameter \\$splitOnPoint with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Parameter \\#1 \\$haystack of function strpos expects string, float\\|int given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Parameter \\#2 \\$str of function explode expects string, float\\|int given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:formatStraightNumericValue\\(\\) has parameter \\$format with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:formatStraightNumericValue\\(\\) has parameter \\$useThousands with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:formatStraightNumericValue\\(\\) has parameter \\$value with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:format\\(\\) has parameter \\$format with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\NumberFormatter\\:\\:format\\(\\) has parameter \\$value with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Parameter \\#3 \\$subject of function preg_replace expects array\\|string, array\\|string\\|null given\\.$#"
-			count: 6
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Parameter \\#2 \\$subject of function preg_match expects string, array\\|string\\|null given\\.$#"
-			count: 4
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
-		-
-			message: "#^Parameter \\#2 \\$format of static method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\FractionFormatter\\:\\:format\\(\\) expects string, array\\|string\\|null given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
-
 		-
 			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\PercentageFormatter\\:\\:format\\(\\) has parameter \\$value with no typehint specified\\.$#"
 			count: 1

diff --git a/src/PhpSpreadsheet/Style/NumberFormat/FractionFormatter.php b/src/PhpSpreadsheet/Style/NumberFormat/FractionFormatter.php
@@ -6,14 +6,23 @@
 
 class FractionFormatter extends BaseFormatter
 {
+    /**
+     * @param mixed $value
+     */
     public static function format($value, string $format): string
     {
         $format = self::stripQuotes($format);
+        $value = (float) $value;
+        $absValue = abs($value);
 
         $sign = ($value < 0.0) ? '-' : '';
 
-        $integerPart = floor(abs($value));
-        $decimalPart = trim(fmod(abs($value), 1), '0.');
+        $integerPart = floor($absValue);
+
+        $decimalPart = self::getDecimal((string) $absValue);
+        if ($decimalPart === '0') {
+            return "{$sign}{$integerPart}";
+        }
         $decimalLength = strlen($decimalPart);
         $decimalDivisor = 10 ** $decimalLength;
 
@@ -42,4 +51,14 @@ public static function format($value, string $format): string
 
         return "{$sign}{$adjustedDecimalPart}/{$adjustedDecimalDivisor}";
     }
+
+    private static function getDecimal(string $value): string
+    {
+        $decimalPart = '0';
+        if (preg_match('/^\\d*[.](\\d*[1-9])0*$/', $value, $matches) === 1) {
+            $decimalPart = $matches[1];
+        }
+
+        return $decimalPart;
+    }
 }
diff --git a/src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php b/src/PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php
@@ -9,7 +9,7 @@ class NumberFormatter
 {
     private const NUMBER_REGEX = '/(0+)(\\.?)(0*)/';
 
-    private static function mergeComplexNumberFormatMasks($numbers, $masks): array
+    private static function mergeComplexNumberFormatMasks(array $numbers, array $masks): array
     {
         $decimalCount = strlen($numbers[1]);
         $postDecimalMasks = [];
@@ -28,7 +28,10 @@ private static function mergeComplexNumberFormatMasks($numbers, $masks): array
         ];
     }
 
-    private static function processComplexNumberFormatMask($number, $mask): string
+    /**
+     * @param mixed $number
+     */
+    private static function processComplexNumberFormatMask($number, string $mask): string
     {
         $result = $number;
         $maskingBlockCount = preg_match_all('/0+/', $mask, $maskingBlocks, PREG_OFFSET_CAPTURE);
@@ -52,13 +55,16 @@ private static function processComplexNumberFormatMask($number, $mask): string
             $result = $mask;
         }
 
-        return $result;
+        return self::makeString($result);
     }
 
-    private static function complexNumberFormatMask($number, $mask, $splitOnPoint = true): string
+    /**
+     * @param mixed $number
+     */
+    private static function complexNumberFormatMask($number, string $mask, bool $splitOnPoint = true): string
     {
         $sign = ($number < 0.0) ? '-' : '';
-        $number = abs($number);
+        $number = (string) abs($number);
 
         if ($splitOnPoint && strpos($mask, '.') !== false && strpos($number, '.') !== false) {
             $numbers = explode('.', $number);
@@ -77,7 +83,10 @@ private static function complexNumberFormatMask($number, $mask, $splitOnPoint =
         return "{$sign}{$result}";
     }
 
-    private static function formatStraightNumericValue($value, $format, array $matches, $useThousands): string
+    /**
+     * @param mixed $value
+     */
+    private static function formatStraightNumericValue($value, string $format, array $matches, bool $useThousands): string
     {
         $left = $matches[1];
         $dec = $matches[2];
@@ -93,7 +102,7 @@ private static function formatStraightNumericValue($value, $format, array $match
                 StringHelper::getThousandsSeparator()
             );
 
-            return preg_replace(self::NUMBER_REGEX, $value, $format);
+            return self::pregReplace(self::NUMBER_REGEX, $value, $format);
         }
 
         if (preg_match('/[0#]E[+-]0/i', $format)) {
@@ -110,10 +119,13 @@ private static function formatStraightNumericValue($value, $format, array $match
         $sprintf_pattern = "%0$minWidth." . strlen($right) . 'f';
         $value = sprintf($sprintf_pattern, $value);
 
-        return preg_replace(self::NUMBER_REGEX, $value, $format);
+        return self::pregReplace(self::NUMBER_REGEX, $value, $format);
     }
 
-    public static function format($value, $format): string
+    /**
+     * @param mixed $value
+     */
+    public static function format($value, string $format): string
     {
         // The "_" in this string has already been stripped out,
         // so this test is never true. Furthermore, testing
@@ -123,15 +135,15 @@ public static function format($value, $format): string
         //}
 
         // Some non-number strings are quoted, so we'll get rid of the quotes, likewise any positional * symbols
-        $format = str_replace(['"', '*'], '', $format);
+        $format = self::makeString(str_replace(['"', '*'], '', $format));
 
         // Find out if we need thousands separator
         // This is indicated by a comma enclosed by a digit placeholder:
         //        #,#   or   0,0
-        $useThousands = preg_match('/(#,#|0,0)/', $format);
+        $useThousands = (bool) preg_match('/(#,#|0,0)/', $format);
         if ($useThousands) {
-            $format = preg_replace('/0,0/', '00', $format);
-            $format = preg_replace('/#,#/', '##', $format);
+            $format = self::pregReplace('/0,0/', '00', $format);
+            $format = self::pregReplace('/#,#/', '##', $format);
         }
 
         // Scale thousands, millions,...
@@ -143,32 +155,30 @@ public static function format($value, $format): string
             $scale = 1000 ** strlen($matches[2]);
 
             // strip the commas
-            $format = preg_replace('/0,+/', '0', $format);
-            $format = preg_replace('/#,+/', '#', $format);
+            $format = self::pregReplace('/0,+/', '0', $format);
+            $format = self::pregReplace('/#,+/', '#', $format);
         }
         if (preg_match('/#?.*\?\/\?/', $format, $m)) {
-            if ($value != (int) $value) {
-                $value = FractionFormatter::format($value, $format);
-            }
+            $value = FractionFormatter::format($value, $format);
         } else {
             // Handle the number itself
 
             // scale number
             $value = $value / $scale;
             // Strip #
-            $format = preg_replace('/\\#/', '0', $format);
+            $format = self::pregReplace('/\\#/', '0', $format);
             // Remove locale code [$-###]
-            $format = preg_replace('/\[\$\-.*\]/', '', $format);
+            $format = self::pregReplace('/\[\$\-.*\]/', '', $format);
 
             $n = '/\\[[^\\]]+\\]/';
-            $m = preg_replace($n, '', $format);
+            $m = self::pregReplace($n, '', $format);
             if (preg_match(self::NUMBER_REGEX, $m, $matches)) {
                 // There are placeholders for digits, so inject digits from the value into the mask
                 $value = self::formatStraightNumericValue($value, $format, $matches, $useThousands);
             } elseif ($format !== NumberFormat::FORMAT_GENERAL) {
                 // Yes, I know that this is basically just a hack;
                 //      if there's no placeholders for digits, just return the format mask "as is"
-                $value = str_replace('?', '', $format ?? '');
+                $value = self::makeString(str_replace('?', '', $format));
             }
         }
 
@@ -179,9 +189,22 @@ public static function format($value, $format): string
             if ($currencyCode == '') {
                 $currencyCode = StringHelper::getCurrencyCode();
             }
-            $value = preg_replace('/\[\$([^\]]*)\]/u', $currencyCode, $value);
+            $value = self::pregReplace('/\[\$([^\]]*)\]/u', $currencyCode, (string) $value);
         }
 
-        return $value;
+        return (string) $value;
+    }
+
+    /**
+     * @param mixed $value
+     */
+    private static function makeString($value): string
+    {
+        return is_array($value) ? '' : (string) $value;
+    }
+
+    private static function pregReplace(string $pattern, string $replacement, string $subject): string
+    {
+        return self::makeString(preg_replace($pattern, $replacement, $subject));
     }
 }
diff --git a/tests/data/Style/NumberFormat.php b/tests/data/Style/NumberFormat.php
@@ -297,6 +297,36 @@
         0.75,
         '? ??/???',
     ],
+    [
+        ' 3/4',
+        '0.75000',
+        '? ??/???',
+    ],
+    [
+        '5 1/16',
+        5.0625,
+        '? ??/???',
+    ],
+    [
+        '- 5/8',
+        -0.625,
+        '? ??/???',
+    ],
+    [
+        '0',
+        0,
+        '? ??/???',
+    ],
+    [
+        '0',
+        '0.000',
+        '? ??/???',
+    ],
+    [
+        '-16',
+        '-016.0',
+        '? ??/???',
+    ],
     // Complex formats
     [
         '(001) 2-3456-789',