Clientbase
/
PhpSpreadsheet
mirror of https://github.com/basemaster/PhpSpreadsheet.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Csv Handling of Booleans (and an 8.1 Deprecation) (#2232) 

* Csv Handling of Booleans (and an 8.1 Deprecation)

PhpSpreadsheet writes boolean values to a Csv as null-string/1, and treats input values of 'true' and 'false' as if they were strings. On the other hand, Excel writes boolean values to a Csv as TRUE/FALSE, and case-insensitively treats a matching string as boolean on read. This PR changes PhpSpreadsheet to match Excel.

A side-effect of this change is that it fixes behavior incorrectly reported as a bug in PR #2048. That issue was closed, correctly, as user error. The user had altered Csv Writer, including adding ```declare(strict_types=1);```; that declaration was the cause of the error. The "offending" statements, calls to strpbrk and str_replace, will now work correctly whether or not strict_types is in use.

And, just as I was getting ready to push this, the dailies for PHP 8.1 introduced a change deprecating auto_detect_line_endings. Csv Reader uses that setting; it allows it to process a Csv with Mac line endings, which happens to be something that Excel can do. As they say in https://wiki.php.net/rfc/deprecations_php_8_1, where the proposal passed without a single dissenting vote, "These newlines were used by “Classic” Mac OS, a system which has been discontinued in 2001, nearly two decades ago. Interoperability with such systems is no longer relevant." I tend to agree, but I don't know that we're ready to pull the plug yet. I don't see an easy way to emulate that functionality. For now, I have silenced the deprecation notices with at signs. I have also added a test case which will fail when support for that setting is pulled; this will give time to consider alternatives.

* Scrutinizer: Handling ini_set

This could be interesting. It doesn't like not handling an error condition for ini_set. Let's see if this satisfies it.
This commit is contained in:
oleibman 2021-08-04 07:00:17 -07:00 committed by GitHub
parent b9f6c70b86
commit 0cd20f3099
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 195 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
phpstan-baseline.neon
View File

@ -4985,11 +4985,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Worksheet/Worksheet.php
-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Writer\\\\Csv\\:\\:\\$enclosureRequired has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Writer/Csv.php
-
message: "#^Call to function array_key_exists\\(\\) with int and array\\('none' \\=\\> 'none', 'dashDot' \\=\\> '1px dashed', 'dashDotDot' \\=\\> '1px dotted', 'dashed' \\=\\> '1px dashed', 'dotted' \\=\\> '1px dotted', 'double' \\=\\> '3px double', 'hair' \\=\\> '1px solid', 'medium' \\=\\> '2px solid', \\.\\.\\.\\) will always evaluate to false\\.$#"
count: 1
@ -6535,16 +6530,6 @@ parameters:
count: 1
path: tests/PhpSpreadsheetTests/Worksheet/RowCellIterator2Test.php
-
message: "#^Property PhpOffice\\\\PhpSpreadsheetTests\\\\Writer\\\\Csv\\\\CsvEnclosureTest\\:\\:\\$cellValues has no typehint specified\\.$#"
count: 1
path: tests/PhpSpreadsheetTests/Writer/Csv/CsvEnclosureTest.php
-
message: "#^Parameter \\#3 \\$subject of function preg_replace expects array\\|string, string\\|false given\\.$#"
count: 4
path: tests/PhpSpreadsheetTests/Writer/Csv/CsvEnclosureTest.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheetTests\\\\Writer\\\\Html\\\\CallbackTest\\:\\:yellowBody\\(\\) should return string but returns string\\|null\\.$#"
count: 1

40
src/PhpSpreadsheet/Reader/Csv.php
View File

@ -272,13 +272,26 @@ class Csv extends BaseReader
}
}
private static function setAutoDetect(?string $value): ?string
{
$retVal = null;
if ($value !== null) {
$retVal2 = @ini_set('auto_detect_line_endings', $value);
if (is_string($retVal2)) {
$retVal = $retVal2;
}
}
return $retVal;
}
/**
* Loads PhpSpreadsheet from file into PhpSpreadsheet instance.
*/
public function loadIntoExisting(string $pFilename, Spreadsheet $spreadsheet): Spreadsheet
{
$lineEnding = ini_get('auto_detect_line_endings') ?: '0';
ini_set('auto_detect_line_endings', '1');
// Deprecated in Php8.1
$iniset = self::setAutoDetect('1');
// Open file
$this->openFileOrMemory($pFilename);
@ -305,7 +318,8 @@ class Csv extends BaseReader
$noOutputYet = true;
$columnLetter = 'A';
foreach ($rowData as $rowDatum) {
if ($rowDatum != '' && $this->readFilter->readCell($columnLetter, $currentRow)) {
self::convertBoolean($rowDatum);
if ($rowDatum !== '' && $this->readFilter->readCell($columnLetter, $currentRow)) {
if ($this->contiguous) {
if ($noOutputYet) {
$noOutputYet = false;
@ -326,12 +340,30 @@ class Csv extends BaseReader
// Close file
fclose($fileHandle);
ini_set('auto_detect_line_endings', $lineEnding);
self::setAutoDetect($iniset);
// Return
return $spreadsheet;
}
/**
* Convert string true/false to boolean, and null to null-string.
*
* @param mixed $rowDatum
*/
private static function convertBoolean(&$rowDatum): void
{
if (is_string($rowDatum)) {
if (strcasecmp('true', $rowDatum) === 0) {
$rowDatum = true;
} elseif (strcasecmp('false', $rowDatum) === 0) {
$rowDatum = false;
}
} elseif ($rowDatum === null) {
$rowDatum = '';
}
}
public function getDelimiter(): ?string
{
return $this->delimiter;

16
src/PhpSpreadsheet/Writer/Csv.php
View File

@ -329,6 +329,7 @@ class Csv extends BaseWriter
return $this;
}
/** @var bool */
private $enclosureRequired = true;
public function setEnclosureRequired(bool $value): self
@ -343,6 +344,20 @@ class Csv extends BaseWriter
return $this->enclosureRequired;
}
/**
* Convert boolean to TRUE/FALSE; otherwise return element cast to string.
*
* @param mixed $element
*/
private static function elementToString($element): string
{
if (is_bool($element)) {
return $element ? 'TRUE' : 'FALSE';
}
return (string) $element;
}
/**
* Write line to CSV file.
*
@ -358,6 +373,7 @@ class Csv extends BaseWriter
$line = '';
foreach ($pValues as $element) {
$element = self::elementToString($element);
// Add delimiter
$line .= $delimiter;
$delimiter = $this->delimiter;

47
tests/PhpSpreadsheetTests/Reader/Csv/CsvLineEndingTest.php Normal file
View File

@ -0,0 +1,47 @@
<?php
namespace PhpOffice\PhpSpreadsheetTests\Reader\Csv;
use PhpOffice\PhpSpreadsheet\Reader\Csv;
use PhpOffice\PhpSpreadsheet\Shared\File;
use PHPUnit\Framework\TestCase;
class CsvLineEndingTest extends TestCase
{
/** @var string */
private $tempFile = '';
protected function tearDown(): void
{
if ($this->tempFile !== '') {
unlink($this->tempFile);
$this->tempFile = '';
}
}
/**
* @dataProvider providerEndings
*/
public function testEndings(string $ending): void
{
$this->tempFile = $filename = File::temporaryFilename();
$data = ['123', '456', '789'];
file_put_contents($filename, implode($ending, $data));
$reader = new Csv();
$spreadsheet = $reader->load($filename);
$sheet = $spreadsheet->getActiveSheet();
self::assertEquals($data[0], $sheet->getCell('A1')->getValue());
self::assertEquals($data[1], $sheet->getCell('A2')->getValue());
self::assertEquals($data[2], $sheet->getCell('A3')->getValue());
$spreadsheet->disconnectWorksheets();
}
public function providerEndings(): array
{
return [
'Unix endings' => ["\n"],
'Mac endings' => ["\r"],
'Windows endings' => ["\r\n"],
];
}
}

111
tests/PhpSpreadsheetTests/Writer/Csv/CsvEnclosureTest.php
View File

@ -10,24 +10,29 @@ use PhpOffice\PhpSpreadsheetTests\Functional;
class CsvEnclosureTest extends Functional\AbstractFunctional
{
private static $cellValues = [
private const CELL_VALUES = [
'A1' => '2020-06-03',
'B1' => '000123',
'C1' => '06.53',
'D1' => '14.22',
'D1' => 14.22,
'A2' => '2020-06-04',
'B2' => '000234',
'C2' => '07.12',
'D2' => '15.44',
];
private static function getFileData(string $filename): string
{
return file_get_contents($filename) ?: '';
}
public function testNormalEnclosure(): void
{
$delimiter = ';';
$enclosure = '"';
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
foreach (self::$cellValues as $key => $value) {
foreach (self::CELL_VALUES as $key => $value) {
$sheet->setCellValue($key, $value);
}
$writer = new CsvWriter($spreadsheet);
@ -35,7 +40,7 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
$writer->setEnclosure($enclosure);
$filename = File::temporaryFilename();
$writer->save($filename);
$filedata = file_get_contents($filename);
$filedata = self::getFileData($filename);
$filedata = preg_replace('/\\r?\\n/', $delimiter, $filedata);
$reader = new CsvReader();
$reader->setDelimiter($delimiter);
@ -44,11 +49,13 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
unlink($filename);
$sheet = $newspreadsheet->getActiveSheet();
$expected = '';
foreach (self::$cellValues as $key => $value) {
foreach (self::CELL_VALUES as $key => $value) {
self::assertEquals($value, $sheet->getCell($key)->getValue());
$expected .= "$enclosure$value$enclosure$delimiter";
}
self::assertEquals($expected, $filedata);
$spreadsheet->disconnectWorksheets();
$newspreadsheet->disconnectWorksheets();
}
public function testNoEnclosure(): void
@ -57,7 +64,7 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
$enclosure = '';
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
foreach (self::$cellValues as $key => $value) {
foreach (self::CELL_VALUES as $key => $value) {
$sheet->setCellValue($key, $value);
}
$writer = new CsvWriter($spreadsheet);
@ -66,7 +73,7 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
self::assertEquals('', $writer->getEnclosure());
$filename = File::temporaryFilename();
$writer->save($filename);
$filedata = file_get_contents($filename);
$filedata = self::getFileData($filename);
$filedata = preg_replace('/\\r?\\n/', $delimiter, $filedata);
$reader = new CsvReader();
$reader->setDelimiter($delimiter);
@ -76,11 +83,13 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
unlink($filename);
$sheet = $newspreadsheet->getActiveSheet();
$expected = '';
foreach (self::$cellValues as $key => $value) {
foreach (self::CELL_VALUES as $key => $value) {
self::assertEquals($value, $sheet->getCell($key)->getValue());
$expected .= "$enclosure$value$enclosure$delimiter";
}
self::assertEquals($expected, $filedata);
$spreadsheet->disconnectWorksheets();
$newspreadsheet->disconnectWorksheets();
}
public function testNotRequiredEnclosure1(): void
@ -89,7 +98,7 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
$enclosure = '"';
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
foreach (self::$cellValues as $key => $value) {
foreach (self::CELL_VALUES as $key => $value) {
$sheet->setCellValue($key, $value);
}
$writer = new CsvWriter($spreadsheet);
@ -97,7 +106,7 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
$writer->setEnclosureRequired(false)->setDelimiter($delimiter)->setEnclosure($enclosure);
$filename = File::temporaryFilename();
$writer->save($filename);
$filedata = file_get_contents($filename);
$filedata = self::getFileData($filename);
$filedata = preg_replace('/\\r?\\n/', $delimiter, $filedata);
$reader = new CsvReader();
$reader->setDelimiter($delimiter);
@ -106,11 +115,13 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
unlink($filename);
$sheet = $newspreadsheet->getActiveSheet();
$expected = '';
foreach (self::$cellValues as $key => $value) {
foreach (self::CELL_VALUES as $key => $value) {
self::assertEquals($value, $sheet->getCell($key)->getValue());
$expected .= "$value$delimiter";
}
self::assertEquals($expected, $filedata);
$spreadsheet->disconnectWorksheets();
$newspreadsheet->disconnectWorksheets();
}
public function testNotRequiredEnclosure2(): void
@ -123,7 +134,7 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
'A2' => 'has space',
'B2' => "has\nnewline",
'C2' => '',
'D2' => '15.44',
'D2' => 15.44,
'A3' => ' leadingspace',
'B3' => 'trailingspace ',
'C3' => '=D2*2',
@ -132,13 +143,18 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
'B4' => 'unused',
'C4' => 'unused',
'D4' => 'unused',
'A5' => false,
'B5' => true,
'C5' => null,
'D5' => 0,
];
$calcc3 = '30.88';
$expected1 = '2020-06-03,"has,separator",has;non-separator,"has""enclosure"';
$expected2 = 'has space,"has' . "\n" . 'newline",,15.44';
$expected3 = ' leadingspace,trailingspace ,' . $calcc3 . ',",leadingcomma"';
$expected4 = '"trailingquote""",unused,unused,unused';
$expectedfile = "$expected1\n$expected2\n$expected3\n$expected4\n";
$expected5 = 'FALSE,TRUE,,0';
$expectedfile = "$expected1\n$expected2\n$expected3\n$expected4\n$expected5\n";
$delimiter = ',';
$enclosure = '"';
$spreadsheet = new Spreadsheet();
@ -151,7 +167,7 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
$writer->setEnclosureRequired(false)->setDelimiter($delimiter)->setEnclosure($enclosure);
$filename = File::temporaryFilename();
$writer->save($filename);
$filedata = file_get_contents($filename);
$filedata = self::getFileData($filename);
$filedata = preg_replace('/\\r/', '', $filedata);
$reader = new CsvReader();
$reader->setDelimiter($delimiter);
@ -160,9 +176,70 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
unlink($filename);
$sheet = $newspreadsheet->getActiveSheet();
foreach ($cellValues2 as $key => $value) {
self::assertEquals(($key === 'C3') ? $calcc3 : $value, $sheet->getCell($key)->getValue());
self::assertEquals(($key === 'C3') ? $calcc3 : $value, $sheet->getCell($key)->getValue(), "Failure for cell $key");
}
self::assertEquals($expectedfile, $filedata);
$spreadsheet->disconnectWorksheets();
$newspreadsheet->disconnectWorksheets();
}
public function testRequiredEnclosure2(): void
{
$cellValues2 = [
'A1' => '2020-06-03',
'B1' => 'has,separator',
'C1' => 'has;non-separator',
'D1' => 'has"enclosure',
'A2' => 'has space',
'B2' => "has\nnewline",
'C2' => '',
'D2' => 15.44,
'A3' => ' leadingspace',
'B3' => 'trailingspace ',
'C3' => '=D2*2',
'D3' => ',leadingcomma',
'A4' => 'trailingquote"',
'B4' => 'unused',
'C4' => 'unused',
'D4' => 'unused',
'A5' => false,
'B5' => true,
'C5' => null,
'D5' => 0,
];
$calcc3 = '30.88';
$expected1 = '"2020-06-03","has,separator","has;non-separator","has""enclosure"';
$expected2 = '"has space","has' . "\n" . 'newline","","15.44"';
$expected3 = '" leadingspace","trailingspace ","' . $calcc3 . '",",leadingcomma"';
$expected4 = '"trailingquote""","unused","unused","unused"';
$expected5 = '"FALSE","TRUE","","0"';
$expectedfile = "$expected1\n$expected2\n$expected3\n$expected4\n$expected5\n";
$delimiter = ',';
$enclosure = '"';
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
foreach ($cellValues2 as $key => $value) {
$sheet->setCellValue($key, $value);
}
$writer = new CsvWriter($spreadsheet);
self::assertTrue($writer->getEnclosureRequired());
$writer->setEnclosureRequired(true)->setDelimiter($delimiter)->setEnclosure($enclosure);
$filename = File::temporaryFilename();
$writer->save($filename);
$filedata = self::getFileData($filename);
$filedata = preg_replace('/\\r/', '', $filedata);
$reader = new CsvReader();
$reader->setDelimiter($delimiter);
$reader->setEnclosure($enclosure);
$newspreadsheet = $reader->load($filename);
unlink($filename);
$sheet = $newspreadsheet->getActiveSheet();
foreach ($cellValues2 as $key => $value) {
self::assertEquals(($key === 'C3') ? $calcc3 : $value, $sheet->getCell($key)->getValue(), "Failure for cell $key");
}
self::assertEquals($expectedfile, $filedata);
$spreadsheet->disconnectWorksheets();
$newspreadsheet->disconnectWorksheets();
}
public function testGoodReread(): void
@ -187,6 +264,8 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
self::assertEquals('1', $sheet->getCell('A1')->getValue());
self::assertEquals('2,3', $sheet->getCell('B1')->getValue());
self::assertEquals('4', $sheet->getCell('C1')->getValue());
$spreadsheet->disconnectWorksheets();
$newspreadsheet->disconnectWorksheets();
}
public function testBadReread(): void
@ -212,5 +291,7 @@ class CsvEnclosureTest extends Functional\AbstractFunctional
self::assertEquals('2', $sheet->getCell('B1')->getValue());
self::assertEquals('3', $sheet->getCell('C1')->getValue());
self::assertEquals('4', $sheet->getCell('D1')->getValue());
$spreadsheet->disconnectWorksheets();
$newspreadsheet->disconnectWorksheets();
}
}