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

parse_str() might return arrays by-ref #1994

Merged
merged 3 commits into from Dec 15, 2022
Merged

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Nov 13, 2022

@staabm staabm force-pushed the bug8356 branch 2 times, most recently from 563e438 to d0eb162 Compare November 13, 2022 19:43
Copy link
Contributor Author

@staabm staabm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: the issue-bot did not realize a change in the origin issue snippet, even though the testsuite shows that the PR will change it

$this->bleedingEdge = true;
$this->analyse([__DIR__ . '/data/bug-8356.php'], [
[
"Offset 'x' might not exist on array|string.",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before this PR, the rule reported
Offset 'x' does not exist on array|string.
.. now we get ..
Offset 'x' might not exist on array|string.

when running this test without $this->bleedingEdge = true; the signature changes in the stub do not have any effect and we still get Offset 'x' does not exist on array|string.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bot only goes through triaged issues labeled as bug or feature-request.

@@ -70,10 +70,16 @@ function bin2hex(string $string): string {}

/**
* @param array<mixed> $result
* @param-out array<int|string, string|list<string>> $result
* @param-out array<int|string, string|list<string>|array<mixed>> $result
*/
function parse_str(string $string, array &$result): void {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved parse_str and mb_parse_str next to each other, and equalized the type signatures

@staabm staabm marked this pull request as ready for review November 13, 2022 19:54
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

stubs/core.stub Outdated Show resolved Hide resolved
stubs/core.stub Outdated Show resolved Hide resolved
@staabm staabm force-pushed the bug8356 branch 3 times, most recently from af7655a to 70e3d75 Compare November 24, 2022 16:50
@staabm staabm force-pushed the bug8356 branch 2 times, most recently from ece2980 to 399ce72 Compare December 2, 2022 10:20
@ondrejmirtes
Copy link
Member

Thank you!

@staabm staabm deleted the bug8356 branch December 15, 2022 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants