Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsolvable Redis::connect override issue #8733

Closed
greg0ire opened this issue Jan 18, 2023 · 12 comments · Fixed by phpstan/phpstan-src#2187 or phpstan/phpstan-src#2188
Closed

Unsolvable Redis::connect override issue #8733

greg0ire opened this issue Jan 18, 2023 · 12 comments · Fixed by phpstan/phpstan-src#2187 or phpstan/phpstan-src#2188
Labels
Milestone

Comments

@greg0ire
Copy link

Bug report

We have a class that extends \Redis for instrumentation purposes.

Code snippet that reproduces the problem

Old code, that JetBrains/phpstorm-stubs#1436 seems to break:
https://phpstan.org/r/8e46f0df-101e-4d9b-b114-f2cd35af8556

New code that still has an error
https://phpstan.org/r/5dd2a695-1065-4111-be9d-1ded759214d6

Expected output

No errors with the new code

Did PHPStan help you today? Did it make you happy in any way?

Yes :)

@ondrejmirtes ondrejmirtes added this to the Easy fixes milestone Jan 18, 2023
@greg0ire
Copy link
Author

I can give it a try.

@phpstan-bot
Copy link
Contributor

@greg0ire After the latest push in 1.9.x, PHPStan now reports different result with your code snippet:

@@ @@
-33: Parameter #4 $reserved (null) of method Manomano\Observability\Instrumentation\Redis\ObservableClient::connect() should be compatible with parameter $persistent_id (string) of method Redis::connect()
+33: Parameter #4 $reserved (null) of method Manomano\Observability\Instrumentation\Redis\ObservableClient::connect() should be compatible with parameter $persistent_id (string) of method Redis::connect()
+36: Parameter #4 $persistent_id of method Redis::connect() expects string, null given.
Full report
Line Error
33 Parameter #4 $reserved (null) of method Manomano\Observability\Instrumentation\Redis\ObservableClient::connect() should be compatible with parameter $persistent_id (string) of method Redis::connect()
36 Parameter #4 $persistent_id of method Redis::connect() expects string, null given.

@phpstan-bot
Copy link
Contributor

@greg0ire After the latest push in 1.9.x, PHPStan now reports different result with your code snippet:

@@ @@
-36: Parameter #4 $persistent_id of method Redis::connect() expects null, string|null given.
+36: Parameter #4 $persistent_id of method Redis::connect() expects string, string|null given.
Full report
Line Error
36 `Parameter #4 $persistent_id of method Redis::connect() expects string, string

@phpstan-bot
Copy link
Contributor

@greg0ire After the latest push in 1.10.x, PHPStan now reports different result with your code snippet:

@@ @@
-33: Parameter #4 $reserved (null) of method Manomano\Observability\Instrumentation\Redis\ObservableClient::connect() should be compatible with parameter $persistent_id (string) of method Redis::connect()
+33: Parameter #4 $reserved (null) of method Manomano\Observability\Instrumentation\Redis\ObservableClient::connect() should be compatible with parameter $persistent_id (string) of method Redis::connect()
+36: Parameter #4 $persistent_id of method Redis::connect() expects string, null given.
Full report
Line Error
33 Parameter #4 $reserved (null) of method Manomano\Observability\Instrumentation\Redis\ObservableClient::connect() should be compatible with parameter $persistent_id (string) of method Redis::connect()
36 Parameter #4 $persistent_id of method Redis::connect() expects string, null given.

@phpstan-bot
Copy link
Contributor

@greg0ire After the latest push in 1.10.x, PHPStan now reports different result with your code snippet:

@@ @@
-36: Parameter #4 $persistent_id of method Redis::connect() expects null, string|null given.
+36: Parameter #4 $persistent_id of method Redis::connect() expects string, string|null given.
Full report
Line Error
36 `Parameter #4 $persistent_id of method Redis::connect() expects string, string

@ondrejmirtes
Copy link
Member

Looks like you're gonna have to give it one more try 😂

@greg0ire
Copy link
Author

Some of these are legit I think, the first snippet is still supposed to be invalid.

@greg0ire
Copy link
Author

🤔 but yeah, null should probably be allowed here, and I think https://github.com/JetBrains/phpstorm-stubs/pull/1436/files should have used string|null as well (right?)

@phpstan-bot
Copy link
Contributor

@greg0ire After the latest push in 1.9.x, PHPStan now reports different result with your code snippet:

@@ @@
-36: Parameter #4 $persistent_id of method Redis::connect() expects null, string|null given.
+No errors

@phpstan-bot
Copy link
Contributor

@greg0ire After the latest push in 1.10.x, PHPStan now reports different result with your code snippet:

@@ @@
-36: Parameter #4 $persistent_id of method Redis::connect() expects null, string|null given.
+No errors

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
3 participants