Skip to content

[java] Merge prefix/postfix expressions into one node#2155

Merged
oowekyala merged 9 commits into
pmd:java-grammarfrom
oowekyala:grammar-prefix-postfix
Dec 15, 2019
Merged

[java] Merge prefix/postfix expressions into one node#2155
oowekyala merged 9 commits into
pmd:java-grammarfrom
oowekyala:grammar-prefix-postfix

Conversation

@oowekyala

@oowekyala oowekyala commented Dec 9, 2019

Copy link
Copy Markdown
Member

Merges IncrementExpression into UnaryExpression. Instead the operator has a method to check whether it has side effects. This doesn't hurt usability, and fixes the inconsistency with the naming of those nodes (increment expressions may also be decrementing, and they are unary, by definition).

This section of the wiki can be updated to read:


Merge unary expressions

  • What: Merge AST nodes for postfix and prefix expressions into the single UnaryExpression node. The merged nodes are:
    • PreIncrementExpression
    • PreDecrementExpression
    • UnaryExpression
    • UnaryExpressionNotPlusMinus
  • Why: Those nodes were asymmetric, and inconsistently nested within UnaryExpression. By definition they're all unary, so that using a single node is appropriate.
  • #1890 [java] Merge different increment/decrement expressions
CodeOld ASTNew AST
++a;
--b;
c++;
d--;
StatementExpression
  + PreIncrementExpression
    + PrimaryExpression
      + PrimaryPrefix
        + Name "a"
StatementExpression
  + PreDecrementExpression
    + PrimaryExpression
      + PrimaryPrefix
        + Name "b"
StatementExpression
  + PostfixExpression "++"
    + PrimaryExpression
      + PrimaryPrefix
        + Name "c"
StatementExpression
  + PostfixExpression "--"
    + PrimaryExpression
      + PrimaryPrefix
        + Name "d"
StatementExpression
  + UnaryExpression[@Prefix=true()][@Operator="++"]
    + VariableAccess "a"
StatementExpression
  + UnaryExpression[@Prefix=true()][@Operator="--"]
    + VariableAccess "b"
StatementExpression
  + UnaryExpression[@Prefix=false()][@Operator="++"]
    + VariableAccess "c"
StatementExpression
  + UnaryExpression[@Prefix=false()][@Operator="--"]
    + VariableAccess "d"
~a
+a
UnaryExpression[@Image=null]
  + UnaryExpressionNotPlusMinus[@Image="~"]
    + PrimaryExpression
      + PrimaryPrefix
        + Name "a"

UnaryExpression[@Image="+"]
  + PrimaryExpression
    + PrimaryPrefix
      + Name "a"
+ UnaryExpression[@Operator="~"]
  + VariableAccess "a"

+ UnaryExpression[@Operator="+"]
  + VariableAccess "a"

Among the different possible categorisations
of unary expressions, this is probably the
most logical and easiest to document.

Here's a comparison of different possible
categorisations. Note: `_++` is the postfix
increment operator, while `++_` is the prefix
one - idem for decrement. The last one is the
one implemented by this commit.

\## 6.0.x

```
  Unary = { -, + }
  UnaryNotPlusMinus { !, ~ }
  PreIncrement = { ++_ }
  PreDecrement { --_ }
  Postfix { _++, _++ }
```

* Very assymmetric, splits operators based on parsing
concerns

\## Before pmd#1890:

```
  Unary = { -, + , !, ~ }
  PreIncrement = { ++_ }
  PreDecrement { --_ }
  Postfix { _++, _++ }
```

* Minor simplification

\## pmd#1890:

```
  Unary = Prefix \ { ++_, --_ })
  Increment ( { ++ , -- } x (postfix, prefix) )
```
* Names are weird (IncrementExpr may be decrement, Unary != Increment
  even though semantically, Increment \subset Unary)
* No possibility to introduce a supertype (what would it be?)
* But easy to match all increment/decrement expressions

\## JLS (also, Eclipse):

```
  Prefix = { !, ~, -, +, ++_, --_ }
  Postfix = { _++, _-- }
```

* Both can have super interface UnaryExpr
  * This allows matching all increment/decrement expressions easily too
* Easiest to document, JLS like, AST like
* Fits well with `InfixExpr`
@oowekyala oowekyala added the in:ast About the AST structure or API, the parsing step label Dec 9, 2019
@oowekyala oowekyala added this to the 7.0.0 milestone Dec 9, 2019

@adangel adangel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good 👍

@ghost

ghost commented Dec 10, 2019

Copy link
Copy Markdown
1 Message
📖 No java rules are changed!

Generated by 🚫 Danger

@oowekyala oowekyala merged commit 218fdc7 into pmd:java-grammar Dec 15, 2019
@oowekyala oowekyala deleted the grammar-prefix-postfix branch December 15, 2019 00:35
@adangel adangel mentioned this pull request Jan 23, 2023
55 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in:ast About the AST structure or API, the parsing step

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants