-
Notifications
You must be signed in to change notification settings - Fork 132
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
Always require PHP version for PhpStormStubsSourceStubber
- PHP_VERSION_ID
as default
#874
Conversation
PhpStormStubsSourceStubber
- PHP_VERSION_ID
as defaultPhpStormStubsSourceStubber
- PHP_VERSION_ID
as default
@@ -33,6 +33,8 @@ | |||
use function str_replace; | |||
use function strtolower; | |||
|
|||
use const PHP_VERSION_ID; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this uses "the PHP version BR is running under", rather than "the PHP version of the code being analysed" as the default? What is the mechanism for overriding this? e.g. if we are using PHP 8.0 to analyse a codebase that runs on PHP 7.4?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can always specify the version by yourself: 419c1a7#diff-462651b2de3f83cc239bebc471d6800cebf5893e7258569982934b031c8c422eR169
Ok, tests failed because |
…e able to reflect interfaces that exist only in PHP 8.1 stubs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - constructor used to define PHP version is also more than acceptable.
Thanks for keeping an eye out for the upcoming stubs patch, @kukulich! |
It's necessary for stubs like: https://github.com/JetBrains/phpstorm-stubs/blob/master/bcmath/bcmath.php#L198-L201
We have to know the version to choose the right parameter.