Skip to content

Allow Redis::pconnect to have 7 parameters - #2695

Merged
ondrejmirtes merged 1 commit into
phpstan:1.10.xfrom
Vaalyn:fix-redis-pconnect-function-signature
Oct 30, 2023
Merged

Allow Redis::pconnect to have 7 parameters#2695
ondrejmirtes merged 1 commit into
phpstan:1.10.xfrom
Vaalyn:fix-redis-pconnect-function-signature

Conversation

@Vaalyn

@Vaalyn Vaalyn commented Oct 24, 2023

Copy link
Copy Markdown
Contributor

This PR fixes a similar issue that was reported in phpstan/phpstan#9413 and subsequently fixed in #2442.

The Redis::pconnect method also accepts 7 parameters like the Redis::connect method, currently using all 7 available parameters on the pconnect method will show the error:

Method Redis::pconnect() invoked with 7 parameters, 1-5 required.

As can be seen here on the redis.stub.php the signature in the PHPStan function map is missing 2 parameters:

public function pconnect(string $host, int $port = 6379, float $timeout = 0, ?string $persistent_id = null, int $retry_interval = 0, float $read_timeout = 0, ?array $context = null): bool;

I've updated the function map to match the method signature.

@phpstan-bot

Copy link
Copy Markdown
Collaborator

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

@ondrejmirtes

Copy link
Copy Markdown
Member

I will merge this if you rebase it on top of 1.10.x.

@Vaalyn
Vaalyn force-pushed the fix-redis-pconnect-function-signature branch from 1218ecd to 64a0444 Compare October 29, 2023 12:21
@Vaalyn

Vaalyn commented Oct 29, 2023

Copy link
Copy Markdown
Contributor Author

@ondrejmirtes 👍 done

@ondrejmirtes
ondrejmirtes changed the base branch from 1.11.x to 1.10.x October 30, 2023 12:28
@ondrejmirtes
ondrejmirtes merged commit 4a17328 into phpstan:1.10.x Oct 30, 2023
@ondrejmirtes

Copy link
Copy Markdown
Member

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