@dFayet mget requires minimum one key

@yatsukhnenko The mget behaviour is probably right, but the doc (and the function return type) is incorrect, or it misses an exception if the $array parameter is empty

@yatsukhnenko I would expect that requirement to be enforced somehow (with an exception, for instance)

@greg0ire this will be breaking change and we can't do it until next major release

Sure! I'm not in a hurry ;)

I'm not sure about this change. @greg0ire why do you think that returning correct response on incorrect request is right behaviour?

If by "correct response", you mean false, I don't see how it is correct: it's not documented, and it makes the return type a union type. I'd much rather get an exception than just false. Many PHP functions return false in case of failure, but I believe this is the legacy of a time when exceptions did not exist. And PHP is moving away from that in some cases: https://wiki.php.net/rfc/json_throw_on_error

I think what @yatsukhnenko is saying is that MGET with no keys is a syntax error

127.0.0.1:6379> mget
(error) ERR wrong number of arguments for 'mget' command

Phpredis is identifying this and avoiding a round trip to the server. That said I'm not opposed to just returning an empty array here but like Pavlo mentioned it would be a breaking change so it has to wait until PhpRedis 6.

I'm not suggesting you return an empty array, I'm suggesting you throw an exception, which would make the signature of mget be mGet(array $keys): array rather than mGet(array $keys): array|false (and should wait until PhpRedis 6 too, for obvious reasons).

@greg0ire json_decode throws exception because it may return false/null is success and failure cases which doesn't allow to identify error. In phpredis we don't have this problem and use false may definetly identify error. Also I think that simple compare to false is better to read and understand than wrapping code to try/catch blocks

@yatsukhnenko It might be easier but you will allow an incorrect behaviour, as the exception is thrown because the user tries to mget with no keys (which is not supported, as you said earlier).
From a DX pov I think it is more easier to debug an exception than a false.

Also I think that simple compare to false is better to read and understand than wrapping code to try/catch blocks

It is indeed simpler! The thing is, that kind of exception is not meant to be caught, and should instead result in a rewrite of the calling code, just like @dFayet and I rewrote our code today to avoid calling mGet with an empty array. Having a clear exception message telling us should not call mGet with an empty array would have IMO been a far better experience than getting a TypeError about false not being an array as we did.

This should be revisited because the signature is now \Redis::mget(): \Redis|array, it mustn't return false (or the stub needs to be fixed), currently it says it will do one thing and then does another.

false also returned in case of errors like noAUTH Authentication required. Tested with phpredis 6.0.2, so stubs are wrong