Skip to content

Conversation

sayuprc
Copy link
Contributor

@sayuprc sayuprc commented Sep 14, 2025

The definition in functionMap.php was inconsistent with the RedisCluster stub.
This PR updates it to match the stub.

ref: #4283

@VincentLanglet
Copy link
Contributor

Hi @sayuprc, the diff of the PR is pretty huge, and then difficult for ondrej to review it.

Also, be aware there is no need to describe every method here since PHPStan rely on PHPStorm stubs
that's why I already removed some definition in #4231
For instance, you're adding _compress definition

'RedisCluster::_compress' => ['string', 'value'=>'string'],

but it's useless because it already exists (and is correct) on PHPStorm side
https://github.com/JetBrains/phpstorm-stubs/blob/d74d654cf966aaa56fd508c1f727c46313903aa2/redis/RedisCluster.php#L3653

So:

Also you can see the impact of the PR with the
Reflection golden test / Reflection golden test (8.5) (pull_request) and looking at the Reflection Golden test section

You'll see diff like

1) PHPStan\Reflection\ReflectionProviderGoldenTest::test with data set "METHOD RedisCluster::pfMerge" ('METHOD RedisCluster::pfMerge', 'Is built-in: Yes\nHas side-ef...luster')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
     /**
      * @param string $key
      * @param array $keys
-     * @return bool
+     * @return (bool|RedisCluster)
      */
     function pfmerge(string $key, array $keys): bool|RedisCluster'

/home/runner/work/phpstan-src/phpstan-src/tests/PHPStan/Reflection/ReflectionProviderGoldenTest.php:73

to check if it's ok.

Currently the golden test reflection reports 321 changes ; it's too many to be reviewed

@ondrejmirtes
Copy link
Member

Yeah, there's no need to "match a stub". If you want to fix some specific bugs (false positives) reported by PHPStan feel free to do so in a smaller focused PR. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants