phpstan-src icon indicating copy to clipboard operation
phpstan-src copied to clipboard

Remove Redis signatures from functionMap and rely solely on phpstorm-stubs

Open ondrejmirtes opened this issue 7 months ago • 5 comments

ondrejmirtes avatar Jun 10 '25 07:06 ondrejmirtes

I closed my PR, because it will not be merged anyway. Now I'm checking that reflection test.

But where is this from? (There are many issues like this)

     /**
      * @param string $key
-     * @param int $min
-     * @param int $max
-     * @param int $options
-     * @param int $limit
-     * @return array
+     * @param string $min
+     * @param string $max
+     * @param array|null $options
+     * @return array|bool|RedisCluster
      */
-    function zRevRangeByLex(string $key, string $min, string $max, array|null $options = null, mixed $limit): array|bool|RedisCluster'
+    function zRevRangeByLex(string $key, string $min, string $max, array|null $options = null): array|bool|RedisCluster'

Here is from phpstorm stub file which was recently updated https://github.com/JetBrains/phpstorm-stubs/pull/1750

    /**
     * @param   string $key
     * @param   string $min
     * @param   string $max
     * @param   null|array $options
     *
     * @return  RedisCluster|bool|array
     * @throws  RedisClusterException
     * @see     zRangeByLex()
     *
     * @link    https://redis.io/commands/zrevrangebylex
     */
    public function zRevRangeByLex(string $key, string $min, string $max, ?array $options = null): RedisCluster|bool|array {}

There are few phpdocs issues but function signatures are correct.

RobiNN1 avatar Jun 13 '25 22:06 RobiNN1

Yeah, that's expected. The new (+ lines in the diff) match exactly what's in phpstorm-stubs. Because I removed the old (wrong) signatures from functionMap.php.

ondrejmirtes avatar Jun 14 '25 15:06 ondrejmirtes

I don't think it will be possible to rely on phpstorm-stubs for redis because we're using __benevolent types for some return values. This PR might be closed without replacement, and we'll fix redis issues one by one when reported...

VincentLanglet avatar Oct 05 '25 15:10 VincentLanglet

There are way too much changes since RedisCluster stubs are outdated here. I sent PR for this before, but it was hard to review.

Anyway I noticed these issues with latest phpstan

Method RedisCluster::rawcommand() invoked with 2 parameters, 3 required

public function rawcommand(string|array $key_or_address, string $command, mixed ...$args): mixed {} $args is optional

Method RedisCluster::info() invoked with 2 parameters, 0-1 required. public function info(string|array $key_or_address, string ...$sections): RedisCluster|array|false {}

In most of signatures is missing new string|array $key_or_address

RobiNN1 avatar Oct 05 '25 17:10 RobiNN1

There are way too much changes since RedisCluster stubs are outdated here. I sent PR for this before, but it was hard to review.

I recommand to reopen https://github.com/phpstan/phpstan-src/pull/4045/files but with smaller PR, you could

  • Fix existing definition in one PR
  • Add new definition in another PR

And if the fix existing definition is still to big you can split into smaller BR, maybe

  • One PR to add missing parameters
  • One PR to fix some param type
  • One PR to fix return type

Or limiting PR at 5-10 functions signatures fixes

VincentLanglet avatar Oct 05 '25 18:10 VincentLanglet