AMORDEGRC and Csv Reader

Backports of PR #4162 and PR #4164 intended for Php 3.0.0. Signed-off-by: oleibman <[email protected]>
PHPOffice · Sep 8, 2024 · c2428f6 · c2428f6
1 parent c370bfc
commit c2428f6
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);