[Downgrade 7.0] Strip unnecessary parentheses around expressions #1617
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jtojnar
commented
Jan 3, 2022
rules/DowngradePhp70/Rector/Expr/DowngradeUnnecessarilyParenthesizedExpressionRector.php
Outdated
Show resolved
Hide resolved
jtojnar
commented
Jan 3, 2022
rules/DowngradePhp70/Rector/Expr/DowngradeUnnecessarilyParenthesizedExpressionRector.php
Outdated
Show resolved
Hide resolved
jtojnar
force-pushed
the
remove-extra-parens
branch
from
b150a32
to
3996410
Compare
|
The class name seems too long: https://github.com/rectorphp/rector-src/runs/4708654341?check_suite_focus=true#step:5:9 Could you try rename to the shorter name? Thank you |
|
It looks to me the rule thinks more than five lines of comments must be code even when it is documentation: |
|
I see, How about move comment to the method? |
|
The comment already has a method for itself (sharing it with two statements). I guess I could make the lines slightly longer so it fits on five lines. |
|
Or remove the rule since it does not actually check for commented out code but comment length. |
TomasVotruba
reviewed
Jan 9, 2022
rules/DowngradePhp70/Rector/Expr/DowngradeUnnecessarilyParenthesizedExpressionRector.php
Outdated
Show resolved
Hide resolved
jtojnar
force-pushed
the
remove-extra-parens
branch
4 times, most recently
from
be68c84
to
e1246ab
Compare
samsonasik
reviewed
Jan 24, 2022
rules/DowngradePhp70/Rector/Expr/DowngradeUnnecessarilyParenthesizedExpressionRector.php
Outdated
Show resolved
Hide resolved
jtojnar
force-pushed
the
remove-extra-parens
branch
3 times, most recently
from
0f0010c
to
bfd678a
Compare
The Uniform variable syntax RFC implemented in PHP 7.0 allows wrapping arbitrary expressions in parentheses, which is not supported by PHP 5. Since PHP 5 cannot parse that we need to strip them. And because parenthesization is not part of the AST, we need to force the code to be re-generated. Rector will re-generate code for AST nodes that changed. Or we can remove the original node reference to the same effect. The code generator will only put parentheses where strictly necessary, like to disambiguate method call from calling a callable property in `($foo->bar)()`. That should be handled by a different rule.
jtojnar
force-pushed
the
remove-extra-parens
branch
from
bfd678a
to
197e5cb
Compare
samsonasik
approved these changes
Jan 24, 2022
TomasVotruba
approved these changes
Jan 24, 2022
|
Thank you |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Uniform variable syntax RFC implemented in PHP 7.0 allows wrapping arbitrary expressions in parentheses, which is not supported by PHP 5. Let’s re-create those AST nodes so that the parentheses are stripped where not strictly necessary.
This does not deal with syntax like
($foo->bar)(), where parentheses are mandatory, which is used to disambiguate method calls from calling callable properties.