Skip to content
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

[ForeachItemsAssignToEmptyArrayToAssignRector] apply on code it should not #2587

Conversation

gnutix
Copy link
Contributor

@gnutix gnutix commented Jan 6, 2020

No description provided.

@TomasVotruba
Copy link
Member

Could add fix as well?

@gnutix
Copy link
Contributor Author

gnutix commented Jan 6, 2020

Again, I might be able to - provided some context. What is this rector rule really about ? What are all the edge cases that might be broken, except the two I had in my code base ? What's the vision for it and how to implement it ?

I know nothing of it right now, and as I only have a small amount of time to dedicate to OSS, the better the explanation the greater the chances I can contribute efficiently. :)

@TomasVotruba
Copy link
Member

Allright :) I'll try my best. Ask whatever you need to know.

The rule is about removing foreach of array, that does basically assign of array:
https://github.com/rectorphp/rector/blob/master/docs/AllRectorsOverview.md#foreachitemsassigntoemptyarraytoassignrector

 class SomeClass
 {
     public function run($items)
     {
         $items2 = [];
-        foreach ($items as $item) {
-             $items2[] = $item;
-        }
+        $items2 = $items;
     }
 }

Everything else should be skipped - those edge-cases you've detected. I'll put comment there

@gnutix gnutix force-pushed the ForeachItemsAssignToEmptyArrayToAssignRector/add-broken-tests branch from 09636e8 to 0a64874 Compare January 11, 2020 00:56
@gnutix gnutix requested a review from TomasVotruba January 11, 2020 00:56
@gnutix
Copy link
Contributor Author

gnutix commented Jan 11, 2020

All good! :) Thanks for the help.

@gnutix gnutix added the bug label Jan 11, 2020
@gnutix gnutix changed the title [CodeQuality] ForeachItemsAssignToEmptyArrayToAssignRector apply on code it should not [ForeachItemsAssignToEmptyArrayToAssignRector] apply on code it should not Jan 11, 2020
@gnutix gnutix requested a review from TomasVotruba January 11, 2020 11:30
@TomasVotruba TomasVotruba merged commit 413a348 into rectorphp:master Jan 11, 2020
@TomasVotruba TomasVotruba deleted the ForeachItemsAssignToEmptyArrayToAssignRector/add-broken-tests branch January 11, 2020 11:34
@TomasVotruba
Copy link
Member

Great job, thanks 👍

TomasVotruba added a commit that referenced this pull request Jun 29, 2022
rectorphp/rector-src@d180768 [Core] Apply auto fallback to use Scope from File when Node both doesnot has parent Node and Scope (#2587)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants