Sitelet https://github.com/phpredis/phpredis/pull/2737
Skip to content

feat: Add method annotations for methods that are callable via magic __call method - #2737

Merged
michael-grunder merged 4 commits into
phpredis:developfrom
mitelg:fix/redis-array-stub
Oct 31, 2025
Merged

michael-grunder merged 4 commits into
phpredis:developfrom
mitelg:fix/redis-array-stub

Conversation

@mitelg

@mitelg mitelg commented Oct 23, 2025 •

Copy link
Copy Markdown
Contributor

The RedisArray class is supporting the same method as the base Redis class via the magic __call method.
To enable auto-completion and fixing static analyzer issues, I added @method annotations for all the methods that are not explicitly implemented.

Also see discussion here: #2736

The class PHPdoc was generated with this script:
<?php

declare(strict_types=1);

$redisStubPath = __DIR__ . '/redis.stub.php';
$redisStubContent = file_get_contents($redisStubPath);
preg_match_all('/public function (?<methodNames>[a-zA-Z].*)(?<parameters>\(.*\)):? ?(?<returnTypes>.*)?;/', $redisStubContent, $matches);
$methodNames = $matches['methodNames'];
$parameters = $matches['parameters'];
$returnTypes = $matches['returnTypes'];

$combinedMethods = [];
foreach ($methodNames as $key => $methodName) {
    $combinedMethods[$methodName] = [
        'parameters' => $parameters[$key],
        'returnType' => $returnTypes[$key],
    ];
}

$redisArrayStubPath = __DIR__ . '/redis_array.stub.php';
$redisArrayStubContent = file_get_contents($redisArrayStubPath);
preg_match_all('/public function (?<methodNames>[a-zA-Z].*)\(.*\)(: [a-zA-Z].*)?;/', $redisArrayStubContent, $arrayMatches);
$arrayMethodNames = $arrayMatches['methodNames'];

$arrayIndexedByMethodName = array_flip($arrayMethodNames);

$result = array_diff_key($combinedMethods, $arrayIndexedByMethodName);

$classComment = "/**\n";
foreach ($result as $methodName => $parametersAndReturnType) {
    $returnType = $parametersAndReturnType['returnType'] ? $parametersAndReturnType['returnType'] . ' ' : '';
    $classComment .= sprintf(" * @method %s%s%s\n", $returnType, $methodName, $parametersAndReturnType['parameters']);
}
$classComment .= " */";

print_r($classComment);

Executed in the root directory of this project.

To make this script work I adjusted some method definitions in the Redis and RedisArray stub. Hope this changes are okay.

Finally I fixed some PHPDoc comments in the stubs as they were not aligned with the actual types.

@mitelg
mitelg force-pushed the fix/redis-array-stub branch from 149e1ed to 7557625 Compare October 23, 2025 12:54
@mitelg
mitelg marked this pull request as draft October 23, 2025 13:22
@mitelg
mitelg force-pushed the fix/redis-array-stub branch from 7557625 to 69cb7c5 Compare October 23, 2025 13:24
@mitelg
mitelg marked this pull request as ready for review October 23, 2025 14:23
@michael-grunder

Copy link
Copy Markdown
Member

This is good. There are just a couple of general issues that need to be tweaked.

The *.stub.php files are run through PHP's gen_stub.php helper. This helper script actually generates all of the *arginfo.h C header files.

I don't know why, but gen_stub.php will reject formulations like this:

@param string ...$keys

And instead wants:

@param string $keys...

I'm not sure what system you're using, but gen_stub.php is bundled with PHP.

# On a macOS where PHP is installed with brew
find $(brew --prefix php@8.4)/ -name 'gen_stub.php'
/opt/homebrew/opt/php/lib/php/build/gen_stub.php

# On Linux
find $(php-config --extension-dir)/ -name 'gen_stub.php'
/usr/lib/php/20240924/build/gen_stub.php

And to run PHP 8.4's gen_stub.php against the PhpRedis stubs, you cd into phpredis and do:

# Assuming you're on a mac. Use the php-config formulation otherwise
php $(find $(brew --prefix php@8.4)/ -name 'gen_stub.php') *.stub.php
In redis_cluster.stub.php:
RedisCluster::info(): @param doesn't contain a variable name or has an invalid format "string       ...$sections    Optional section(s) you wish Redis server to return."

Note that each major.minor version of PHP updates gen_stub.php so you'll want to use PHP 8.4's.

I was able to work through the warnings pretty quickly. It was literally just making sure the ... came after the variable name.

gen_stub_fixup.patch

The only other issue is that we can't change anything that isn't in a comment. Even changing flushdb to flushDB will modify C code. Changing casing is probably not going to cause an issue but we only change the C parts of the arginfo files on major version bumps. One exception to this is pure additions, which are fine.

cc @yatsukhnenko I'm fine with php-doc @method annotations for RedisArray if it helps linting.

@michael-grunder michael-grunder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just what I mentioned in the previous comment.

@mitelg
mitelg force-pushed the fix/redis-array-stub branch from 7f6c917 to 0579686 Compare October 24, 2025 07:15
@mitelg

mitelg commented Oct 24, 2025

Copy link
Copy Markdown
Contributor Author

hey @michael-grunder

thanks for your feedback 👍

I reverted the method casing in RedisArray and applied your patch, as I couldn't find any gen_stub.php script on my machine 🙈

@yatsukhnenko

Copy link
Copy Markdown
Member

I couldn't find any gen_stub.php script on my machine 🙈

@mitelg try to run phpize command, script should appear in build directory

@mitelg
mitelg force-pushed the fix/redis-array-stub branch from 0579686 to 8db2abf Compare October 24, 2025 08:43
@mitelg

mitelg commented Oct 24, 2025

Copy link
Copy Markdown
Contributor Author

hey @yatsukhnenko @michael-grunder

I reverted the changes at the variadic parameters, as imo this is the wrong syntax writing it after the variable name.
I raised an issue on php-src repo: php/php-src#20277

@mitelg

mitelg commented Oct 24, 2025

Copy link
Copy Markdown
Contributor Author

I then executed the gen_stub.php with PHP 8.5. Hope this was okay

@michael-grunder

Copy link
Copy Markdown
Member

I reverted the changes at the variadic parameters, as imo this is the wrong syntax writing it after the variable name.

That makes sense to me. It would be nice to be able to use consistent syntax both in the phpdoc and the actual stub prototypes. I assume the gen_stub.php issue is just about parsing because modifying the @param type annotations only changes the SHA256 hash, not any of the generated C code.

Comment thread redis_array.stub.php Outdated
@mitelg

mitelg commented Oct 24, 2025

Copy link
Copy Markdown
Contributor Author

@michael-grunder unfortunately I have no knowledge about PHP internals 🙈 so I hope, it will be fixed with the issue I created.

@yatsukhnenko done!

@michael-grunder

Copy link
Copy Markdown
Member

Looks really good. I'll go over it this weekend and then get it merged.

We're just about to release 6.3.0 GA so we may get this into 6.3.1 which will almost certainly come pretty quick after 6.3.0

@mitelg

mitelg commented Oct 30, 2025

Copy link
Copy Markdown
Contributor Author

FYI: I created a PR to support variadic syntax in the PHPDoc php/php-src#20342

@mitelg
mitelg force-pushed the fix/redis-array-stub branch from c6a310f to 2281dfa Compare October 31, 2025 07:37
@mitelg
mitelg force-pushed the fix/redis-array-stub branch from 2281dfa to 175f173 Compare October 31, 2025 07:40
@mitelg

mitelg commented Oct 31, 2025

Copy link
Copy Markdown
Contributor Author

I am unsure now, do I still need to execute gen_stub.php, although only the hash would change? 🤔

@michael-grunder

Copy link
Copy Markdown
Member

It doesn't make any functional difference so you don't need to.

I always do because I added a .git hook to tell me when they're out of sync 😄

@michael-grunder michael-grunder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Completely safe to merge as well, since this doesn't change anything but the arginfo hashes

@michael-grunder
michael-grunder merged commit b742bb8 into phpredis:develop Oct 31, 2025
39 checks passed
@mitelg
mitelg deleted the fix/redis-array-stub branch November 3, 2025 07:25
@mitelg

mitelg commented Nov 3, 2025

Copy link
Copy Markdown
Contributor Author

thanks for merging!

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