get_defined_vars() return type contains know variables #3624
Conversation
MartinMystikJonas force-pushed the get-defined-vars branch 4 times, most recently from 0bd65db to 5eea5cb Compare 
MartinMystikJonas force-pushed the get-defined-vars branch from 5eea5cb to 8b88081 Compare MartinMystikJonas force-pushed the get-defined-vars branch from 8b88081 to e9ee834 Compare MartinMystikJonas force-pushed the get-defined-vars branch from e9ee834 to 4b62aaa Compare | assertType('array{param: int, local: \'foo\'}', get_defined_vars()); | ||
| assertType('array{\'param\', \'local\'}', array_keys(get_defined_vars())); |
There was a problem hiding this comment.
When escaping single quotes, I always (when possible) switch to double quoted string to prevent that.
- easier on the eyes
- easier to copy and paste
- easier to search for the unescaped string in the codebase
| assertType('array{param: int, local: \'foo\'}', get_defined_vars()); | |
| assertType('array{\'param\', \'local\'}', array_keys(get_defined_vars())); | |
| assertType("array{param: int, local: 'foo'}", get_defined_vars()); | |
| assertType("array{'param', 'local'}", array_keys(get_defined_vars())); |
There was a problem hiding this comment.
I do that too but I checked how it is done in other parts of PHPStan and found out that escaping seems to be standard here.
There was a problem hiding this comment.
I also often do the double quote. but I think in general it's very inconsistent :D not sure if such thing exists, but a coding standard rule might be good for this 😊
UPDATE: on the other hand - in those tests files it wouldn't be enforced anyway I guess..
There was a problem hiding this comment.
| (the unrelated lint build errors will be fixed after rebase; please remove the merge commit) |
MartinMystikJonas force-pushed the get-defined-vars branch 2 times, most recently from 6eed675 to 1f7ebbe Compare | @staabm Removed merge commit. Should be ready to merge - two failing checks are unrelated. |
| the "Lint / Coding Standard (pull_request) " fail is related to the PR. just run |
MartinMystikJonas force-pushed the get-defined-vars branch from 1f7ebbe to ddba1cd Compare | CS fixed |
| $maybeIndexes = range(count($definedVariables), count($definedVariables) + count($maybeVariables)); | ||
| } | ||
| | ||
| return new ConstantArrayType($keys, $values, [0], $maybeIndexes); |
There was a problem hiding this comment.
Use ConstantArrayTypeBuilder instead.
There was a problem hiding this comment.
Rewritten
How to do it properly? Can I just change target branch? |
MartinMystikJonas force-pushed the get-defined-vars branch from ddba1cd to 2110e67 Compare
try locally running a rebase against 1.12.x. if it works cleanly just force push it and change the target branch of the PR |
MartinMystikJonas force-pushed the get-defined-vars branch from 2110e67 to 7a7c05c Compare 
Co-Authored-By: Ruud Kamphuis <ruudk@users.noreply.github.com>
MartinMystikJonas force-pushed the get-defined-vars branch from 7a7c05c to a503e9b Compare | Well for some reason (maby I messed up) I was a bit more complicated but it seems it is ok now. I will see if check find any issue and fix it eventualy. |
| Ok it should be ready for merge into 1.12.x |
| Perfect, thank you! |
5 participants
Solves phpstan/phpstan#11948
Solves phpstan/phpstan#11740
Credits to @ruudk