From 68a92d0119da6a58cfb8fe9992c5e5ea78f3bc02 Mon Sep 17 00:00:00 2001 From: Owen Leibman Date: Thu, 29 Apr 2021 17:37:47 -0700 Subject: [PATCH] Fix for Issue 2042 (SUM Partially Broken) As issue #2042 documents, SUM behaves differently with invalid strings depending on whether they come from a cell or are used as literals in the formula. SUM is not alone in this regard; COUNTA is another function within this behavior, and the solution to this one is modeled on COUNTA. New tests are added for SUM, and the resulting tests are duplicated to confirm correct behavior for both cells and literals. Samples 16 (CSV), 17 (Html), and 21 (PDF) were adversely affected by this problem. 17 and 21 were immediately fixed, but 16 had another problem - Excel was not interpreting the UTF8 currency symbols correctly, even though the file was saved with a BOM. After some experimenting, it appears that the `sep=;` line generated by setExcelCompatibility(true) causes Excel to mis-handle the file. This seems like a bug - there is apparently no way to save a UTF-8 CSV with non-ASCII characters which specifies a non-standard separator which Excel will open correctly. I don't know if this is a recent change or if it is just the case that nobody noticed this problem till now. So, I changed Sample 16 to use setUseBom rather than setExcelCompatibility, which solved its problem. I then added new tests for setExcelCompatibility, with documentation of this problem. --- samples/Basic/16_Csv.php | 3 +- .../Calculation/MathTrig/Sum.php | 8 ++- .../Functions/MathTrig/SumTest.php | 18 +++++++ .../Writer/Csv/CsvExcelCompatibilityTest.php | 49 +++++++++++++++++++ tests/data/Calculation/MathTrig/SUM.php | 6 ++- .../data/Calculation/MathTrig/SUMLITERALS.php | 12 +++++ 6 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Writer/Csv/CsvExcelCompatibilityTest.php create mode 100644 tests/data/Calculation/MathTrig/SUMLITERALS.php diff --git a/samples/Basic/16_Csv.php b/samples/Basic/16_Csv.php index 15bbf0d456..137f6469c1 100644 --- a/samples/Basic/16_Csv.php +++ b/samples/Basic/16_Csv.php @@ -38,7 +38,8 @@ $filenameCSV = $helper->getFilename(__FILE__, 'csv'); /** @var \PhpOffice\PhpSpreadsheet\Writer\Csv $writerCSV */ $writerCSV = new CsvWriter($spreadsheetFromCSV); -$writerCSV->setExcelCompatibility(true); +//$writerCSV->setExcelCompatibility(true); +$writerCSV->setUseBom(true); // because of non-ASCII chars $callStartTime = microtime(true); $writerCSV->save($filenameCSV); diff --git a/src/PhpSpreadsheet/Calculation/MathTrig/Sum.php b/src/PhpSpreadsheet/Calculation/MathTrig/Sum.php index cd29248bd6..e881933d9a 100644 --- a/src/PhpSpreadsheet/Calculation/MathTrig/Sum.php +++ b/src/PhpSpreadsheet/Calculation/MathTrig/Sum.php @@ -52,13 +52,17 @@ public static function funcSumNoStrings(...$args) $returnValue = 0; // Loop through the arguments - foreach (Functions::flattenArray($args) as $arg) { + $aArgs = Functions::flattenArrayIndexed($args); + foreach ($aArgs as $k => $arg) { // Is it a numeric value? if (is_numeric($arg)) { $returnValue += $arg; + } elseif (is_bool($arg)) { + $returnValue += (int) $arg; } elseif (Functions::isError($arg)) { return $arg; - } else { + // ignore non-numerics from cell, but fail as literals (except null) + } elseif ($arg !== null && !Functions::isCellValue($k)) { return Functions::VALUE(); } } diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/SumTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/SumTest.php index a9ea7f2986..b85f0c9071 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/SumTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/SumTest.php @@ -26,4 +26,22 @@ public function providerSUM(): array { return require 'tests/data/Calculation/MathTrig/SUM.php'; } + + /** + * @dataProvider providerSUMLiterals + * + * @param mixed $expectedResult + */ + public function testSUMLiterals($expectedResult, string $args): void + { + $sheet = $this->sheet; + $sheet->getCell('B1')->setValue("=SUM($args)"); + $result = $sheet->getCell('B1')->getCalculatedValue(); + self::assertEqualsWithDelta($expectedResult, $result, 1E-12); + } + + public function providerSUMLiterals(): array + { + return require 'tests/data/Calculation/MathTrig/SUMLITERALS.php'; + } } diff --git a/tests/PhpSpreadsheetTests/Writer/Csv/CsvExcelCompatibilityTest.php b/tests/PhpSpreadsheetTests/Writer/Csv/CsvExcelCompatibilityTest.php new file mode 100644 index 0000000000..9b7d16aae2 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Csv/CsvExcelCompatibilityTest.php @@ -0,0 +1,49 @@ +getActiveSheet(); + $sheet->setCellValue('A1', '1'); + $sheet->setCellValue('B1', '2'); + $sheet->setCellValue('C1', '3'); + $sheet->setCellValue('A2', '4'); + $sheet->setCellValue('B2', '5'); + $sheet->setCellValue('C2', '6'); + $writer = new CsvWriter($spreadsheet); + $writer->setExcelCompatibility(true); + self::assertSame('', $writer->getOutputEncoding()); + $filename = File::temporaryFilename(); + $writer->save($filename); + $reader = new CsvReader(); + $spreadsheet2 = $reader->load($filename); + $contents = file_get_contents($filename); + unlink($filename); + self::assertEquals(1, $spreadsheet2->getActiveSheet()->getCell('A1')->getValue()); + self::assertEquals(6, $spreadsheet2->getActiveSheet()->getCell('C2')->getValue()); + self::assertStringContainsString(CsvReader::UTF8_BOM, $contents); + self::assertStringContainsString("\r\n", $contents); + self::assertStringContainsString('sep=;', $contents); + self::assertStringContainsString('"1";"2";"3"', $contents); + self::assertStringContainsString('"4";"5";"6"', $contents); + } +} diff --git a/tests/data/Calculation/MathTrig/SUM.php b/tests/data/Calculation/MathTrig/SUM.php index a821907681..0c54613e2a 100644 --- a/tests/data/Calculation/MathTrig/SUM.php +++ b/tests/data/Calculation/MathTrig/SUM.php @@ -4,5 +4,9 @@ [50, 5, 15, 30], [52, 5, 15, 30, 2], [53.1, 5.7, 15, 30, 2.4], - ['#VALUE!', 5.7, 'X', 30, 2.4], // error here conflicts with SUMIF + [52.1, 5.7, '14', 30, 2.4], + [38.1, 5.7, 'X', 30, 2.4], // error if entered in formula, but not in cell + [38.1, 5.7, null, 30, 2.4], + [38.1, 5.7, false, 30, 2.4], + [39.1, 5.7, true, 30, 2.4], ]; diff --git a/tests/data/Calculation/MathTrig/SUMLITERALS.php b/tests/data/Calculation/MathTrig/SUMLITERALS.php new file mode 100644 index 0000000000..fd184ebd50 --- /dev/null +++ b/tests/data/Calculation/MathTrig/SUMLITERALS.php @@ -0,0 +1,12 @@ +