From c2428f622bf842dbf509aae987e716a813911d7a Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 7 Sep 2024 21:32:15 -0700 Subject: [PATCH] AMORDEGRC and Csv Reader Backports of PR #4162 and PR #4164 intended for Php 3.0.0. Signed-off-by: oleibman <10341515+oleibman@users.noreply.github.com> --- CHANGELOG.md | 22 ++++--------- .../Calculation/Financial/Amortization.php | 4 +-- src/PhpSpreadsheet/Helper/Sample.php | 7 +++- src/PhpSpreadsheet/Reader/Csv.php | 33 ++++++++++++++----- .../Functional/StreamTest.php | 12 ++++++- .../Writer/Dompdf/PaperSizeArrayTest.php | 18 ++++++++-- 6 files changed, 65 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e9ffd6f92..3490c45fe1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,27 +5,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com) and this project adheres to [Semantic Versioning](https://semver.org). -## TBD - 2.2.3 +## TBD - 2.3.0 -### Added - -- Nothing yet. - -### Changed +### Fixed -- Nothing yet. +- Improve Xlsx Reader speed (backport of PR #4153 intended for 3.0.0). [Issue #3917](https://github.com/PHPOffice/PhpSpreadsheet/issues/3917) +- Change to Csv Reader (see below under Deprecated). Backport of PR #4162 intended for 3.0.0. [Issue #4161](https://github.com/PHPOffice/PhpSpreadsheet/issues/4161) +- Tweak to AMORDEGRC. Backport of PR #4164 intended for 3.0.0. ### Deprecated -- Nothing yet. - -### Moved - -- Nothing yet. - -### Fixed - -- Improve Xlsx Reader speed (backport of PR #4153 intended for 3.0.0). [Issue #3917](https://github.com/PHPOffice/PhpSpreadsheet/issues/3917) +- Php8.4 will deprecate the escape parameter of fgetcsv. Csv Reader is affected by this; code is changed to be unaffected, but this will mean a breaking change is coming with Php9. Any code which uses the default escape value of backslash will fail in Php9. It is recommended to explicitly set the escape value to null string before then. ## 2024-08-07 - 2.2.2 diff --git a/src/PhpSpreadsheet/Calculation/Financial/Amortization.php b/src/PhpSpreadsheet/Calculation/Financial/Amortization.php index e56fabe913..b53829b767 100644 --- a/src/PhpSpreadsheet/Calculation/Financial/Amortization.php +++ b/src/PhpSpreadsheet/Calculation/Financial/Amortization.php @@ -9,8 +9,6 @@ class Amortization { - private const ROUNDING_ADJUSTMENT = (PHP_VERSION_ID < 80400) ? 0 : 1e-14; - /** * AMORDEGRC. * @@ -82,7 +80,7 @@ public static function AMORDEGRC( $amortiseCoeff = self::getAmortizationCoefficient($rate); $rate *= $amortiseCoeff; - $rate += self::ROUNDING_ADJUSTMENT; + $rate = (float) (string) $rate; // ugly way to avoid rounding problem $fNRate = round($yearFrac * $rate * $cost, 0); $cost -= $fNRate; $fRest = $cost - $salvage; diff --git a/src/PhpSpreadsheet/Helper/Sample.php b/src/PhpSpreadsheet/Helper/Sample.php index eded9ae467..3e2ed75695 100644 --- a/src/PhpSpreadsheet/Helper/Sample.php +++ b/src/PhpSpreadsheet/Helper/Sample.php @@ -9,6 +9,7 @@ use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; use PhpOffice\PhpSpreadsheet\Writer\IWriter; +use PhpOffice\PhpSpreadsheet\Writer\Pdf\Dompdf; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; use RecursiveRegexIterator; @@ -125,7 +126,11 @@ public function write(Spreadsheet $spreadsheet, string $filename, array $writers $writerCallback($writer); } $callStartTime = microtime(true); - $writer->save($path); + if (PHP_VERSION_ID >= 80400 && $writer instanceof Dompdf) { + @$writer->save($path); + } else { + $writer->save($path); + } $this->logWrite($writer, $path, $callStartTime); if ($this->isCli() === false) { // @codeCoverageIgnoreStart diff --git a/src/PhpSpreadsheet/Reader/Csv.php b/src/PhpSpreadsheet/Reader/Csv.php index e3cf0dc325..762ec31910 100644 --- a/src/PhpSpreadsheet/Reader/Csv.php +++ b/src/PhpSpreadsheet/Reader/Csv.php @@ -62,8 +62,19 @@ class Csv extends BaseReader /** * The character that can escape the enclosure. + * This will probably become unsupported in Php 9. + * Not yet ready to mark deprecated in order to give users + * a migration path. */ - private string $escapeCharacter = '\\'; + private ?string $escapeCharacter = null; + + /** + * The character that will be supplied to fgetcsv + * when escapeCharacter is null. + * It is anticipated that it will conditionally be set + * to null-string for Php9 and above. + */ + private static string $defaultEscapeCharacter = '\\'; /** * Callback for setting defaults in construction. @@ -185,7 +196,7 @@ protected function inferSeparator(): void return; } - $inferenceEngine = new Delimiter($this->fileHandle, $this->escapeCharacter, $this->enclosure); + $inferenceEngine = new Delimiter($this->fileHandle, $this->escapeCharacter ?? self::$defaultEscapeCharacter, $this->enclosure); // If number of lines is 0, nothing to infer : fall back to the default if ($inferenceEngine->linesCounted() === 0) { @@ -228,11 +239,11 @@ public function listWorksheetInfo(string $filename): array $delimiter = $this->delimiter ?? ''; // Loop through each line of the file in turn - $rowData = self::getcsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); + $rowData = self::getCsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); while (is_array($rowData)) { ++$worksheetInfo[0]['totalRows']; $worksheetInfo[0]['lastColumnIndex'] = max($worksheetInfo[0]['lastColumnIndex'], count($rowData) - 1); - $rowData = self::getcsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); + $rowData = self::getCsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); } $worksheetInfo[0]['lastColumnLetter'] = Coordinate::stringFromColumnIndex($worksheetInfo[0]['lastColumnIndex'] + 1); @@ -379,7 +390,7 @@ private function loadStringOrFile(string $filename, Spreadsheet $spreadsheet, bo // Loop through each line of the file in turn $delimiter = $this->delimiter ?? ''; - $rowData = self::getcsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); + $rowData = self::getCsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); $valueBinder = Cell::getValueBinder(); $preserveBooleanString = method_exists($valueBinder, 'getBooleanConversion') && $valueBinder->getBooleanConversion(); $this->getTrue = Calculation::getTRUE(); @@ -416,7 +427,7 @@ private function loadStringOrFile(string $filename, Spreadsheet $spreadsheet, bo } ++$columnLetter; } - $rowData = self::getcsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); + $rowData = self::getCsv($fileHandle, 0, $delimiter, $this->enclosure, $this->escapeCharacter); ++$currentRow; } @@ -527,6 +538,11 @@ public function getContiguous(): bool return $this->contiguous; } + /** + * Php9 intends to drop support for this parameter in fgetcsv. + * Not yet ready to mark deprecated in order to give users + * a migration path. + */ public function setEscapeCharacter(string $escapeCharacter): self { $this->escapeCharacter = $escapeCharacter; @@ -536,7 +552,7 @@ public function setEscapeCharacter(string $escapeCharacter): self public function getEscapeCharacter(): string { - return $this->escapeCharacter; + return $this->escapeCharacter ?? self::$defaultEscapeCharacter; } /** @@ -664,8 +680,9 @@ private static function getCsv( ?int $length = null, string $separator = ',', string $enclosure = '"', - string $escape = '\\' + ?string $escape = null ): array|false { + $escape = $escape ?? self::$defaultEscapeCharacter; if (PHP_VERSION_ID >= 80400 && $escape !== '') { return @fgetcsv($stream, $length, $separator, $enclosure, $escape); } diff --git a/tests/PhpSpreadsheetTests/Functional/StreamTest.php b/tests/PhpSpreadsheetTests/Functional/StreamTest.php index 3430b7a31b..c67c1ab22c 100644 --- a/tests/PhpSpreadsheetTests/Functional/StreamTest.php +++ b/tests/PhpSpreadsheetTests/Functional/StreamTest.php @@ -8,6 +8,12 @@ use PhpOffice\PhpSpreadsheet\Spreadsheet; use PHPUnit\Framework\TestCase; +/** + * Not clear that Dompdf will be Php8.4 compatible in time. + * Run in separate process and add version test till it is ready. + * + * @runTestsInSeparateProcesses + */ class StreamTest extends TestCase { public static function providerFormats(): array @@ -42,7 +48,11 @@ public function testAllWritersCanWriteToStream(string $format): void } else { self::assertSame(0, $stat['size']); - $writer->save($stream); + if ($format === 'Dompdf' && PHP_VERSION_ID >= 80400) { + @$writer->save($stream); + } else { + $writer->save($stream); + } self::assertIsResource($stream, 'should not close the stream for further usage out of PhpSpreadsheet'); $stat = fstat($stream); diff --git a/tests/PhpSpreadsheetTests/Writer/Dompdf/PaperSizeArrayTest.php b/tests/PhpSpreadsheetTests/Writer/Dompdf/PaperSizeArrayTest.php index 4c0b86b637..ccd0e84f05 100644 --- a/tests/PhpSpreadsheetTests/Writer/Dompdf/PaperSizeArrayTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Dompdf/PaperSizeArrayTest.php @@ -10,6 +10,12 @@ use PhpOffice\PhpSpreadsheet\Writer\Pdf\Dompdf; use PHPUnit\Framework\TestCase; +/** + * Not clear that Dompdf will be Php8.4 compatible in time. + * Run in separate process and add version test till it is ready. + * + * @runTestsInSeparateProcesses + */ class PaperSizeArrayTest extends TestCase { private string $outfile = ''; @@ -36,7 +42,11 @@ public function testPaperSizeArray(): void $sheet->setCellValue('A7', 'Lorem Ipsum'); $writer = new Dompdf($spreadsheet); $this->outfile = File::temporaryFilename(); - $writer->save($this->outfile); + if (PHP_VERSION_ID >= 80400) { // hopefully temporary + @$writer->save($this->outfile); + } else { + $writer->save($this->outfile); + } $spreadsheet->disconnectWorksheets(); unset($spreadsheet); $contents = file_get_contents($this->outfile); @@ -56,7 +66,11 @@ public function testPaperSizeNotArray(): void $sheet->setCellValue('A7', 'Lorem Ipsum'); $writer = new Dompdf($spreadsheet); $this->outfile = File::temporaryFilename(); - $writer->save($this->outfile); + if (PHP_VERSION_ID >= 80400) { // hopefully temporary + @$writer->save($this->outfile); + } else { + $writer->save($this->outfile); + } $spreadsheet->disconnectWorksheets(); unset($spreadsheet); $contents = file_get_contents($this->outfile);