pmd-lang-test: Improvement to check node position#1824
Merged
Conversation
Note: this is a temporary fix. A more general fix is in branch java-grammar
Generated by 🚫 Danger |
oowekyala
reviewed
May 14, 2019
|
|
||
| public void jjtSetFirstToken(final GenericToken token) { | ||
| this.firstToken = token; | ||
| this.beginLine = token.getBeginLine(); |
Member
There was a problem hiding this comment.
Note: the counterpart of this change for jjtSetLastToken is in #1826
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.
This file contains hidden or 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
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.
Changes:
assertTextRangeIsOk, that is executed on every node in the kotlin-based tests#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
jjtClosemethods 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).