Skip to content

Commit

Permalink
New Looming Problems with PHP8.1
Browse files Browse the repository at this point in the history
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 PHPOffice#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.
  • Loading branch information
oleibman committed Jul 23, 2021
1 parent 3c5750b commit 8d699db
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 31 deletions.
10 changes: 0 additions & 10 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/PhpSpreadsheet/Worksheet/ColumnIterator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
4 changes: 1 addition & 3 deletions src/PhpSpreadsheet/Worksheet/Iterator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 1 addition & 3 deletions src/PhpSpreadsheet/Worksheet/RowCellIterator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 1 addition & 3 deletions src/PhpSpreadsheet/Worksheet/RowIterator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
11 changes: 3 additions & 8 deletions tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}

0 comments on commit 8d699db

Please sign in to comment.