InvalidKeyInArray: report float deprecation since PHP 8.1 #3220
Conversation
|
You've opened the pull request against the latest branch 1.12.x. If your code is relevant on 1.11.x and you want it to be released sooner, please rebase your pull request and change its target to 1.11.x. |
|
This pull request has been marked as ready for review. |
|
Seeing the integration test failures, this is often triggered when numeric operation is performed with mixed. E.g. |
|
would it help if |
|
Saying often is maybe too abmitious, I checked few :) It would be great if integration tests used |
|
the error at https://github.com/pmmp/PocketMine-MP/blob/8dedbb747108c45b7534b8ac6f05e8464ee8eb22/src/crash/CrashDump.php#L229-L234 looks like it might be useful handle but maybe its just #3220 (comment) alternatively you could consider not reporting the "maybe" error for floats because of the too high false-positive rate. lets see what ondrey thinks about it |
| ]; | ||
| } | ||
|
|
||
| if ($this->phpVersion->getVersionId() >= 80100) { |
There was a problem hiding this comment.
Instead of condition like this, we need a new method on PhpVersion that would be used here.
| if ($isFloat->yes()) { | ||
| return [ | ||
| RuleErrorBuilder::message( | ||
| 'Float used as array key, this emits deprecation notice.', |
There was a problem hiding this comment.
Message format to be used: Deprecated in PHP 8.1: Float can no longer be used as array key, %s given.
| return [ | ||
| RuleErrorBuilder::message( | ||
| 'Float used as array key, this emits deprecation notice.', | ||
| )->identifier('array.invalidOffset')->build(), |
There was a problem hiding this comment.
array.floatKey might be better?
| if ($this->phpVersion->getVersionId() >= 80100) { | ||
| $isFloat = $dimensionType->isFloat(); | ||
|
|
||
| if ($isFloat->yes()) { |
There was a problem hiding this comment.
We need to use RuleLevelHelper::findTypeToCheck here, it's going to help us on level 9 and also with benevolent union types.
Case in point: $mixed - 1 is already benevolent union type so this problem would already be solved: https://phpstan.org/r/5af2741f-b958-42f7-9248-435416d1b584
| @@ -47,6 +51,24 @@ public function processNode(Node $node, Scope $scope): array | |||
| ]; | |||
| } | |||
|
|
|||
| if ($this->phpVersion->getVersionId() >= 80100) { | |||
There was a problem hiding this comment.
All the same points as in the other place :)
|
We should also have a test for this in LevelsIntegrationTest. The best way to develop that is to comment out all the existing "topics", and add a new one. Write a few code samples in there and run the test. It will (re)generate the current levels-snapshot JSON. The behaviour I'd like this to have:
The thing about |
|
Altough I agree with your points, I'm not sure I'll invest the time into all that. Mostly because the core issue is covered in our (even stricter) rule: shipmonk-rnd/phpstan-rules#254 |
No description provided.