-
-
Notifications
You must be signed in to change notification settings - Fork 340
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
[Php74] Add CurlyToSquareBracketArrayStringRector #858
Conversation
13b365d
to
faa2f4c
Compare
All checks have passed 🎉 @TomasVotruba it is ready for review. |
389edf1
to
beb61b7
Compare
rebased. |
public function refactor(Node $node): ArrayDimFetch | ||
{ | ||
$node->setAttribute(AttributeKey::ORIGINAL_NODE, null); | ||
return $node; |
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 will lead to reprint of every node regardless the feature is present
There should be some check to verify the syntax change is needed.
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.
any way to check that? because printing check first is always return square one.
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.
That should not be hidden in the printer IMO.
Maybe using "kind" attribute as array has?
Check for the trailing comma rule, there is some logic with tokens too.
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 will try, probably something like FollowedByCommaAnalyzer
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.
implemented 🎉 I added FollowedByCurlyBracketAnalyzer
service for it https://github.com/rectorphp/rector-src/pull/858/files#diff-9b13ecfac64fb2e2e12b342348070427427b0d5ac27dd0a72774869535003034
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.
Thanks!
Could you add comment to original node to null? Just so it's clear what is the goal of 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.
I added the comment e1e8472, the purpose is to re-draw the ArrayDimFetch to use [] bracket
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.
Thank you
d4fb876
to
eb1afbf
Compare
61ff814
to
31bc052
Compare
All checks have passed 🎉 @TomasVotruba I think it is ready. |
No description provided.