Skip to content

Commit

Permalink
Breaking Change - 3 Defaults
Browse files Browse the repository at this point in the history
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)`.
  • Loading branch information
oleibman committed Feb 4, 2025
1 parent eafbed6 commit 95213bf
Show file tree
Hide file tree
Showing 12 changed files with 41 additions and 34 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Reader/Csv.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)?
Expand Down
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Writer/Html.php
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class Html extends BaseWriter
/** @var Chart[] */
private $sheetCharts;

private bool $betterBoolean = false;
private bool $betterBoolean = true;

private string $getTrue = 'TRUE';

Expand Down Expand Up @@ -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 . '"';
Expand Down
6 changes: 5 additions & 1 deletion src/PhpSpreadsheet/Writer/Xlsx.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
{
Expand Down
7 changes: 4 additions & 3 deletions tests/PhpSpreadsheetTests/Reader/Csv/CsvLineEndingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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") {
Expand Down
2 changes: 1 addition & 1 deletion tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4248Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public function testHtml(): void
. ' <td class="column0 style0">&nbsp;</td>'
. ' <td class="column1 style28 null"></td>'
. ' <td class="column2 style35 s">Eligible </td>'
. ' <td class="column3 style70 f">Non</td>';
. ' <td class="column3 style70 s">Non</td>';
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');
Expand Down
4 changes: 2 additions & 2 deletions tests/PhpSpreadsheetTests/Writer/Html/BadHyperlinkTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public function testBadHyperlink(): void
$spreadsheet = $reader->load($infile);
$writer = new HtmlWriter($spreadsheet);
$html = $writer->generateHtmlAll();
self::assertStringContainsString('<td class="column0 style1 f">jav&#9;ascript:alert()</td>', $html);
self::assertStringContainsString('<td class="column0 style1 s">jav&#9;ascript:alert()</td>', $html);
$spreadsheet->disconnectWorksheets();
}

Expand All @@ -29,7 +29,7 @@ public function testControlCharacter(): void
$spreadsheet = $reader->load($infile);
$writer = new HtmlWriter($spreadsheet);
$html = $writer->generateHtmlAll();
self::assertStringContainsString('<td class="column0 style0 f">&#20;j&#13;avascript:alert(1)</td>', $html);
self::assertStringContainsString('<td class="column0 style0 s">&#20;j&#13;avascript:alert(1)</td>', $html);
$spreadsheet->disconnectWorksheets();
}
}
20 changes: 6 additions & 14 deletions tests/PhpSpreadsheetTests/Writer/Html/BetterBooleanTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -125,9 +121,7 @@ public function testLocale(): void
self::assertStringContainsString('<td class="column5 style0 s">AB</td>', $html);
self::assertStringContainsString('<td class="column6 style0 n">3</td>', $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();
Expand Down Expand Up @@ -169,9 +163,7 @@ public function testInline(): void
self::assertStringContainsString('<td style="text-align:left;">AB</td>', $html);
self::assertStringContainsString('<td style="text-align:right;">3</td>', $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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public function testNoJavascriptLinks(): void
$html = $writer->generateHTMLAll();
self::assertStringContainsString('<td class="column0 style0 s"><a href="http://www.example.com">Click me</a></td>', $html, 'http hyperlink retained');
self::assertStringContainsString('<td class="column0 style0 s">javascript:alert(\'hello1\')</td>', $html, 'javascript hyperlink dropped');
self::assertStringContainsString('<td class="column0 style0 f">javascript:alert(\'hello2\')</td>', $html, 'javascript hyperlink function dropped');
self::assertStringContainsString('<td class="column0 style0 s">javascript:alert(\'hello2\')</td>', $html, 'javascript hyperlink function dropped');
$spreadsheet->disconnectWorksheets();
}
}
6 changes: 4 additions & 2 deletions tests/PhpSpreadsheetTests/Writer/PreCalcTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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('<calcPr calcId="999999" calcMode="auto" calcCompleted="0" fullCalcOnLoad="1" forceFullCalc="1"/>', $data);
self::assertStringContainsString('<calcPr calcId="999999" calcMode="auto" calcCompleted="0" fullCalcOnLoad="1" forceFullCalc="0"/>', $data);
} else {
self::assertStringContainsString('<calcPr calcId="999999" calcMode="auto" calcCompleted="1" fullCalcOnLoad="0" forceFullCalc="0"/>', $data);
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}
}
11 changes: 8 additions & 3 deletions tests/PhpSpreadsheetTests/Writer/Xlsx/Issue4269Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ class Issue4269Test extends TestCase
{
private string $outputFile = '';

private static bool $alwaysFalse = false;

protected function tearDown(): void
{
if ($this->outputFile !== '') {
Expand All @@ -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();
Expand All @@ -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);
}
}

0 comments on commit 95213bf

Please sign in to comment.