-
Notifications
You must be signed in to change notification settings - Fork 533
[FIX] dynamic variable undefined inside isset
#4213
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
Conversation
fd4fefe
to
8b91913
Compare
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.
Please look at this problem more thoroughly. There's IssetCheck (
phpstan-src/src/Rules/IssetCheck.php
Line 45 in 5a39902
db07673
to
7733eb3
Compare
Please look at this problem more thoroughly. There's IssetCheck (
phpstan-src/src/Rules/IssetCheck.php
Line 45 in 5a39902
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {) used by several rules where only Variable with string name property is being handled, but the code might find some improvements to the logic useful.
It was my first idea to fix the problem in the IssetCheck, however, the error being returned actually comes from DefinedVariableRule. Being new to the project, I don't yet understand how it all works. However, if you could tell me a little more about what you'd like to see added, there's no problem; I'm open to any suggestions.
I've added the tests as requested.
IssetCheck is used by IssetRule and others. Look at IssetRuleTest what it detects.
You should be able to produce some tests with $$
to see what's currently wrong about the behaviour.
IssetCheck is used by IssetRule and others. Look at IssetRuleTest what it detects.
You should be able to produce some tests with
$$
to see what's currently wrong about the behaviour.
As you can see, I added the tests in IssetRuleTest.php, but nothing is thrown.
Maybe I missed something, feel free to let me know.
492f2d1
to
4c8527b
Compare
Fix phpstan/phpstan#13353
This is the fix for the issue above. If you see any changes to make, please let me know; this is my first contribution to the project.