Skip to content

Stubs multi handling and more specific types#2799

Open
kkmuffme wants to merge 3 commits into
phpredis:developfrom
kkmuffme:stubs-multi-handling
Open

Stubs multi handling and more specific types#2799
kkmuffme wants to merge 3 commits into
phpredis:developfrom
kkmuffme:stubs-multi-handling

Conversation

@kkmuffme

Copy link
Copy Markdown

Follow-up to previous PRs in psalm and discussions in phpredis for stubs.

Initial setup and key methods types were fixed manually, cross-referencing the .c source code, others were bulk replaced due to the sheer number of methods.

There are still further improvements possible to make some types more specific (e.g. some int/array types can be changed to int<0, max> or string[]), which I might add at a later point.

The reason string[] was used instead of array<string, string> even though that would be what usually be expected is that PHP will automatically cast int-like numeric array keys to int

I PRed it here since instead of having to maintain everything twice (= in phpredis + in psalm) it saves me a lot of time to only do it once.

Fix phpstan/phpstan#7745 and various similar ones

This commit is just a proof-of-concept for the necessary changes to allow getting a quick overview of the proposed changes, which will be implemented in the next commits
@michael-grunder

Copy link
Copy Markdown
Member

Looks good. I'll need to regenerate the _arginfo.h files and see how they change.

We're restricted in what we can change except in a major version bump. I'm not actually sure what Redis -> self will generate.

@kkmuffme

Copy link
Copy Markdown
Author

We're restricted in what we can change except in a major version bump

In terms of phpredis itself I hope nothing changed (can't guarantee though, bc it's just so many, but if any change/issue I'll fix it)

I'm not actually sure what Redis -> self will generate.

If it's an issue, I'll just keep it in phpdoc only - however technically it's at least misleading since one assumes that Redis is actually a new instance, while in fact it is not.

We're using these internally in the next days, I'll update it if we encounter any issues and/or I have time to make some additional improvements.

@michael-grunder

Copy link
Copy Markdown
Member

Interesting it looks like gen_stub.php doesn't like self.

Redis::append(): @return doesn't contain a type or has an invalid format "(TMULTI is int ? self<TMULTI> : int|false) The new string length of the key or false on failure."

You can build these locally by doiing this:

cd /path/to/phpredis
phpize
php build/gen_stub.php
# You could also chmod +x build/gen_stub.php && build/gen_stub.php

@kkmuffme

Copy link
Copy Markdown
Author

That's because of the conditional return type though, not bc of "self" is it?

@michael-grunder

michael-grunder commented Jan 27, 2026

Copy link
Copy Markdown
Member

Sorry you're right I didn't look too closely. Just saw that gen_stub.php kicked it back.

Edit: It doesn't like self either. The non-commented part of the stubs have finicky rules.

In ext/redis/redis.stub.php:
Redis::append(): The exact class name must be used instead of "self"

@kkmuffme kkmuffme force-pushed the stubs-multi-handling branch from 235eb8d to 34f362f Compare January 27, 2026 20:44
@kkmuffme

Copy link
Copy Markdown
Author

Edit: It doesn't like self either. The non-commented part of the stubs have finicky rules.

Weird, bc "self" is a native PHP type hint since PHP 7.0.

Anyway: given that the conditional return types aren't supported either, I suggest that I:

  • change the type hints back to Redis but add a @return (where there's none yet) with the same type as the hint except using self instead of Redis
  • change the @return to a @psalm-return where a conditional return type is used

Let me know if that sounds good, then I'll take care of it tomorrow morning so this can get merged

@michael-grunder

Copy link
Copy Markdown
Member

Weird, bc "self" is a native PHP type hint since PHP 7.0.

Yeah the gen_stub.php mechanism is nice but it kind of lives halfway between PHP and C. The purpose of the function prototypes in *.stub.php is to automatically generate the C style *_arginfo.h files.

What you can do to confirm that the stubs parse is something like this:

cd phpredis
phpize && php build/gen_stub.php

Ideally where your system-wide version of php is 8.5 as gen_stub.php is bundled with each major.minor version.

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.

PhpRedis methods don't handle multi() mode correctly

2 participants