-
Notifications
You must be signed in to change notification settings - Fork 63
[CodeQuality] Skip with if on YieldDataProviderRector with yield from and return #557
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
|
@samsonasik please see another case of the same problem as in #556 |
| yield from [ | ||
| ['value3', 'value4'], | ||
| ['value5', 'value6'], | ||
| ['value7', 'value8'], | ||
| ]; |
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.
could you add test to allow multiple yield from ? see https://3v4l.org/seVoc
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 can add it, but the current rector impl will not work with it ;)
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.
or should I add a test, that this scenario will be skipped?
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.
Yeah, I think we can fix with if yield form is multiple, just skip. findInstanceof() and count() > 1 should works.
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.
ok
samsonasik
left a comment
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.
Looks good, let's give it a try 👍
No description provided.