-
Notifications
You must be signed in to change notification settings - Fork 650
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
Update Call Maps and Signature for get_headers #9073
Changes from 2 commits
0d36041
e7b5c99
cb204ed
ec2f93f
e4e52da
0ba8211
bf62ef8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1482,11 +1482,11 @@ function bin2hex(string $string): string {} | |
* | ||
* @param resource|null $context | ||
* | ||
* @return ($associative is 0 ? list<string> : array<string, string|list<string>>)|false | ||
* @return ($associative is false ? list<string> : array<string, string|list<string>>)|false | ||
* | ||
* @psalm-taint-sink ssrf $url | ||
*/ | ||
function get_headers(string $url, int $associative = 0, $context = null) : array|false {} | ||
function get_headers(string $url, bool $associative = false, $context = null) : array|false {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will be harder to handle because we don't have conditional params so you can't really have different types for different versions. Could you try to leave this stub as it is here and try to duplicate it in PHP80.phpstub ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems to work with my local testing... I test my example file in the bug report with php ../psalm --php-version=7.4.0 --no-cache and php ../psalm --php-version=8.0.0--no-cache with both int and bool and correct correct results for each version. I am happy to also add a test case for this, i am uncertain where in the tests that should live... If you can provide some guidance I am to add that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can add your tests on FunctionCallTest::providerValidCodeParse :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the pointer. Test are added. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wan you add the expected return type in the assertion array part of the test? In PHP 7 and 8, you should get slightly different types, this should help us see if the return behaves correctly too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assertions are added. |
||
|
||
/** | ||
* @psalm-pure | ||
|
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.
The change is needed, but the stubs are valid for each php version so this need to stay compatible with old versions.
Can you add handling for both false and 0?
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.
This is completed.