[java] Remove ParenthesizedExpr #1872
Merged
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
adangel
approved these changes
Jun 19, 2019
There was a problem hiding this comment.
Avoiding the extra node level in the AST makes sense. The information about the parentheses is still retained, so rules like UselessParentheses are still possible.
| * of the JLS. One difference though, is that this production | ||
| * also matches lambda expressions, contrary to the JLS. | ||
| * This corresponds to the <a href="https://docs.oracle.com/javase/specs/jls/se9/html/jls-15.html#jls-Expression">Expression</a> | ||
| * of the JLS. | ||
| * | ||
| * <p>From 7.0.0 on, this is an interface which all expression nodes |
There was a problem hiding this comment.
It looks like we should add this comment on the master branch for PMD 6. Here it is already a interface...
| @@ -47,8 +47,16 @@ public boolean isExpressionBody() { | |||
| return !isBlockBody(); | |||
| } | |||
|
|
|||
| // TODO these are copied from AbstractJavaTypeNode because it's easier to extend abstractMethodLikeNode | |||
| // TODO MethodLikeNode should be removed, and this class extend AbstractJavaExpr | |||
There was a problem hiding this comment.
Looks like MethodLikeNode is mostly used in metrics. I see, this is already a point on https://github.com/pmd/pmd/wiki/PMD-7.0.0-Java
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.
Removes ParenthesizedExpr from the current version of java-grammar. It's replaced by an attribute of ASTExpression, that stores the number of parentheses levels around the expression. Most rules, and all analysis passes, would never care about parentheses. So having an extra node causes the following problems:
So eg as today,
1+2*3is parsedbut now, this is the same as
1+(2*3)(except in the latter the multiplicative expression has@Parenthesizedset to true).OTOH
(1+2)*3is parsed asNotice, that this really is a simplification, since because of operator precedence rules, the only way you can nest an addition into a multiplication is with parentheses anyway. That means, that the ParenthesizedExpr in this context was entirely redundant.
Finally,
1 + 2 + 3is parsed aswhereas
(1+2)+3is parsed asmeaning, we have to preserve the parens on a node that would otherwise have been flattened - even though they're unnecessary... But that's in fact consistent with
1+(2+3), which already is not flattened either: