TextData - Minor Changes, Test Coverage

Per agreement on a previous push, I looked into standardizing the initialization of the TextData functions (like Engineering and MathTrig), with particular regard for avoiding multiple later null coercions. This simplifies the code quite a bit. This PR also increases coverage to 100% for all TextData modules. All entries in Phpstan baseline for non-deprecated TEXTDATA functions are removed. There were some minor bugfixes. Whereas Excel (and Gnumeric) treat booleans when supplied as strings as 'TRUE' or 'FALSE', ODS treats them as '1' or '0'. Unlike Excel, ODS generally does not allow bool for int arguments; it does, however, allow them for FIND and SEARCH. ODS allows boolean for into for SUBSTITUTE even though Excel doesn't. ODS allows bool for string for NUMBERVALUE and VALUE even though Excel doesn't. ODS accepts 0 as an argument for CHAR; Excel doesn't. Most of this seems like random decisions on the part of the developers; I've done my best to follow the products in each case. There is a new test member devoted to ODS tests. Gnumeric has an anomaly vis-a-vis the others - if length is supplied to LEFT/MID/RIGHT as null, Gnumeric treats it as 0 rather than 1. All tests now take place in the context of a spreadsheet ... Except for RETURNSTRING, which is not the implementation of an Excel function, and is referred to in the rest of PhpSpreadsheet only in the unit tests for itself. It should probably be deprecated, but that is not part of this PR, just in case there is some reason for it that I couldn't discern. I have tried to make the first line of each doc block identify the Excel function name rather than its name in PhpSpreadsheet. I think it makes things more comprehensible. Some tests call Settings::setLocale, but there was no Settings::getLocale. At the end of the tests which do it, they invoke setLocale('EN-US'), which, in a practical sense, is sufficient. However, in theory it would be better for them to get the current locale before changing it, then changing it back to the original when the time came. I have added getLocale and made the appropriate testing change. The CHAR function took an interesting turn. One can set the value of a cell to, say, CHAR(2), the ASCII/UTF-8 representation of a control character, which is not legal in certain contexts. The only Reader/Writer that could handle this without problems is Xls, which deals with binary data all the time. However, if you tried to write it to Xlsx, Excel would not be able to open the resulting file because of what it considers an illegal character. I changed the Xlsx writer to escape such characters when writing the value of a string function. I did not make any other changes to the Xlsx writer - it seems to me that setting a cell to CHAR(2) is legitimate, but setting it to say `"\x02"` seems less likely to be legitimate, so the latter will still fail (although `="\x02"` should work). The Xlsx reader already supports the escape mechanism that I added to the writer. CHAR control character and Ods - not supported by either Reader or Writer. I did not attempt to add this now. There is lots still missing from ODS, and this item just can't be a high priority amongst all of those. CHAR control character and Csv - it is supported by reader and writer if the file has a csv extension. However, trying to guess the mime type without an extension - the control character makes mime_get_type guess application/octet-stream, and PhpSpreadsheet therefore thinks that Csv can't read it. CHAR control character and Html. Actual use of the control character in the file is subject to the same problems as Xml (i.e. Xlsx and Ods). It wasn't terribly difficult to get the Html Writer to change `"\x02"` to "``". I believe that this is technically legal; however, DOMDocument.loadHTML rejects it as an illegal entity, and I am not convinced that it is wrong to do so, so I haven't changed the Html writer.
oleibman · Jun 10, 2021 · 6ae2a89 · 6ae2a89
1 parent da8f421
commit 6ae2a89
Show file tree

Hide file tree

Showing 66 changed files with 1,429 additions and 563 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -1320,11 +1320,6 @@ parameters:
 			count: 1
 			path: src/PhpSpreadsheet/Calculation/TextData.php
 
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\TextData\\:\\:TRIMSPACES\\(\\) should return string but returns string\\|null\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData.php
-
 		-
 			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\TextData\\:\\:CONCATENATE\\(\\) has parameter \\$args with no typehint specified\\.$#"
 			count: 1
@@ -1340,96 +1335,6 @@ parameters:
 			count: 1
 			path: src/PhpSpreadsheet/Calculation/TextData.php
 
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\TextData\\\\CharacterConvert\\:\\:character\\(\\) should return string but returns string\\|false\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\TextData\\\\CharacterConvert\\:\\:unicodeToOrd\\(\\) has no return typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\TextData\\\\CharacterConvert\\:\\:unicodeToOrd\\(\\) has parameter \\$character with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php
-
-		-
-			message: "#^Cannot access offset 1 on array\\|false\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php
-
-		-
-			message: "#^Parameter \\#2 \\$data of function unpack expects string, string\\|false given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\TextData\\\\CharacterConvert\\:\\:convertBooleanValue\\(\\) has no return typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\TextData\\\\CharacterConvert\\:\\:convertBooleanValue\\(\\) has parameter \\$value with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\TextData\\\\Concatenate\\:\\:CONCATENATE\\(\\) has parameter \\$args with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/Concatenate.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\TextData\\\\Concatenate\\:\\:convertBooleanValue\\(\\) has no return typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/Concatenate.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\TextData\\\\Concatenate\\:\\:convertBooleanValue\\(\\) has parameter \\$value with no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/Concatenate.php
-
-		-
-			message: "#^Parameter \\#3 \\$length of function mb_substr expects int\\|null, float\\|int\\<0, max\\>\\|string given\\.$#"
-			count: 3
-			path: src/PhpSpreadsheet/Calculation/TextData/Extract.php
-
-		-
-			message: "#^Parameter \\#2 \\$start of function mb_substr expects int, float\\|int given\\.$#"
-			count: 2
-			path: src/PhpSpreadsheet/Calculation/TextData/Extract.php
-
-		-
-			message: "#^Cannot cast array\\|float\\|int\\|string to float\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/Format.php
-
-		-
-			message: "#^Parameter \\#3 \\$offset of function mb_strpos expects int, float\\|int given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/Search.php
-
-		-
-			message: "#^Parameter \\#3 \\$offset of function mb_stripos expects int, float\\|int given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/Search.php
-
-		-
-			message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\TextData\\\\Trim\\:\\:\\$invalidChars has no typehint specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/Trim.php
-
-		-
-			message: "#^Parameter \\#1 \\$str of function trim expects string, float\\|int\\|string given\\.$#"
-			count: 2
-			path: src/PhpSpreadsheet/Calculation/TextData/Trim.php
-
-		-
-			message: "#^Parameter \\#1 \\$str of function trim expects string, string\\|null given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/TextData/Trim.php
-
 		-
 			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Token\\\\Stack\\:\\:getStackItem\\(\\) has no return typehint specified\\.$#"
 			count: 1

diff --git a/src/PhpSpreadsheet/Calculation/TextData/CaseConvert.php b/src/PhpSpreadsheet/Calculation/TextData/CaseConvert.php
@@ -2,7 +2,6 @@
 
 namespace PhpOffice\PhpSpreadsheet\Calculation\TextData;
 
-use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
 use PhpOffice\PhpSpreadsheet\Calculation\Functions;
 use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
 
@@ -18,10 +17,7 @@ class CaseConvert
     public static function lower($mixedCaseValue): string
     {
         $mixedCaseValue = Functions::flattenSingleValue($mixedCaseValue);
-
-        if (is_bool($mixedCaseValue)) {
-            $mixedCaseValue = ($mixedCaseValue === true) ? Calculation::getTRUE() : Calculation::getFALSE();
-        }
+        $mixedCaseValue = Helpers::extractString($mixedCaseValue);
 
         return StringHelper::strToLower($mixedCaseValue);
     }
@@ -36,10 +32,7 @@ public static function lower($mixedCaseValue): string
     public static function upper($mixedCaseValue): string
     {
         $mixedCaseValue = Functions::flattenSingleValue($mixedCaseValue);
-
-        if (is_bool($mixedCaseValue)) {
-            $mixedCaseValue = ($mixedCaseValue === true) ? Calculation::getTRUE() : Calculation::getFALSE();
-        }
+        $mixedCaseValue = Helpers::extractString($mixedCaseValue);
 
         return StringHelper::strToUpper($mixedCaseValue);
     }
@@ -54,10 +47,7 @@ public static function upper($mixedCaseValue): string
     public static function proper($mixedCaseValue): string
     {
         $mixedCaseValue = Functions::flattenSingleValue($mixedCaseValue);
-
-        if (is_bool($mixedCaseValue)) {
-            $mixedCaseValue = ($mixedCaseValue === true) ? Calculation::getTRUE() : Calculation::getFALSE();
-        }
+        $mixedCaseValue = Helpers::extractString($mixedCaseValue);
 
         return StringHelper::strToTitle($mixedCaseValue);
     }

diff --git a/src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php b/src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php
@@ -2,48 +2,40 @@
 
 namespace PhpOffice\PhpSpreadsheet\Calculation\TextData;
 
-use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
 use PhpOffice\PhpSpreadsheet\Calculation\Functions;
 
 class CharacterConvert
 {
     /**
-     * CHARACTER.
+     * CHAR.
      *
      * @param mixed $character Integer Value to convert to its character representation
      */
     public static function character($character): string
     {
-        $character = Functions::flattenSingleValue($character);
-
-        if (!is_numeric($character)) {
-            return Functions::VALUE();
-        }
-
-        $character = (int) $character;
-        if ($character < 1 || $character > 255) {
+        $character = Helpers::validateInt($character);
+        $min = Functions::getCompatibilityMode() === Functions::COMPATIBILITY_OPENOFFICE ? 0 : 1;
+        if ($character < $min || $character > 255) {
             return Functions::VALUE();
         }
+        $result = iconv('UCS-4LE', 'UTF-8', pack('V', $character));
 
-        return iconv('UCS-4LE', 'UTF-8', pack('V', $character));
+        return ($result === false) ? '' : $result;
     }
 
     /**
-     * ASCIICODE.
+     * CODE.
      *
      * @param mixed $characters String character to convert to its ASCII value
      *
      * @return int|string A string if arguments are invalid
      */
     public static function code($characters)
     {
-        if (($characters === null) || ($characters === '')) {
+        $characters = Helpers::extractString($characters);
+        if ($characters === '') {
             return Functions::VALUE();
         }
-        $characters = Functions::flattenSingleValue($characters);
-        if (is_bool($characters)) {
-            $characters = self::convertBooleanValue($characters);
-        }
 
         $character = $characters;
         if (mb_strlen($characters, 'UTF-8') > 1) {
@@ -53,17 +45,17 @@ public static function code($characters)
         return self::unicodeToOrd($character);
     }
 
-    private static function unicodeToOrd($character)
-    {
-        return unpack('V', iconv('UTF-8', 'UCS-4LE', $character))[1];
-    }
-
-    private static function convertBooleanValue($value)
+    private static function unicodeToOrd(string $character): int
     {
-        if (Functions::getCompatibilityMode() == Functions::COMPATIBILITY_OPENOFFICE) {
-            return (int) $value;
+        $retVal = 0;
+        $iconv = iconv('UTF-8', 'UCS-4LE', $character);
+        if ($iconv !== false) {
+            $result = unpack('V', $iconv);
+            if (is_array($result) && isset($result[1])) {
+                $retVal = $result[1];
+            }
         }
 
-        return ($value) ? Calculation::getTRUE() : Calculation::getFALSE();
+        return $retVal;
     }
 }
diff --git a/src/PhpSpreadsheet/Calculation/TextData/Concatenate.php b/src/PhpSpreadsheet/Calculation/TextData/Concatenate.php
@@ -2,25 +2,24 @@
 
 namespace PhpOffice\PhpSpreadsheet\Calculation\TextData;
 
-use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
 use PhpOffice\PhpSpreadsheet\Calculation\Functions;
 
 class Concatenate
 {
     /**
      * CONCATENATE.
+     *
+     * @param array $args
      */
     public static function CONCATENATE(...$args): string
     {
         $returnValue = '';
 
         // Loop through arguments
         $aArgs = Functions::flattenArray($args);
+
         foreach ($aArgs as $arg) {
-            if (is_bool($arg)) {
-                $arg = self::convertBooleanValue($arg);
-            }
-            $returnValue .= $arg;
+            $returnValue .= Helpers::extractString($arg);
         }
 
         return $returnValue;
@@ -35,13 +34,15 @@ public static function CONCATENATE(...$args): string
      */
     public static function TEXTJOIN($delimiter, $ignoreEmpty, ...$args): string
     {
+        $delimiter = Functions::flattenSingleValue($delimiter);
+        $ignoreEmpty = Functions::flattenSingleValue($ignoreEmpty);
         // Loop through arguments
         $aArgs = Functions::flattenArray($args);
         foreach ($aArgs as $key => &$arg) {
             if ($ignoreEmpty === true && is_string($arg) && trim($arg) === '') {
                 unset($aArgs[$key]);
             } elseif (is_bool($arg)) {
-                $arg = self::convertBooleanValue($arg);
+                $arg = Helpers::convertBooleanValue($arg);
             }
         }
 
@@ -59,24 +60,12 @@ public static function TEXTJOIN($delimiter, $ignoreEmpty, ...$args): string
     public static function builtinREPT($stringValue, $repeatCount): string
     {
         $repeatCount = Functions::flattenSingleValue($repeatCount);
+        $stringValue = Helpers::extractString($stringValue);
 
         if (!is_numeric($repeatCount) || $repeatCount < 0) {
             return Functions::VALUE();
         }
 
-        if (is_bool($stringValue)) {
-            $stringValue = self::convertBooleanValue($stringValue);
-        }
-
         return str_repeat($stringValue, (int) $repeatCount);
     }
-
-    private static function convertBooleanValue($value)
-    {
-        if (Functions::getCompatibilityMode() === Functions::COMPATIBILITY_OPENOFFICE) {
-            return (int) $value;
-        }
-
-        return ($value) ? Calculation::getTRUE() : Calculation::getFALSE();
-    }
 }