Skip to content

Commit

Permalink
AMORDEGRC and Csv Reader
Browse files Browse the repository at this point in the history
Backports of PR #4162 and PR #4164 intended for 3.0.0.
  • Loading branch information
oleibman committed Sep 8, 2024
1 parent 8edc294 commit ec0d4af
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 12 deletions.
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,19 @@ 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).

## 1.29.2 - TBD

### Fixed

- Backported security patches.
- Support for Php8.4.
- 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)
- Tweaks to ROUNDUP, ROUNDDOWN, TRUNC, AMORDEGRC (results had been different under 8.4).

### Deprecated

- 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.

## 1.29.1 - 2024-09-03

### Fixed
Expand Down
4 changes: 1 addition & 3 deletions src/PhpSpreadsheet/Calculation/Financial/Amortization.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@

class Amortization
{
private const ROUNDING_ADJUSTMENT = (PHP_VERSION_ID < 80400) ? 0 : 1e-14;

/**
* AMORDEGRC.
*
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion src/PhpSpreadsheet/Helper/Sample.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,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;
Expand Down Expand Up @@ -137,7 +138,11 @@ public function write(Spreadsheet $spreadsheet, $filename, array $writers = ['Xl
$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, /** @scrutinizer ignore-type */ $callStartTime);
if ($this->isCli() === false) {
echo '<a href="/download.php?type=' . pathinfo($path, PATHINFO_EXTENSION) . '&name=' . basename($path) . '">Download ' . basename($path) . '</a><br />';
Expand Down
24 changes: 19 additions & 5 deletions src/PhpSpreadsheet/Reader/Csv.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,17 @@ class Csv extends BaseReader
/**
* The character that can escape the enclosure.
*
* @var string
* @var ?string
*/
private $escapeCharacter = '\\';
private $escapeCharacter;

/**
* 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.
Expand Down Expand Up @@ -198,7 +206,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) {
Expand Down Expand Up @@ -529,6 +537,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;
Expand All @@ -538,7 +551,7 @@ public function setEscapeCharacter(string $escapeCharacter): self

public function getEscapeCharacter(): string
{
return $this->escapeCharacter;
return $this->escapeCharacter ?? self::$defaultEscapeCharacter;
}

/**
Expand Down Expand Up @@ -657,8 +670,9 @@ private static function getCsv(
?int $length = null,
string $separator = ',',
string $enclosure = '"',
string $escape = '\\'
?string $escape = null
) {
$escape = $escape ?? self::$defaultEscapeCharacter;
if (PHP_VERSION_ID >= 80400 && $escape !== '') {
return @fgetcsv($stream, $length, $separator, $enclosure, $escape);
}
Expand Down
12 changes: 11 additions & 1 deletion tests/PhpSpreadsheetTests/Functional/StreamTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,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
Expand Down Expand Up @@ -40,7 +46,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);
Expand Down
18 changes: 16 additions & 2 deletions tests/PhpSpreadsheetTests/Writer/Dompdf/PaperSizeArrayTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,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
{
/** @var string */
Expand Down Expand Up @@ -35,7 +41,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);
Expand All @@ -55,7 +65,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);
Expand Down

0 comments on commit ec0d4af

Please sign in to comment.