From b9f6c70b86fb3d331ff774db9bbb30d6717b534a Mon Sep 17 00:00:00 2001 From: oleibman Date: Tue, 3 Aug 2021 21:37:53 -0700 Subject: [PATCH] New Looming Problems with PHP8.1 (#2231) * 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. * Minor Scrutinizer Error Hopefully fixed now. --- 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 a12fd2f5..49dd41a4 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -6575,16 +6575,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 0d771ff7..e6b79c7a 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -4368,7 +4368,7 @@ class Calculation $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 0651a7a4..179e9f27 100644 --- a/src/PhpSpreadsheet/Worksheet/ColumnIterator.php +++ b/src/PhpSpreadsheet/Worksheet/ColumnIterator.php @@ -131,10 +131,8 @@ class ColumnIterator implements Iterator /** * 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 134b619a..a4061b78 100644 --- a/src/PhpSpreadsheet/Worksheet/Iterator.php +++ b/src/PhpSpreadsheet/Worksheet/Iterator.php @@ -63,10 +63,8 @@ class Iterator implements \Iterator /** * 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 6a96a826..fc04ccd3 100644 --- a/src/PhpSpreadsheet/Worksheet/RowCellIterator.php +++ b/src/PhpSpreadsheet/Worksheet/RowCellIterator.php @@ -153,10 +153,8 @@ class RowCellIterator extends CellIterator /** * 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 7d49f1ac..7fe0f80b 100644 --- a/src/PhpSpreadsheet/Worksheet/RowIterator.php +++ b/src/PhpSpreadsheet/Worksheet/RowIterator.php @@ -115,10 +115,8 @@ class RowIterator implements Iterator /** * 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 3a9fb1cc..13a13bc9 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php @@ -31,7 +31,7 @@ class LocaleFloatsTest extends TestCase protected function tearDown(): void { - if ($this->localeAdjusted) { + if ($this->localeAdjusted && is_string($this->currentLocale)) { setlocale(LC_ALL, $this->currentLocale); } } @@ -55,12 +55,7 @@ class LocaleFloatsTest extends TestCase $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); } }