-
-
Notifications
You must be signed in to change notification settings - Fork 429
[DeadCode] Fix: Array Duplicated Key which is dynamic #6999
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
[DeadCode] Fix: Array Duplicated Key which is dynamic #6999
Conversation
|
Thank you Peter 🙂 |
| if ($arrayItem->key instanceof CallLike) { | ||
| continue; | ||
| } | ||
|
|
||
| if ($arrayItem->key instanceof PropertyFetch) { | ||
| continue; | ||
| } |
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.
this should use ExprAnalyzer->isDynamicExpr()
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.
I created PR for it, with additional fixture test:
|
|
||
| namespace Rector\Tests\DeadCode\Rector\Array_\RemoveDuplicatedArrayKeyRector\Fixture; | ||
|
|
||
| class SkipMethodAndFunctCalls |
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.
| class SkipMethodAndFunctCalls | |
| class SkipPropertyFetch |
|
|
||
| namespace Rector\Tests\DeadCode\Rector\Array_\RemoveDuplicatedArrayKeyRector\Fixture; | ||
|
|
||
| class SkipMethodAndFunctCalls |
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.
| class SkipMethodAndFunctCalls | |
| class SkipCallLikeKeys |
|
This pull request has been automatically locked because it has been closed for 150 days. Please open a new PR if you want to continue the work. |
Changes
Why
I found this dead code rule could end up removing array keys if the key was set by a method call, function call or a property fetch that might be dynamic. There might be a way to allow these options back by using PHPStan to look for constant values returned by the dynamic call but for now I think it best to just exclude them so the rule is less likely to remove intended user code.