Correct Some Problems Which Will Show Up for PHP8.1#2191
Merged
MarkBaker merged 7 commits intoPHPOffice:masterfrom Jun 29, 2021
Merged
Correct Some Problems Which Will Show Up for PHP8.1#2191MarkBaker merged 7 commits intoPHPOffice:masterfrom
MarkBaker merged 7 commits intoPHPOffice:masterfrom
Conversation
Just reviewing Scrutinizer's list of "bugs". There are 19 ascribed to me. For some, I will definitely take no action (e.g. use of bitwise operators in AND, OR, and XOR functions). However, where I can clean things up so that Scrutinizer is satisfied and the resulting code is not too contorted, I will make an attempt. I believe this is the only one with which will involve more than 2 or 3 changes. It fixes 5 items ascribed to me, and 4 to others.
Per suggestion from Mark Baker.
PHP8.1 wants to issue a message when you use a float where it thinks you ought to be using an int (it wants its implicit casts made explicit). This is causing unit tests to fail. The following corrections are made in this PR: - Calculation.php tests `isset(self::binaryOperators[$token])`, where token can be a float. No numeric values are members of that array, so we can test for numeric before isset. - SharedOle.php packs a float, intending it as an int, in 2 places. I simplified the logic here, and added explicit casts to avoid the problem. This is used by Xls Reader and Writer; as added confirmation, I added some timestamps from before 1970 (i.e. negative values) to Document/EpochTest. Because of this, the test suite has been verified for 32-bit PHP as well as PHP 8.1. - Writer/Xlsx/StringTable tests `isset($aFlippedStringTable[$cellValue])`. This is the same problem as in Calculation, but requires a different solution. The same if statement here also tests that the datatype is string, but it does so after the isset test. Changing the order of these tests avoids the problem. There remain PHP8.1 errors having to do with vendor code used as a polyfill for Enum. @MarkBaker has reported the issue to the vendor.
Collaborator
Author
|
Apologies for the multiple commits. I started out from the wrong baseline. |
Member
|
Enum polyfill issue has now been resolved |
5 tasks
Member
|
Three issues now remain for PHP 8.1, all in mPdf |
Collaborator
Author
|
FWIW, I reported the Mpdf Php8.1 issues a while back. They closed my ticket immediately, and were, I thought, quite rude about it. The fix on their side is really easy, but I'm not going to attempt it again. Perhaps you'll have better luck. There is still an issue with LocaleFloatsTest. It fortuitously is skipped because there is no "French" environment set up when Github runs the unit tests. However, if there were one, the test would fail, for the same reason as I thought it would fail for 8.0 (issue #1663). |
BlackyTay
pushed a commit
to BlackyTay/PhpSpreadsheet
that referenced
this pull request
Aug 8, 2025
* Reader/Gnumeric vs. Scrutinizer Just reviewing Scrutinizer's list of "bugs". There are 19 ascribed to me. For some, I will definitely take no action (e.g. use of bitwise operators in AND, OR, and XOR functions). However, where I can clean things up so that Scrutinizer is satisfied and the resulting code is not too contorted, I will make an attempt. I believe this is the only one with which will involve more than 2 or 3 changes. It fixes 5 items ascribed to me, and 4 to others. * Use Strict Checking for in_array * Correct Some Problems Which Will Show Up for PHP8.1 PHP8.1 wants to issue a message when you use a float where it thinks you ought to be using an int (it wants its implicit casts made explicit). This is causing unit tests to fail. The following corrections are made in this PR: - Calculation.php tests `isset(self::binaryOperators[$token])`, where token can be a float. No numeric values are members of that array, so we can test for numeric before isset. - SharedOle.php packs a float, intending it as an int, in 2 places. I simplified the logic here, and added explicit casts to avoid the problem. This is used by Xls Reader and Writer; as added confirmation, I added some timestamps from before 1970 (i.e. negative values) to Document/EpochTest. Because of this, the test suite has been verified for 32-bit PHP as well as PHP 8.1. - Writer/Xlsx/StringTable tests `isset($aFlippedStringTable[$cellValue])`. This is the same problem as in Calculation, but requires a different solution. The same if statement here also tests that the datatype is string, but it does so after the isset test. Changing the order of these tests avoids the problem. * Update OLE.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is:
Checklist:
Why this change is needed?
Correct Some Problems Which Will Show Up for PHP8.1
PHP8.1 wants to issue a message when you use a float where it thinks you ought to be using an int (it wants its implicit casts made explicit). This is causing unit tests to fail. The following corrections are made in this PR:
isset(self::binaryOperators[$token]), where token can be a float. No numeric values are members of that array, so we can test for numeric before isset.isset($aFlippedStringTable[$cellValue]). This is the same problem as in Calculation, but requires a different solution. The same if statement here also tests that the datatype is string, but it does so after the isset test. Changing the order of these tests avoids the problem.There remain PHP8.1 errors having to do with vendor code used as a polyfill for Enum. @MarkBaker has reported the issue to the vendor.