Fix for Issue 2042 (SUM Partially Broken) (#2045)

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

@@ -38,7 +38,8 @@ $helper->write($spreadsheetFromCSV, __FILE__, ['Xlsx']);
 $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);

src/PhpSpreadsheet/Calculation/MathTrig/Sum.php

@@ -51,16 +51,20 @@ class Sum
     {
         $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) || empty($arg)) {
                 if (is_string($arg)) {
                     $arg = (int) $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();
             }
         }

tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/SumTest.php

@@ -26,4 +26,22 @@ class SumTest extends AllSetupTeardown
     {
         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';
+    }
 }

tests/PhpSpreadsheetTests/Writer/Csv/CsvExcelCompatibilityTest.php

@@ -0,0 +1,49 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Writer\Csv;
+
+use PhpOffice\PhpSpreadsheet\Reader\Csv as CsvReader;
+use PhpOffice\PhpSpreadsheet\Shared\File;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Writer\Csv as CsvWriter;
+use PhpOffice\PhpSpreadsheetTests\Functional;
+
+class CsvExcelCompatibilityTest extends Functional\AbstractFunctional
+{
+    // Excel seems to have changed with how they handle this.
+    // In particular, it does not recognize UTF-8 non-ASCII characters
+    //    if a file is written with ExcelCompatibility on.
+    //    The initial 'sep=;' line seems to confuse it, even though
+    //    it has a BOM. The Unix "file" command also indicates a difference
+    //    when the sep line is or is not included:
+    //        UTF-8 Unicode (with BOM) text, with CRLF line terminators
+    //     vs CSV text (without sep line, with or without BOM)
+    // So, this test has no UTF-8 yet while more research is conducted.
+    public function testExcelCompatibility(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->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);
+    }
+}

tests/data/Calculation/MathTrig/SUM.php

@@ -4,5 +4,9 @@ return [
     [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],
 ];

tests/data/Calculation/MathTrig/SUMLITERALS.php

@@ -0,0 +1,12 @@
+<?php
+
+return [
+    [50, '5, 15, 30'],
+    [52, '5, 15, 30, 2'],
+    [53.1, '5.7, 15, 30, 2.4'],
+    [52.1, '5.7, "14", 30, 2.4'],
+    ['#VALUE!', '5.7, "X", 30, 2.4'], // error if entered in formula, but not in cell
+    [38.1, '5.7, , 30, 2.4'],
+    [38.1, '5.7, false, 30, 2.4'],
+    [39.1, '5.7, true, 30, 2.4'],
+];