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

pecl_http stubs appear to have wrong namespace qualification #5460

Closed
asherkin opened this issue Aug 12, 2021 · 3 comments
Closed

pecl_http stubs appear to have wrong namespace qualification #5460

asherkin opened this issue Aug 12, 2021 · 3 comments
Labels
Milestone

Comments

@asherkin
Copy link
Contributor

Bug report

I think this is an error on the phpstorm-stubs side, but that is mostly a guess after poking around a bit. If PHPStan is using the latest stubs I suspect the cause was JetBrains/phpstorm-stubs#1075, but I haven't been able to work out how to override the stub files in PHPStan - just downloading and modifying http3.php and including it in stubFiles doesn't appear to work.

PhpStorm itself seems happy with most of these (including those below) so I'm not sure if that should be fixed on the PHPStan side or in phpstorm-stubs, but PhpStorm can't resolve at least the ones in the Parser class (which makes sense as those are in a nested namespace) - so at least part of the fix seems to be on the stubs side.

I made a change to re-fully-qualify all the namespaces and PHPStan was certainly happier parsing it as a stub file, but without the ability to run the upstream tests (no Docker), being able to confirm it fixed the erroneous errors, or being sure it wouldn't be overwritten by the next attempt to unify the code style (if that PR was the cause), I didn't feel comfortable submitting a PR directly :)

JetBrains/phpstorm-stubs@master...asherkin:master

Code snippet that reproduces the problem

https://phpstan.org/r/885974be-633c-423c-8ea4-6b4917b88229

pecl_http is of course fairly dead and I will probably aim to just delete the code using it (it is behind runtime checks and shouldn't be live, PHPStan is of course dutifully analysing it anyway though).

Expected output

I wouldn't expect any errors from this code.

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

PHPStan helps me every day, at the moment it is helping me clean up 15 years of accumulated cruft.

@ondrejmirtes
Copy link
Member

The stub in PhpStorm is correct. At first sight looks like it isn't: https://github.com/JetBrains/phpstorm-stubs/blob/97228cfffca0fe530d7a13a0dc01d9b2c5ccffee/http/http3.php#L197

But there's use http; at the top.

The bug will probably be somewhere in https://github.com/ondrejmirtes/BetterReflection/blob/master/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php. The class generates the final inspected source code for each function and class stubs. It omits any uses at the top of the file, because PHP-Parser's NameResolver makes all the Names in source code fully qualified. But something's probably wrong with that assumption.

@ondrejmirtes ondrejmirtes added this to the Easy fixes milestone Aug 15, 2021
@phpstan-bot
Copy link
Contributor

@asherkin After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
- 9: Parameter #1 $request of method http\Client::enqueue() expects http\http\Client\Request, http\Client\Request given.
-11: Parameter #1 $request of method http\Client::getResponse() expects http\http\Client\Request|null, http\Client\Request given.
+No errors

MidnightDesign pushed a commit to MidnightDesign/phpstan-src that referenced this issue Nov 30, 2021
@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 Dec 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants