From 95213bf50a855af8df68fe4d40ebd8717d43685f Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Tue, 4 Feb 2025 06:58:31 -0800 Subject: [PATCH] Breaking Change - 3 Defaults Fix #4092. Change default value for Csv Reader autodetect line endings. Prior behavior can be enabled via `setTestAutodetect(true)`. Change default value for Html Writer "better boolean" logic. Prior behavior can be explicitly enabled via `setBetterBoolean(false)`. Change default for Xlsx Writer forceFullCalc option. Prior behavior can be explicitly enabled via `setForceFullCalc(null)`. --- CHANGELOG.md | 5 ++++- CONTRIBUTING.md | 4 ++-- src/PhpSpreadsheet/Reader/Csv.php | 4 ++-- src/PhpSpreadsheet/Writer/Html.php | 4 ++-- src/PhpSpreadsheet/Writer/Xlsx.php | 6 +++++- .../Reader/Csv/CsvLineEndingTest.php | 7 ++++--- .../Reader/Xlsx/Issue4248Test.php | 2 +- .../Writer/Html/BadHyperlinkTest.php | 4 ++-- .../Writer/Html/BetterBooleanTest.php | 20 ++++++------------- .../Writer/Html/NoJavascriptLinksTest.php | 2 +- .../Writer/PreCalcTest.php | 6 ++++-- .../Writer/Xlsx/Issue4269Test.php | 11 +++++++--- 12 files changed, 41 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 959648a020..d1dfa92699 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Data Validations will be stored by worksheet, not cell. Index can be one or more cells or cell ranges. [Issue #797](https://github.com/PHPOffice/PhpSpreadsheet/issues/797) [Issue #4091](https://github.com/PHPOffice/PhpSpreadsheet/issues/4091) [Issue #4206](https://github.com/PHPOffice/PhpSpreadsheet/issues/4206) [PR #4240](https://github.com/PHPOffice/PhpSpreadsheet/pull/4240) - Conditional Formatting adds Priority property and handles overlapping ranges better. [Issue #4312](https://github.com/PHPOffice/PhpSpreadsheet/issues/4312) [Issue #4318](https://github.com/PHPOffice/PhpSpreadsheet/issues/4318) [PR #4314](https://github.com/PHPOffice/PhpSpreadsheet/pull/4314) +- Csv Reader will no longer auto-detect Mac line endings by default. Prior behavior can be explicitly enabled via `setTestAutoDetect(true)`, and it will not be possible at all with Php9+. [Issue #4092](https://github.com/PHPOffice/PhpSpreadsheet/issues/4092) No PR yet. +- Html Writer will now use "better boolean" logic. Booleans will now be output by default as TRUE/FALSE rather than 1/null-string. Prior behavior can be explicitly enabled via `setBetterBoolean(false)`. No PR yet. +- Xlsx Writer will now use false as the default for `forceFullCalc`. This affects writes with `preCalculateFormulas` set to false. Prior behavior can be explicitly enabled via `setForceFullCalc(null)`. - Deletion of items deprecated in Release 3. See "removed" below. ### Added @@ -35,7 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed -- Xls writer Parser Mishandling True/False Argument. [Issue #4331](https://github.com/PHPOffice/PhpSpreadsheet/issues/4331) [PR #4333](https://github.com/PHPOffice/PhpSpreadsheet/pull/4333) +- Xls Writer Parser Mishandling True/False Argument. [Issue #4331](https://github.com/PHPOffice/PhpSpreadsheet/issues/4331) [PR #4333](https://github.com/PHPOffice/PhpSpreadsheet/pull/4333) ## 2025-01-26 - 3.9.0 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 709792031f..88a03398f2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -42,8 +42,8 @@ This makes it easier to see exactly what is being tested when reviewing the PR. 3. Push the tag with `git push --tags`, GitHub Actions will create a GitHub release automatically, and the release details will automatically be sent to packagist. 4. By default, Github removes markdown headings in the Release Notes. You can either edit to restore these, or, probably preferably, change the default comment character on your system - `git config core.commentChar ";"`. -> **Note:** Tagged releases are made from the `master` branch. Only in an emergency should a tagged release be made from the `release` branch. (i.e. cherry-picked hot-fixes.) However, there are 3 branches which have been updated to apply security patches, and those may be tagged if future security updates are needed. +> **Note:** Tagged releases are made from the `master` branch. Only in an emergency should a tagged release be made from the `release` branch. (i.e. cherry-picked hot-fixes.) However, there are 4 branches which have been updated to apply security patches, and those may be tagged if future security updates are needed. - release1291 - release210 - release222 - +- release390 diff --git a/src/PhpSpreadsheet/Reader/Csv.php b/src/PhpSpreadsheet/Reader/Csv.php index 89cbfd5ad1..9455993e8b 100644 --- a/src/PhpSpreadsheet/Reader/Csv.php +++ b/src/PhpSpreadsheet/Reader/Csv.php @@ -84,8 +84,8 @@ class Csv extends BaseReader */ private static $constructorCallback; - /** Will be changed to false in next major release */ - public const DEFAULT_TEST_AUTODETECT = true; + /** Changed from true to false in release 4.0.0 */ + public const DEFAULT_TEST_AUTODETECT = false; /** * Attempt autodetect line endings (deprecated after PHP8.1)? diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php index 2b1d0a474e..83c99983ec 100644 --- a/src/PhpSpreadsheet/Writer/Html.php +++ b/src/PhpSpreadsheet/Writer/Html.php @@ -143,7 +143,7 @@ class Html extends BaseWriter /** @var Chart[] */ private $sheetCharts; - private bool $betterBoolean = false; + private bool $betterBoolean = true; private string $getTrue = 'TRUE'; @@ -1494,7 +1494,7 @@ private function generateRowWriteCell( $dataType = $worksheet->getCell($coordinate)->getDataType(); if ($dataType === DataType::TYPE_BOOL) { $html .= ' data-type="' . DataType::TYPE_BOOL . '"'; - } elseif ($dataType === DataType::TYPE_FORMULA && is_bool($worksheet->getCell($coordinate)->getCalculatedValue())) { + } elseif ($dataType === DataType::TYPE_FORMULA && $this->preCalculateFormulas && is_bool($worksheet->getCell($coordinate)->getCalculatedValue())) { $html .= ' data-type="' . DataType::TYPE_BOOL . '"'; } elseif (is_numeric($cellData) && $worksheet->getCell($coordinate)->getDataType() === DataType::TYPE_STRING) { $html .= ' data-type="' . DataType::TYPE_STRING . '"'; diff --git a/src/PhpSpreadsheet/Writer/Xlsx.php b/src/PhpSpreadsheet/Writer/Xlsx.php index b74d810502..c2fbd0cbc4 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx.php +++ b/src/PhpSpreadsheet/Writer/Xlsx.php @@ -140,7 +140,10 @@ class Xlsx extends BaseWriter private bool $useDynamicArray = false; - private ?bool $forceFullCalc = null; + public const DEFAULT_FORCE_FULL_CALC = false; + + // Default changed from null in PhpSpreadsheet 4.0.0. + private ?bool $forceFullCalc = self::DEFAULT_FORCE_FULL_CALC; /** * Create a new Xlsx Writer. @@ -758,6 +761,7 @@ private function determineUseDynamicArrays(): void * If null, this will be set to the opposite of $preCalculateFormulas. * It is likely that false is the desired setting, although * cases have been reported where true is required (issue #456). + * Nevertheless, default is set to false in PhpSpreadsheet 4.0.0. */ public function setForceFullCalc(?bool $forceFullCalc): self { diff --git a/tests/PhpSpreadsheetTests/Reader/Csv/CsvLineEndingTest.php b/tests/PhpSpreadsheetTests/Reader/Csv/CsvLineEndingTest.php index 3fc1d3031c..51ead8bdbb 100644 --- a/tests/PhpSpreadsheetTests/Reader/Csv/CsvLineEndingTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Csv/CsvLineEndingTest.php @@ -6,6 +6,7 @@ use PhpOffice\PhpSpreadsheet\Reader\Csv; use PhpOffice\PhpSpreadsheet\Shared\File; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; class CsvLineEndingTest extends TestCase @@ -22,7 +23,7 @@ protected function tearDown(): void } } - #[\PHPUnit\Framework\Attributes\DataProvider('providerEndings')] + #[DataProvider('providerEndings')] public function testEndings(string $ending): void { if ($ending === "\r" && PHP_VERSION_ID >= 90000) { @@ -43,14 +44,14 @@ public function testEndings(string $ending): void $spreadsheet->disconnectWorksheets(); } - #[\PHPUnit\Framework\Attributes\DataProvider('providerEndings')] + #[DataProvider('providerEndings')] public function testEndingsNoDetect(string $ending): void { $this->tempFile = $filename = File::temporaryFilename(); $data = ['123', '456', '789']; file_put_contents($filename, implode($ending, $data)); $reader = new Csv(); - $reader->setTestAutoDetect(false); + self::assertSame(self::$alwaysFalse, Csv::DEFAULT_TEST_AUTODETECT); $spreadsheet = $reader->load($filename); $sheet = $spreadsheet->getActiveSheet(); if ($ending === "\r") { diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4248Test.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4248Test.php index de5ec8f029..4fd45c395d 100644 --- a/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4248Test.php +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4248Test.php @@ -94,7 +94,7 @@ public function testHtml(): void . '  ' . ' ' . ' Eligible ' - . ' Non'; + . ' Non'; self::assertStringContainsString($expected, $data, 'Cell D18 style'); $expected = ' td.style70, th.style70 { vertical-align:middle; text-align:center; border-bottom:1px solid #000000 !important; border-top:2px solid #000000 !important; border-left:2px solid #000000 !important; border-right:1px solid #000000 !important; font-weight:bold; color:#000000; font-family:\'Calibri\'; font-size:16pt; background-color:#BDD7EE }'; self::assertStringContainsString($expected, $data, 'background color'); diff --git a/tests/PhpSpreadsheetTests/Writer/Html/BadHyperlinkTest.php b/tests/PhpSpreadsheetTests/Writer/Html/BadHyperlinkTest.php index 6e2634c6cf..63cc4d89c2 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/BadHyperlinkTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/BadHyperlinkTest.php @@ -18,7 +18,7 @@ public function testBadHyperlink(): void $spreadsheet = $reader->load($infile); $writer = new HtmlWriter($spreadsheet); $html = $writer->generateHtmlAll(); - self::assertStringContainsString('jav ascript:alert()', $html); + self::assertStringContainsString('jav ascript:alert()', $html); $spreadsheet->disconnectWorksheets(); } @@ -29,7 +29,7 @@ public function testControlCharacter(): void $spreadsheet = $reader->load($infile); $writer = new HtmlWriter($spreadsheet); $html = $writer->generateHtmlAll(); - self::assertStringContainsString('j avascript:alert(1)', $html); + self::assertStringContainsString('j avascript:alert(1)', $html); $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Writer/Html/BetterBooleanTest.php b/tests/PhpSpreadsheetTests/Writer/Html/BetterBooleanTest.php index 0eac757d4d..71d81122a9 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/BetterBooleanTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/BetterBooleanTest.php @@ -31,8 +31,8 @@ public function testDefault(): void { $spreadsheet = new Spreadsheet(); $writer = new HtmlWriter($spreadsheet); - // Default will change with next PhpSpreadsheet release - self::assertFalse($writer->getBetterBoolean()); + // Default change with release 4.0.0 + self::assertTrue($writer->getBetterBoolean()); $spreadsheet->disconnectWorksheets(); } @@ -58,9 +58,7 @@ public function testBetterBoolean(): void $sheet->getCell('F1')->setValue('="A"&"B"'); $sheet->getCell('G1')->setValue('=1+2'); - /** @var callable */ - $callableWriter = [$this, 'setBetter']; - $reloaded = $this->writeAndReload($spreadsheet, 'Html', null, $callableWriter); + $reloaded = $this->writeAndReload($spreadsheet, 'Html'); $spreadsheet->disconnectWorksheets(); $rsheet = $reloaded->getActiveSheet(); @@ -86,9 +84,7 @@ public function testNotBetterBoolean(): void $sheet->getCell('F1')->setValue('="A"&"B"'); $sheet->getCell('G1')->setValue('=1+2'); - /** @var callable */ - $callableWriter = [$this, 'setNotBetter']; - $reloaded = $this->writeAndReload($spreadsheet, 'Html', null, $callableWriter); + $reloaded = $this->writeAndReload($spreadsheet, 'Html', null, $this->setNotBetter(...)); $spreadsheet->disconnectWorksheets(); $rsheet = $reloaded->getActiveSheet(); @@ -125,9 +121,7 @@ public function testLocale(): void self::assertStringContainsString('AB', $html); self::assertStringContainsString('3', $html); - /** @var callable */ - $callableWriter = [$this, 'setBetter']; - $reloaded = $this->writeAndReload($spreadsheet, 'Html', null, $callableWriter); + $reloaded = $this->writeAndReload($spreadsheet, 'Html', null, $this->setBetter(...)); $spreadsheet->disconnectWorksheets(); $rsheet = $reloaded->getActiveSheet(); @@ -169,9 +163,7 @@ public function testInline(): void self::assertStringContainsString('AB', $html); self::assertStringContainsString('3', $html); - /** @var callable */ - $callableWriter = [$this, 'setBetter']; - $reloaded = $this->writeAndReload($spreadsheet, 'Html', null, $callableWriter); + $reloaded = $this->writeAndReload($spreadsheet, 'Html', null, $this->setBetter(...)); $spreadsheet->disconnectWorksheets(); $rsheet = $reloaded->getActiveSheet(); diff --git a/tests/PhpSpreadsheetTests/Writer/Html/NoJavascriptLinksTest.php b/tests/PhpSpreadsheetTests/Writer/Html/NoJavascriptLinksTest.php index cc0511d4fe..156f9079d4 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/NoJavascriptLinksTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/NoJavascriptLinksTest.php @@ -27,7 +27,7 @@ public function testNoJavascriptLinks(): void $html = $writer->generateHTMLAll(); self::assertStringContainsString('Click me', $html, 'http hyperlink retained'); self::assertStringContainsString('javascript:alert(\'hello1\')', $html, 'javascript hyperlink dropped'); - self::assertStringContainsString('javascript:alert(\'hello2\')', $html, 'javascript hyperlink function dropped'); + self::assertStringContainsString('javascript:alert(\'hello2\')', $html, 'javascript hyperlink function dropped'); $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Writer/PreCalcTest.php b/tests/PhpSpreadsheetTests/Writer/PreCalcTest.php index eb9530bde8..8180701712 100644 --- a/tests/PhpSpreadsheetTests/Writer/PreCalcTest.php +++ b/tests/PhpSpreadsheetTests/Writer/PreCalcTest.php @@ -10,6 +10,7 @@ use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Worksheet\ColumnDimension; use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional; +use PHPUnit\Framework\Attributes\DataProvider; class PreCalcTest extends AbstractFunctional { @@ -117,7 +118,7 @@ private function verifyXlsx(?bool $preCalc, string $type): void $data = self::readFile($file); // confirm whether workbook is set to recalculate if ($preCalc === false) { - self::assertStringContainsString('', $data); + self::assertStringContainsString('', $data); } else { self::assertStringContainsString('', $data); } @@ -174,7 +175,7 @@ private function verifyCsv(?bool $preCalc, string $type): void } } - #[\PHPUnit\Framework\Attributes\DataProvider('providerPreCalc')] + #[DataProvider('providerPreCalc')] public function testPreCalc(?bool $preCalc, string $type): void { $spreadsheet = new Spreadsheet(); @@ -203,5 +204,6 @@ public function testPreCalc(?bool $preCalc, string $type): void $this->verifyOds($preCalc, $type); $this->verifyHtml($preCalc, $type); $this->verifyCsv($preCalc, $type); + $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue4269Test.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue4269Test.php index ffe9e30ae0..b78a9394b1 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue4269Test.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue4269Test.php @@ -14,6 +14,8 @@ class Issue4269Test extends TestCase { private string $outputFile = ''; + private static bool $alwaysFalse = false; + protected function tearDown(): void { if ($this->outputFile !== '') { @@ -35,9 +37,7 @@ public function testWriteArrayFormulaTextJoin( $writer = new XlsxWriter($spreadsheet); $writer->setPreCalculateFormulas($preCalculateFormulas); - if ($forceFullCalc !== null) { - $writer->setForceFullCalc($forceFullCalc); - } + $writer->setForceFullCalc($forceFullCalc); $this->outputFile = File::temporaryFilename(); $writer->save($this->outputFile); $spreadsheet->disconnectWorksheets(); @@ -62,4 +62,9 @@ public static function validationProvider(): array 'unlikely use case' => [true, true, 'calcMode="auto" calcCompleted="1" fullCalcOnLoad="0" forceFullCalc="1"'], ]; } + + public function testDefault(): void + { + self::assertSame(self::$alwaysFalse, XlsxWriter::DEFAULT_FORCE_FULL_CALC); + } }