Fix Redis::keys function signature #2706

Vaalyn · 2023-10-31T14:32:10Z

This PR fixes the Redis::keys function signature to match the stub provided by the Redis extension: https://github.com/phpredis/phpredis/blob/a0c8fcc589bfcf8207ba3d8c7d065109f31cc1c6/redis.stub.php#L1948-L1949

Currently the functionMap says it can only return an array<int,string> but it can also return an instance of Redis when multimode is used or false when no server is reachable.

ondrejmirtes · 2023-10-31T16:58:49Z

There are many functions like this in the Redis extension, please fix all of them.
Please wrap the return types in __benevolent<...> so that this change isn't too annoying on level 7+.

Vaalyn · 2023-10-31T18:13:09Z

👍 will go through all the Redis:: functions to fix the ones that are wrong and if applicable add missing ones.

Vaalyn · 2023-11-01T16:00:54Z

I've gone through all Redis:: entries in the functionMap.php, looking them up in the redis.stub.php from the Redis extension, fixing the return types, parameters, adding missing functions (as thorough as I could, maybe missed some but there are so many in there... 😅) and removing those that were removed from the extension.

ondrejmirtes · 2023-11-02T13:38:51Z

Hello, thank you for this effort, the tests are failing.

and removing those that were removed from the extension

Which methods did you remove and when they were removed from the extension?

Vaalyn · 2023-11-02T14:52:36Z

the tests are failing

Could you point me in the correct direction as to why they might be failing?

The error only says that one of the array keys for the parameters seems to be an integer but as far as I can see they are all strings:

TypeError: Argument 1 passed to PHPStan\Reflection\SignatureMap\SignatureMapParser::getParameterInfoFromName() must be of the type string, int given, called in /home/runner/work/phpstan-src/phpstan-src/src/Reflection/SignatureMap/SignatureMapParser.php on line 67

Which methods did you remove and when they were removed from the extension?

'Redis::blPop\'1' => as far as I could see there are is no alternative function signature in the extension
'Redis::brPop\'1' => as far as I could see there are is no alternative function signature in the extension
'Redis::decrByFloat' => only available in RedisCluster, not in Redis
'Redis::del\'1' => as far as I could see there are is no alternative function signature in the extension
'Redis::delete\'1' => as far as I could see there are is no alternative function signature in the extension
'Redis::evaluate' => no idea, can't find anything about it on the repo
'Redis::exists\'1' => as far as I could see there are is no alternative function signature in the extension
'Redis::getKeys' => alias for "keys", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::getMultiple' => alias for "mget", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::lGet' => alias for "lindex", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::lGetRange' => alias for "lrange", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::listTrim' => alias for "ltrim", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::lSize' => alias for "lLen", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::renameKey' => alias for "rename", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::resetStat' => no idea, can't find anything about it on the repo
'Redis::sContains' => alias for "sismember", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::set\'1' => as far as I could see there are is no alternative function signature in the extension
'Redis::setTimeout' => alias for "expire", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::sGetMembers' => alias for "sMembers", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::slave' => it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::slave\'1' => no idea, see above
'Redis::sRemove' => alias for "srem", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::substr' => alias for "getRange", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::unlink\'1' => as far as I could see there are is no alternative function signature in the extension
'Redis::zAdd\'1' => as far as I could see there are is no alternative function signature in the extension
'Redis::zDelete' => alias for "zRem", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::zDeleteRangeByRank' => alias for "zRemRangeByRank", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::zDeleteRangeByScore' => alias for "zRemRangeByScore", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::zRemove' => alias for "zRem", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there
'Redis::zSize' => alias for "zCard", readme says it will be removed in future versions but it's not in their stub file for at least 1 year no idea when exactly it got yeeted from there

ondrejmirtes · 2023-11-02T14:56:30Z

The error only says that one of the array keys for the parameters seems to be an integer but as far as I can see they are all strings:

If you named a parameter like '1', it's going to be cast by PHP to an int.

You should be able to debug this by running the test and print what it's currently running around that line.

Vaalyn · 2023-11-02T15:56:43Z

Found the cause of the error by var_dumping while running the tests locally.

It was due to one of the parameters missing the parameter name.

ondrejmirtes · 2023-11-02T16:16:17Z

Please return the removed functions back to the file, someone might be still using them.

Vaalyn · 2023-11-02T16:30:59Z

@ondrejmirtes added them back into the map 👍

Is there any guide on when to delete removed functions?

ondrejmirtes · 2023-11-02T16:38:39Z

when to delete removed functions

There isn't. Basically there is never a good reason to remove functions that might still be in use.

ondrejmirtes · 2023-11-02T16:40:39Z

resources/functionMap.php

-'Redis::bitOp' => ['int', 'operation'=>'string', '...args'=>'string'],
-'Redis::bitpos' => ['int', 'key'=>'string', 'bit'=>'int', 'start='=>'int', 'end='=>'int'],
-'Redis::blPop' => ['array', 'keys'=>'string[]', 'timeout'=>'int'],
+'Redis::__construct' => ['void', 'options'=>'?array{host?:string,port?:int,connectTimeout?:float,auth?:list{string|null|false,string}|list{string},ssl?:array<string, mixed>,backoff?:array<string, mixed>}'],


The options parameter should be optional

ondrejmirtes · 2023-11-02T16:51:12Z

Thank you! You might also be interested in cleaning this up in https://github.com/JetBrains/phpstorm-stubs/tree/master/redis :)

RobiNN1 · 2023-11-21T20:46:27Z

Probably after this PR there is issue Method Redis::restore() invoked with 3 parameters, 4 required.

4th parameter is optional $options = null

https://phpstan.org/r/00184a69-6093-4fef-a5fe-abf0be19b79f

ondrejmirtes · 2023-11-22T07:19:03Z

@RobiNN1 Please open a bug report.

Fix Redis::keys function signature

77c0d78

Update function map Redis:: entries

302b804

Fix error in test

d8c45e5

Fix parameter error

8c467f6

Re-add deleted function signatures

3b25628

ondrejmirtes requested changes Nov 2, 2023

View reviewed changes

Make parameter optional

de60a2f

ondrejmirtes merged commit d63e030 into phpstan:1.10.x Nov 2, 2023
412 of 417 checks passed

taka-oyama mentioned this pull request Nov 6, 2023

fix: incorrect argument type for Redis::setex / Redis::setnx #2634

Closed

ondrejmirtes mentioned this pull request Nov 17, 2023

Fix Redis facade tests larastan/larastan#1779

Merged

2 tasks

RobiNN1 mentioned this pull request Nov 22, 2023

The optional parameter in Redis::restore() is now reported as required phpstan/phpstan#10176

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Redis::keys function signature #2706

Fix Redis::keys function signature #2706

Vaalyn commented Oct 31, 2023

ondrejmirtes commented Oct 31, 2023

Vaalyn commented Oct 31, 2023

Vaalyn commented Nov 1, 2023

ondrejmirtes commented Nov 2, 2023

Vaalyn commented Nov 2, 2023

ondrejmirtes commented Nov 2, 2023

Vaalyn commented Nov 2, 2023

ondrejmirtes commented Nov 2, 2023

Vaalyn commented Nov 2, 2023 •

edited

ondrejmirtes commented Nov 2, 2023

ondrejmirtes Nov 2, 2023

Vaalyn Nov 2, 2023

ondrejmirtes commented Nov 2, 2023

RobiNN1 commented Nov 21, 2023 •

edited

ondrejmirtes commented Nov 22, 2023

Fix Redis::keys function signature #2706

Fix Redis::keys function signature #2706

Conversation

Vaalyn commented Oct 31, 2023

ondrejmirtes commented Oct 31, 2023

Vaalyn commented Oct 31, 2023

Vaalyn commented Nov 1, 2023

ondrejmirtes commented Nov 2, 2023

Vaalyn commented Nov 2, 2023

ondrejmirtes commented Nov 2, 2023

Vaalyn commented Nov 2, 2023

ondrejmirtes commented Nov 2, 2023

Vaalyn commented Nov 2, 2023 • edited

ondrejmirtes commented Nov 2, 2023

ondrejmirtes Nov 2, 2023

Choose a reason for hiding this comment

Vaalyn Nov 2, 2023

Choose a reason for hiding this comment

ondrejmirtes commented Nov 2, 2023

RobiNN1 commented Nov 21, 2023 • edited

ondrejmirtes commented Nov 22, 2023

Vaalyn commented Nov 2, 2023 •

edited

RobiNN1 commented Nov 21, 2023 •

edited