-
Notifications
You must be signed in to change notification settings - Fork 545
Fix Memcache/MemcachePool get method signature #2323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
resources/functionMap.php
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we get more precise, e.g. string[] ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@staabm Good idea. Just added commit with string[] for key and int[] for flags
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add these as separate variants? Because they are separate variants at https://www.php.net/manual/en/memcache.get.php
What I mean by that - look at the top of the file and see the function abs():
'abs' => ['0|positive-int', 'number'=>'int'],
'abs\'1' => ['float', 'number'=>'float'],
'abs\'2' => ['float|0|positive-int', 'number'=>'string'],
You can also test that in NodeScopeResolverTest - passing string should return string, passing array should return array.
@ondrejmirtes Sure, I didn't know it was possible to define variants. makes more sense.
Using variants in the latest commit.
Also fixed return value. Memcache::get deserializes so return value should be mixed i.e. mixed[]|false
You can also test that in NodeScopeResolverTest - passing string should return string, passing array should return array.
It seems that PHP documentation isn't correct regarding Memcache::get return type. It doesn't return string|array, but mixed|mixed[] type depending on value. Below is a simple script that checks this.
How to test this in NodeScopeResolverTest? Is there a stub/mock of Memcache class that I could use? I didn't find any.
<?php declare(strict_types=1); $memcache = new Memcache(); $memcache->connect('localhost'); $memcache->set('key-number', 123); $memcache->set('key-string', "abc"); $memcache->set('key-array', array(1,2,3)); $memcache->set('key-object', (object) array(1,2,3)); echo "\n---- Memcache::get (single) ----\n"; echo "key-number type -> " . gettype($memcache->get('key-number')) . "\n"; echo "key-string type -> " . gettype($memcache->get('key-string')) . "\n"; echo "key-array type -> " . gettype($memcache->get('key-array')) . "\n"; echo "key-object type -> " . gettype($memcache->get('key-object')) . "\n"; echo "\n---- Memcache::get (multi) ----\n"; $multi = $memcache->get(array('key-number', 'key-string', 'key-array', 'key-object')); foreach ($multi as $key => $value) { echo "$key type -> " . gettype($value) . "\n"; }
and here is the output of running it (php 8.1):
---- Memcache::get (single) ----
key-number type -> integer
key-string type -> string
key-array type -> array
key-object type -> object
---- Memcache::get (multi) ----
key-number type -> integer
key-string type -> string
key-array type -> array
key-object type -> object
Here's the documentation about testing: https://phpstan.org/developing-extensions/testing#type-inference
You can also check out all the files linked in NodeScopeResolverTest to get the idea how it works.
@ondrejmirtes just added tests to the NodeScopeResolverTest. hope it's ok now.
e92b685 to
8fce724
Compare
@dravnic 1) Please base it on top of 1.10.x.
2) Rebase it so that it contains only your own commits and no merge commits.
8fce724 to
0745fbd
Compare
https://www.php.net/manual/en/memcache.get.php