Improve expression resolving of superglobals #2012
Conversation
herndlm
force-pushed
the
improve-superglobal-expression-resolving
branch
4 times, most recently
from
c3dd5c1
to
b54aeb7
Compare
|
Still not a 100% happy with how the superglobals are created and set, but at least it seems to be working fine now. |
herndlm
force-pushed
the
improve-superglobal-expression-resolving
branch
from
b54aeb7
to
98f40eb
Compare
|
This pull request has been marked as ready for review. |
herndlm
force-pushed
the
improve-superglobal-expression-resolving
branch
from
98f40eb
to
d784b9b
Compare
herndlm
force-pushed
the
improve-superglobal-expression-resolving
branch
4 times, most recently
from
ac80adc
to
8ba50ff
Compare
herndlm
force-pushed
the
improve-superglobal-expression-resolving
branch
2 times, most recently
from
8c5fe5c
to
77f6465
Compare
|
This pull request has been marked as ready for review. |
herndlm
force-pushed
the
improve-superglobal-expression-resolving
branch
from
77f6465
to
e3a8fcb
Compare
herndlm
force-pushed
the
improve-superglobal-expression-resolving
branch
from
e3a8fcb
to
51ddfcc
Compare
|
This pull request has been marked as ready for review. |
herndlm
force-pushed
the
improve-superglobal-expression-resolving
branch
from
51ddfcc
to
5c856d7
Compare
|
rector error again WHY? I only rebased on 1.9.x and removed a commit that added a test which is not needed anymore... :/ |
herndlm
force-pushed
the
improve-superglobal-expression-resolving
branch
5 times, most recently
from
3562d82
to
cb75108
Compare
who knows, maybe this changes something for rector..
herndlm
force-pushed
the
improve-superglobal-expression-resolving
branch
from
cb75108
to
e541807
Compare
|
Thank you! |
|
On, tbh I didn't expect that this one would be merged without changes 😊 I think I duplicated the list of superglobals somewhere. Are you fine with that or should I put it somewhere central? Maybe I didn't because it would have needed a public static method or so and felt weird 🤔 |
|
When you open a pull request, there's always a risk I'm gonna merge it :) Feel free to improve your code in follow-up PRs. |
|
Saw that it was in MutatingScope, but I believe that it's fine to not deduplicate it. So, nevermind and sorry for the noise |
|
Sadly I have to revert this because of performance reasons. Blackfire comparison on top of 1.10.x-dev: https://blackfire.io/profiles/compare/21309b29-d2de-4f60-b6fe-1873b47e31b4/graph |
|
oh that's unfortunate. just for my understanding - how did you find out that it was this commit that caused it in the performance graph? or was it just a lucky guess? |
|
Yesterday I saw a million calls to ArrayType::equals() which looked weird to me. Today I looked at recent releases and started git bisect, and the results made my suspicion obvious 😊 |
is this a blackfire run over |
|
Yes |
Handles superglobals as any other expressions, which fixes narrowing them.
Closes phpstan/phpstan#5805 (which I was totally not expecting, most likely unlocked also by recent changes from @rajyan)nope, not anymore, this was fixed in #2030, I do understand that this change then is only improving things if people use superglobal expressions to narrow types, which might not be the case often in modern apps. Therefore I also get it if this is not wanted in the scope :)The superglobals creation is most likely also a good integration point for phpstan/phpstan#8037