From 8d699dbaf6f30e81d4ca8ec61423ec2e7f23ce03 Mon Sep 17 00:00:00 2001 From: Owen Leibman Date: Thu, 22 Jul 2021 20:57:08 -0700 Subject: [PATCH 1/2] New Looming Problems with PHP8.1 More deprecations. The following corrections are made in this PR: - Calculation.php has a call to ctype_upper and apparently one of the samples manages to pass it an int. That function treats int differently from numeric strings, and that treatment is on the deprecation list. Enclosing the argument in quotes cannot cause a problem unless the int represents the ASCII value of an uppercase letter, which I cannot believe is the case; anyhow, if it is, the code will wind up with a nonsense result, e.g. if column is C and row is 1, the cell will be resolved as C1, but if column is int 67 (ASCII for C) and row is 1, the cell will be resolved as 671, not C1. - Several Worksheet iterators need one or more functions to explicitly declare their return types. Thankfully, this does not seem to break earlier PHP versions. - LocaleFloatsTest - see issue #1863. This was supposed to fail in PHP 8.0, but var_dump continued to support the old way (for 64-bit PHP only, not for 32-bit). PHP 8.1 appears to correct that omission, and the test will now fail. It doesn't show up as a failure in Github because of an accident - the attempt to set the locale to France in Github fails, so it skips the test before attempting the var_dump. But it does fail locally on my system. I have changed the test to use sprintf rather than var_dump; I think users are far more likely to use sprintf rather than var_dump in their applications. (They are, of course, even more likely to just cast to string, but the result of doing that is already different in 8.0 than in 7.4.) I would be equally happy to delete the test altogether. There remain PHP 8.1 problems with Mpdf which are, of course, out of scope here. There is one additional problem that I do not address in this ticket. The auto_detect_line_endings setting is being deprecated. This has some implications for Csv. I have another PR ready for Csv, and will discuss that problem there. --- phpstan-baseline.neon | 10 ---------- src/PhpSpreadsheet/Calculation/Calculation.php | 2 +- src/PhpSpreadsheet/Worksheet/ColumnIterator.php | 4 +--- src/PhpSpreadsheet/Worksheet/Iterator.php | 4 +--- src/PhpSpreadsheet/Worksheet/RowCellIterator.php | 4 +--- src/PhpSpreadsheet/Worksheet/RowIterator.php | 4 +--- .../Writer/Xlsx/LocaleFloatsTest.php | 11 +++-------- 7 files changed, 8 insertions(+), 31 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index b970b1f34f..c4e15b069e 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -6660,16 +6660,6 @@ parameters: count: 3 path: tests/PhpSpreadsheetTests/Writer/Html/HtmlCommentsTest.php - - - message: "#^Parameter \\#2 \\$locale of function setlocale expects string\\|null, string\\|false given\\.$#" - count: 1 - path: tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php - - - - message: "#^Parameter \\#2 \\$subject of function preg_match expects string, string\\|false given\\.$#" - count: 1 - path: tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php - - message: "#^Cannot call method getDrawingCollection\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\|null\\.$#" count: 4 diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 0d771ff77c..e6b79c7a14 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -4368,7 +4368,7 @@ private static function dataTestReference(&$operandData) $rowKey = array_shift($rKeys); $cKeys = array_keys(array_keys($operand[$rowKey])); $colKey = array_shift($cKeys); - if (ctype_upper($colKey)) { + if (ctype_upper("$colKey")) { $operandData['reference'] = $colKey . $rowKey; } } diff --git a/src/PhpSpreadsheet/Worksheet/ColumnIterator.php b/src/PhpSpreadsheet/Worksheet/ColumnIterator.php index 0651a7a458..179e9f27bb 100644 --- a/src/PhpSpreadsheet/Worksheet/ColumnIterator.php +++ b/src/PhpSpreadsheet/Worksheet/ColumnIterator.php @@ -131,10 +131,8 @@ public function rewind(): void /** * Return the current column in this worksheet. - * - * @return Column */ - public function current() + public function current(): Column { return new Column($this->worksheet, Coordinate::stringFromColumnIndex($this->currentColumnIndex)); } diff --git a/src/PhpSpreadsheet/Worksheet/Iterator.php b/src/PhpSpreadsheet/Worksheet/Iterator.php index 134b619a68..a4061b787e 100644 --- a/src/PhpSpreadsheet/Worksheet/Iterator.php +++ b/src/PhpSpreadsheet/Worksheet/Iterator.php @@ -63,10 +63,8 @@ public function next(): void /** * Are there more Worksheet instances available? - * - * @return bool */ - public function valid() + public function valid(): bool { return $this->position < $this->subject->getSheetCount() && $this->position >= 0; } diff --git a/src/PhpSpreadsheet/Worksheet/RowCellIterator.php b/src/PhpSpreadsheet/Worksheet/RowCellIterator.php index 6a96a826a5..fc04ccd313 100644 --- a/src/PhpSpreadsheet/Worksheet/RowCellIterator.php +++ b/src/PhpSpreadsheet/Worksheet/RowCellIterator.php @@ -153,10 +153,8 @@ public function prev(): void /** * Indicate if more columns exist in the worksheet range of columns that we're iterating. - * - * @return bool */ - public function valid() + public function valid(): bool { return $this->currentColumnIndex <= $this->endColumnIndex && $this->currentColumnIndex >= $this->startColumnIndex; } diff --git a/src/PhpSpreadsheet/Worksheet/RowIterator.php b/src/PhpSpreadsheet/Worksheet/RowIterator.php index 7d49f1acde..7fe0f80b3d 100644 --- a/src/PhpSpreadsheet/Worksheet/RowIterator.php +++ b/src/PhpSpreadsheet/Worksheet/RowIterator.php @@ -115,10 +115,8 @@ public function rewind(): void /** * Return the current row in this worksheet. - * - * @return Row */ - public function current() + public function current(): Row { return new Row($this->subject, $this->position); } diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php index 3a9fb1cc6c..03bd6ad2de 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php @@ -32,7 +32,7 @@ protected function setUp(): void protected function tearDown(): void { if ($this->localeAdjusted) { - setlocale(LC_ALL, $this->currentLocale); + setlocale(LC_ALL, $this->currentLocale ?: null); } } @@ -55,12 +55,7 @@ public function testLocaleFloatsCorrectlyConvertedByWriter(): void $result = $spreadsheet->getActiveSheet()->getCell('A1')->getValue(); - ob_start(); - var_dump($result); - preg_match('/(?:double|float)\(([^\)]+)\)/mui', ob_get_clean(), $matches); - self::assertArrayHasKey(1, $matches); - $actual = $matches[1]; - // From PHP8, https://wiki.php.net/rfc/locale_independent_float_to_string applies - self::assertEquals('1,1', $actual); + $actual = sprintf('%f', $result); + self::assertStringContainsString('1,1', $actual); } } From 56e767e009f5761bda505b7bba66891088e3c7b4 Mon Sep 17 00:00:00 2001 From: Owen Leibman Date: Thu, 22 Jul 2021 21:28:53 -0700 Subject: [PATCH 2/2] Minor Scrutinizer Error Hopefully fixed now. --- tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php index 03bd6ad2de..13a13bc98e 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php @@ -31,8 +31,8 @@ protected function setUp(): void protected function tearDown(): void { - if ($this->localeAdjusted) { - setlocale(LC_ALL, $this->currentLocale ?: null); + if ($this->localeAdjusted && is_string($this->currentLocale)) { + setlocale(LC_ALL, $this->currentLocale); } }