Csv, Boolean, and StringValueBinder (#2374)

See the discussion in PR #2232 which came about 3 months after it was merged. It caused a problem in an unusual situation which did not come to light until the change was part of the new release version. The original PR changed PhpSpreadsheet's behavior to match Excel's for (not case sensitive) strings `TRUE` and `FALSE`. Excel treats the values as boolean, and now so does PhpSpreadsheet.

When StringValueBinder is used, this becomes a tricky situation. The user wants the original strings preserved, including the case of all the letters. This PR changes the behavior of CSV reader as follows:
- If StringValueBinder is not in effect, convert to boolean.
- If StringValueBinder (actually any binder with method getBooleanConversion) is in effect, and the result of getBooleanConversion is true (which is the default in StringValueBinder), leave the value coming out of Csv Reader as the unchanged string.
- Otherwise, convert to boolean.

This should mean that there are no regression problems with StringValueBinder, while allowing PhpSpreadsheet to continue to match Excel in the default situation. No new settings are required.

src/PhpSpreadsheet/Cell/StringValueBinder.php

@@ -42,6 +42,11 @@ class StringValueBinder implements IValueBinder
         return $this;
     }
 
+    public function getBooleanConversion(): bool
+    {
+        return $this->convertBoolean;
+    }
+
     public function setNumericConversion(bool $suppressConversion = false): self
     {
         $this->convertNumeric = $suppressConversion;

src/PhpSpreadsheet/Reader/Csv.php

@@ -2,6 +2,7 @@
 
 namespace PhpOffice\PhpSpreadsheet\Reader;
 
+use PhpOffice\PhpSpreadsheet\Cell\Cell;
 use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
 use PhpOffice\PhpSpreadsheet\Reader\Csv\Delimiter;
 use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException;
@@ -314,11 +315,13 @@ class Csv extends BaseReader
 
         // Loop through each line of the file in turn
         $rowData = fgetcsv($fileHandle, 0, $this->delimiter ?? '', $this->enclosure, $this->escapeCharacter);
+        $valueBinder = Cell::getValueBinder();
+        $preserveBooleanString = method_exists($valueBinder, 'getBooleanConversion') && $valueBinder->getBooleanConversion();
         while (is_array($rowData)) {
             $noOutputYet = true;
             $columnLetter = 'A';
             foreach ($rowData as $rowDatum) {
-                self::convertBoolean($rowDatum);
+                $this->convertBoolean($rowDatum, $preserveBooleanString);
                 if ($rowDatum !== '' && $this->readFilter->readCell($columnLetter, $currentRow)) {
                     if ($this->contiguous) {
                         if ($noOutputYet) {
@@ -351,9 +354,9 @@ class Csv extends BaseReader
      *
      * @param mixed $rowDatum
      */
-    private static function convertBoolean(&$rowDatum): void
+    private function convertBoolean(&$rowDatum, bool $preserveBooleanString): void
     {
-        if (is_string($rowDatum)) {
+        if (is_string($rowDatum) && !$preserveBooleanString) {
             if (strcasecmp('true', $rowDatum) === 0) {
                 $rowDatum = true;
             } elseif (strcasecmp('false', $rowDatum) === 0) {
@@ -405,7 +408,7 @@ class Csv extends BaseReader
 
     public function setContiguous(bool $contiguous): self
     {
-        $this->contiguous = (bool) $contiguous;
+        $this->contiguous = $contiguous;
 
         return $this;
     }

tests/PhpSpreadsheetTests/Reader/Csv/CsvIssue2232Test.php

@@ -0,0 +1,63 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Csv;
+
+use PhpOffice\PhpSpreadsheet\Cell\Cell;
+use PhpOffice\PhpSpreadsheet\Cell\IValueBinder;
+use PhpOffice\PhpSpreadsheet\Cell\StringValueBinder;
+use PhpOffice\PhpSpreadsheet\Reader\Csv;
+use PHPUnit\Framework\TestCase;
+
+class CsvIssue2232Test extends TestCase
+{
+    /**
+     * @var IValueBinder
+     */
+    private $valueBinder;
+
+    protected function setUp(): void
+    {
+        $this->valueBinder = Cell::getValueBinder();
+    }
+
+    protected function tearDown(): void
+    {
+        Cell::setValueBinder($this->valueBinder);
+    }
+
+    /**
+     * @dataProvider providerIssue2232
+     *
+     * @param mixed $b2Value
+     * @param mixed $b3Value
+     */
+    public function testEncodings(bool $useStringBinder, ?bool $preserveBoolString, $b2Value, $b3Value): void
+    {
+        if ($useStringBinder) {
+            $binder = new StringValueBinder();
+            if (is_bool($preserveBoolString)) {
+                $binder->setBooleanConversion($preserveBoolString);
+            }
+            Cell::setValueBinder($binder);
+        }
+        $reader = new Csv();
+        $filename = 'tests/data/Reader/CSV/issue.2232.csv';
+        $spreadsheet = $reader->load($filename);
+        $sheet = $spreadsheet->getActiveSheet();
+        self::assertSame($b2Value, $sheet->getCell('B2')->getValue());
+        self::assertSame($b3Value, $sheet->getCell('B3')->getValue());
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function providerIssue2232(): array
+    {
+        return [
+            [false, false, false, true],
+            [false, null, false, true],
+            [false, true, false, true],
+            [true, false, false, true],
+            [true, null, 'FaLSe', 'tRUE'],
+            [true, true, 'FaLSe', 'tRUE'],
+        ];
+    }
+}

tests/data/Reader/CSV/issue.2232.csv

@@ -0,0 +1,3 @@
+1,2,3
+a,FaLSe,b
+cc,tRUE,cc