Skip to content

Fix false positives on non-existing-offsets of super-globals #3871

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

Merged
merged 3 commits into from
Mar 11, 2025

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Mar 11, 2025

see https://phpstan.org/r/1eb6a683-733c-486e-ab5a-e38f2d573093

this PR effectively allows to narrow the array type of a super-global variable in scope

@staabm staabm marked this pull request as ready for review March 11, 2025 11:17
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes ondrejmirtes merged commit c8833df into phpstan:2.1.x Mar 11, 2025
415 of 417 checks passed
@ondrejmirtes
Copy link
Member

Thank you!

@herndlm
Copy link
Contributor

herndlm commented Mar 11, 2025

oh cool, this seems to be a simpler version of #3762 which I can close now :)

@staabm
Copy link
Contributor Author

staabm commented Mar 11, 2025

Ohh.. this wasn't intentional 😅

@herndlm
Copy link
Contributor

herndlm commented Mar 11, 2025

that's ok, this solution is simpler / more elegant :)

@herndlm
Copy link
Contributor

herndlm commented Mar 24, 2025

Hi, I remember there was a weird issue when I tried to use my prepared superglobals tests here: https://github.com/phpstan/phpstan-src/pull/3762/files#diff-51a54cb478a71bc6cb00a1276e9df23606258635fe80bfff8843430cc254505c

basically, the truthy/falsey scope bled into the null one. I should have debugged this more.

I believe that is related to this change here causing issues like phpstan/phpstan#12771 and the ones linked to it. @staabm are you available and want to / can look into this? If not I can try later too

@herndlm
Copy link
Contributor

herndlm commented Mar 24, 2025

hmm, looks like the issue is fixed when I set the expression directly as in https://github.com/phpstan/phpstan-src/pull/3762/files#diff-dc817f2bab8672057a95375a542e68599164f6ab2380e4cccd1b50583e295d85R562-R564, which is kind of the only core difference to this PR.

but I'm not a 100% sure how it even worked before tbh. and if I propose the expression fix, then I could also propose the rest I'm doing in that MR (taking over the expressions when entering functions, classes, namespace)? not sure. an opinion from @ondrejmirtes would be interesting before I prepare anything :)

@ondrejmirtes
Copy link
Member

This introduced a regression: https://phpstan.org/r/86811bd5-067b-476e-958a-bf621ad773c5

Please look into it, thanks.

@staabm
Copy link
Contributor Author

staabm commented Mar 24, 2025

This introduced a regression: phpstan.org/r/86811bd5-067b-476e-958a-bf621ad773c5

for posterity: fixed via #3901

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants