Skip to content

pmd-lang-test: Improvement to check node position#1824

Merged
oowekyala merged 7 commits into
pmd:pmd/7.0.xfrom
adangel:pmd7-lang-test-improvements
May 18, 2019
Merged

pmd-lang-test: Improvement to check node position#1824
oowekyala merged 7 commits into
pmd:pmd/7.0.xfrom
adangel:pmd7-lang-test-improvements

Conversation

@adangel

@adangel adangel commented May 11, 2019

Copy link
Copy Markdown
Member

Changes:

  • adds the new assertTextRangeIsOk, that is executed on every node in the kotlin-based tests
  • this ensures, that the position of the nodes in the AST is the same as they appear in the source code
  • For the new switch labeled rules, we have the following grammar
void SwitchStatement():
{}
{
  "switch" "(" Expression() ")" SwitchBlock()
}

void SwitchBlock() #void :
{}
{
    "{"
        (
            SwitchLabel()
            (
                "->" SwitchLabeledRulePart() (SwitchLabeledRule())*
            |
                ":" (LOOKAHEAD(2) SwitchLabel() ":")* (BlockStatement())* (SwitchLabeledStatementGroup())*
            )
        )?
    "}"
}

void SwitchLabeledRulePart() #void:
{checkForSwitchRules();}
{
    (
        ( Expression() ";" ) #SwitchLabeledExpression(2)
      |
        ( Block() ) #SwitchLabeledBlock(2)
      |
        ( ThrowStatement() ) #SwitchLabeledThrowStatement(2)
    )
}

#SwitchLabeledBlock(2) takes the last two nodes (SwitchLabel + Block) and adds them as SwitchLabeledBlock to SwitchStatement. JavaCC sets the first token of SwitchLabeledBlock to be the Block node, but it should be the SwitchLabel node.

This is fixed in the jjtClose methods in the three related SwitchLabeled* nodes.

On the expression grammar PR, there is a better solution (you can mark the nodes via the interface LeftRecursiveNode).

  • So, this actually fixes a bug, that is present in PMD 6.

@adangel adangel added this to the 7.0.0 milestone May 11, 2019
@ghost

ghost commented May 11, 2019

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

Generated by 🚫 Danger


public void jjtSetFirstToken(final GenericToken token) {
this.firstToken = token;
this.beginLine = token.getBeginLine();

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.

Note: the counterpart of this change for jjtSetLastToken is in #1826

@oowekyala oowekyala merged commit a0657dd into pmd:pmd/7.0.x May 18, 2019
oowekyala added a commit that referenced this pull request May 18, 2019
From PR #1824

* This PR is for PMD 7
* It's extracted from #1759

Changes:
* adds the new `assertTextRangeIsOk`, that is executed on every node in
* the kotlin-based tests
* this ensures, that the position of the nodes in the AST is the same as
* they appear in the source code
* For the new switch labeled rules, we have the following grammar

```
void SwitchStatement():
{}
{
  "switch" "(" Expression() ")" SwitchBlock()
}

void SwitchBlock() #void :
{}
{
    "{"
        (
            SwitchLabel()
            (
                "->" SwitchLabeledRulePart() (SwitchLabeledRule())*
            |
                ":" (LOOKAHEAD(2) SwitchLabel() ":")*
(BlockStatement())* (SwitchLabeledStatementGroup())*
            )
        )?
    "}"
}

void SwitchLabeledRulePart() #void:
{checkForSwitchRules();}
{
    (
        ( Expression() ";" ) #SwitchLabeledExpression(2)
      |
        ( Block() ) #SwitchLabeledBlock(2)
      |
        ( ThrowStatement() ) #SwitchLabeledThrowStatement(2)
    )
}
```

`#SwitchLabeledBlock(2)` takes the last two nodes (SwitchLabel + Block)
and adds them as SwitchLabeledBlock to SwitchStatement. JavaCC sets the
first token of SwitchLabeledBlock to be the Block node, but it should be
the SwitchLabel node.

This is fixed in the `jjtClose` methods in the three related
SwitchLabeled* nodes.

On the expression grammar PR, there is a better solution (you can mark
the nodes via the interface `LeftRecursiveNode`).

* So, this actually fixes a bug, that is present in PMD 6.
@adangel adangel deleted the pmd7-lang-test-improvements branch May 26, 2019 10:29
@adangel adangel added the in:testing About tests of pmd, eg the module pmd-lang-test or pmd-test [test] label Jan 12, 2023
@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:testing About tests of pmd, eg the module pmd-lang-test or pmd-test [test]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants