-
Notifications
You must be signed in to change notification settings - Fork 548
Feature/disallow dynamic property option #1234
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
Feature/disallow dynamic property option #1234
Conversation
|
I'll add some tests for the feature. |
|
It might be better to make this as an option for |
|
I worry that this logic needs to be deep in NodeScopeResolver, or not? If not then yeah, parameter for a rule is nicer. |
1ab1af1 to
e64bc9a
Compare
I think it's possible now. |
|
While this option can optionally restore the behavior before #1223 (comment), it would restore the false positive phpstan/phpstan#3171 too. |
Should be fixed by specifying types in isset. Related issue phpstan/phpstan#3171
|
Thank you! Can you please send an update to https://phpstan.org/config-reference#stricter-analysis as well? There's an "Edit this page" link at the bottom :) I'll update phpstan-strict-rules so that the playground with strict rules + bleedingEdge enabled will have this option on. |
|
I'm testing this in the real world and it's really nicely done! The impact is really minimal and exactly in the right weird places where I'd expect it 👍 |
|
Thanks! I'm glad that I could help solving dynamic properties related issues 😄 |
Implemented bool option in
NodeScopeResolveras mentioned in #1223 (comment) to optionally restore the behavior before #1223.Strictly, it is not exactly the same behavior as before because it disallows dynamic properties in
issetandemptytoo, but I believe making them consistent is better.Also, fixes phpstan/phpstan#3171 because it's related.