Skip to content

Commit

Permalink
Handle erroneous nodes in open rewrite (#4412)
Browse files Browse the repository at this point in the history 
* Handle erroneous nodes in a tree

* Add visitErroneous to all java parser visitors

* Override the visitVariable to handle erroneous identifier names set by JavacParser

* retain name and suffix for erroneous varDecl

* override the visitVariable to handle error identifiers in all java parser visitors

* Remove sysout

* Update rewrite-java-test/src/test/java/org/openrewrite/java/JavaParserTest.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update rewrite-java-test/src/test/java/org/openrewrite/java/JavaParserTest.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* handle errors in method params, variable declarations, fix tests

* Add missing license headers

* fix compilation error

* fix compilation error in Java8ParserVisitor

* Apply code suggestions from bot

* fix cases for statementDelim

* fix block statement template generator to handle adding semicolon

* fix ChangeStaticFieldToMethod recipe

* Record compiler errors from erroneous LST nodes

* Adjustments for comments

* Java 17 parser adjustment alos in 8, 11 and 21

* Add `FindCompileErrorsTest` & move away from deprecated `print()`

---------

Co-authored-by: Jonathan Schnéider <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Tim te Beek <[email protected]>
Co-authored-by: aboyko <[email protected]>
  • Loading branch information
@vudayani @jkschneider
@github-actions @timtebeek @BoykoAlex
5 people authored Dec 19, 2024
1 parent 6b2b63c commit e5d9337
Show file tree
Hide file tree
Showing 16 changed files with 805 additions and 57 deletions.
103 changes: 98 additions & 5 deletions ...te-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11ParserVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@
import org.openrewrite.Cursor;
import org.openrewrite.ExecutionContext;
import org.openrewrite.FileAttributes;
import org.openrewrite.PrintOutputCapture;
import org.openrewrite.internal.EncodingDetectingInputStream;
import org.openrewrite.internal.ListUtils;
import org.openrewrite.java.JavaParsingException;
import org.openrewrite.java.JavaPrinter;
import org.openrewrite.java.internal.JavaTypeCache;
import org.openrewrite.java.marker.OmitParentheses;
import org.openrewrite.java.tree.*;
Expand Down Expand Up @@ -191,6 +193,16 @@ public J visitAssignment(AssignmentTree node, Space fmt) {
typeMapping.type(node));
}

@Override
public J visitErroneous(ErroneousTree node, Space fmt) {
String erroneousNode = source.substring(((JCTree) node).getStartPosition(), ((JCTree) node).getEndPosition(endPosTable));
return new J.Erroneous(
randomId(),
fmt,
Markers.EMPTY,
erroneousNode);
}

@Override
public J visitBinary(BinaryTree node, Space fmt) {
Expression left = convert(node.getLeftOperand());
Expand Down Expand Up @@ -1439,6 +1451,22 @@ public J visitUnary(UnaryTree node, Space fmt) {

@Override
public J visitVariable(VariableTree node, Space fmt) {
JCTree.JCVariableDecl jcVariableDecl = (JCTree.JCVariableDecl) node;
if ("<error>".equals(jcVariableDecl.getName().toString())) {
int startPos = jcVariableDecl.getStartPosition();
int endPos = jcVariableDecl.getEndPosition(endPosTable);

if (startPos == endPos) {
endPos = startPos + 1;
}
String erroneousNode = source.substring(startPos, endPos);
return new J.Erroneous(
randomId(),
fmt,
Markers.EMPTY,
erroneousNode
);
}
return hasFlag(node.getModifiers(), Flags.ENUM) ?
visitEnumVariable(node, fmt) :
visitVariables(singletonList(node), fmt); // method arguments cannot be multi-declarations
Expand Down Expand Up @@ -1619,10 +1647,43 @@ private <J2 extends J> JRightPadded<J2> convert(Tree t, Function<Tree, Space> su
J2 j = convert(t);
@SuppressWarnings("ConstantConditions") JRightPadded<J2> rightPadded = j == null ? null :
new JRightPadded<>(j, suffix.apply(t), Markers.EMPTY);
cursor(max(endPos(t), cursor)); // if there is a non-empty suffix, the cursor may have already moved past it
int idx = findFirstNonWhitespaceChar(rightPadded.getAfter().getWhitespace());
if (idx >= 0) {
rightPadded = (JRightPadded<J2>) JRightPadded.build(getErroneous(List.of(rightPadded)));
}
// Cursor hasn't been updated but points at the end of erroneous node already
// This means that error node start position == end position
// Therefore ensure that cursor has moved to the end of erroneous node bu adding its length to the cursor
// Example `/pet` results in 2 erroeneous nodes: `/` and `pet`. The `/` node would have start and end position the
// same from the JC compiler.
if (endPos(t) == cursor && rightPadded.getElement() instanceof J.Erroneous) {
cursor += ((J.Erroneous) rightPadded.getElement()).getText().length();
} else {
cursor(max(endPos(t), cursor)); // if there is a non-empty suffix, the cursor may have already moved past it
}
return rightPadded;
}

private <J2 extends J> J.Erroneous getErroneous(List<JRightPadded<J2>> converted) {
PrintOutputCapture p = new PrintOutputCapture<>(0);
new JavaPrinter<>().visitContainer(JContainer.build(EMPTY, converted, Markers.EMPTY), JContainer.Location.METHOD_INVOCATION_ARGUMENTS, p);
return new J.Erroneous(
org.openrewrite.Tree.randomId(),
EMPTY,
Markers.EMPTY,
p.getOut()
);
}

private static int findFirstNonWhitespaceChar(String s) {
for (int i = 0; i < s.length(); i++) {
if (!Character.isWhitespace(s.charAt(i))) {
return i;
}
}
return -1;
}

private long lineNumber(Tree tree) {
return source.substring(0, ((JCTree) tree).getStartPosition()).chars().filter(c -> c == '\n').count() + 1;
}
Expand Down Expand Up @@ -1687,19 +1748,47 @@ private Space statementDelim(@Nullable Tree t) {
t instanceof JCNewClass ||
t instanceof JCReturn ||
t instanceof JCThrow ||
t instanceof JCUnary ||
t instanceof JCExpressionStatement ||
t instanceof JCVariableDecl) {
t instanceof JCUnary) {
return sourceBefore(";");
}

if (t instanceof JCLabeledStatement) {
return statementDelim(((JCLabeledStatement) t).getStatement());
}

if (t instanceof JCExpressionStatement) {
ExpressionTree expTree = ((ExpressionStatementTree) t).getExpression();
if (expTree instanceof ErroneousTree) {
return Space.build(source.substring(((JCTree) expTree).getEndPosition(endPosTable),((JCTree) t).getEndPosition(endPosTable)), Collections.emptyList());
} else {
return sourceBefore(";");
}
}

if (t instanceof JCVariableDecl) {
JCTree.JCVariableDecl varTree = (JCTree.JCVariableDecl) t;
if ("<error>".contentEquals(varTree.getName())) {
int start = varTree.vartype.getEndPosition(endPosTable);
int end = varTree.getEndPosition(endPosTable);
String whitespace = source.substring(start, end);
if (whitespace.contains("\n")) {
return EMPTY;
} else {
return Space.build(source.substring(start, end), Collections.emptyList());
}
}
return sourceBefore(";");
}

if (t instanceof JCMethodDecl) {
JCMethodDecl m = (JCMethodDecl) t;
return sourceBefore(m.body == null || m.defaultValue != null ? ";" : "");
if (m.body == null || m.defaultValue != null) {
String suffix = source.substring(cursor, positionOfNext(";", null));
int idx = findFirstNonWhitespaceChar(suffix);
return sourceBefore(idx >= 0 ? "" : ";");
} else {
return sourceBefore("");
}
}

return EMPTY;
Expand All @@ -1724,6 +1813,10 @@ private List<JRightPadded<Statement>> convertStatements(@Nullable List<? extends
List<JRightPadded<Statement>> converted = new ArrayList<>(treesGroupedByStartPosition.size());
for (List<? extends Tree> treeGroup : treesGroupedByStartPosition.values()) {
if (treeGroup.size() == 1) {
Tree t = treeGroup.get(0);
int startPosition = ((JCTree) t).getStartPosition();
if (cursor > startPosition)
continue;
converted.add(convert(treeGroup.get(0), suffix));
} else {
// multi-variable declarations are split into independent overlapping JCVariableDecl's by the OpenJDK AST
Expand Down
103 changes: 97 additions & 6 deletions ...te-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@
import org.openrewrite.Cursor;
import org.openrewrite.ExecutionContext;
import org.openrewrite.FileAttributes;
import org.openrewrite.PrintOutputCapture;
import org.openrewrite.internal.EncodingDetectingInputStream;
import org.openrewrite.internal.ListUtils;
import org.openrewrite.java.JavaParsingException;
import org.openrewrite.java.JavaPrinter;
import org.openrewrite.java.internal.JavaTypeCache;
import org.openrewrite.java.marker.CompactConstructor;
import org.openrewrite.java.marker.OmitParentheses;
Expand Down Expand Up @@ -199,6 +201,16 @@ public J visitAssignment(AssignmentTree node, Space fmt) {
typeMapping.type(node));
}

@Override
public J visitErroneous(ErroneousTree node, Space fmt) {
String erroneousNode = source.substring(((JCTree) node).getStartPosition(), ((JCTree) node).getEndPosition(endPosTable));
return new J.Erroneous(
randomId(),
fmt,
Markers.EMPTY,
erroneousNode);
}

@Override
public J visitBinary(BinaryTree node, Space fmt) {
Expression left = convert(node.getLeftOperand());
Expand Down Expand Up @@ -1521,6 +1533,22 @@ public J visitUnary(UnaryTree node, Space fmt) {

@Override
public J visitVariable(VariableTree node, Space fmt) {
JCTree.JCVariableDecl jcVariableDecl = (JCTree.JCVariableDecl) node;
if ("<error>".equals(jcVariableDecl.getName().toString())) {
int startPos = jcVariableDecl.getStartPosition();
int endPos = jcVariableDecl.getEndPosition(endPosTable);

if (startPos == endPos) {
endPos = startPos + 1; // For cases where the error node is a single character like "/"
}
String erroneousNode = source.substring(startPos, endPos);
return new J.Erroneous(
randomId(),
fmt,
Markers.EMPTY,
erroneousNode
);
}
return hasFlag(node.getModifiers(), Flags.ENUM) ?
visitEnumVariable(node, fmt) :
visitVariables(singletonList(node), fmt); // method arguments cannot be multi-declarations
Expand Down Expand Up @@ -1732,11 +1760,45 @@ private void reportJavaParsingException(Throwable ex) {
return null;
}
J2 j = convert(t);
JRightPadded<J2> rightPadded = new JRightPadded<>(j, suffix.apply(t), Markers.EMPTY);
cursor(max(endPos(t), cursor)); // if there is a non-empty suffix, the cursor may have already moved past it
@SuppressWarnings("ConstantConditions") JRightPadded<J2> rightPadded = j == null ? null :
new JRightPadded<>(j, suffix.apply(t), Markers.EMPTY);
int idx = findFirstNonWhitespaceChar(rightPadded.getAfter().getWhitespace());
if (idx >= 0) {
rightPadded = (JRightPadded<J2>) JRightPadded.build(getErroneous(List.of(rightPadded)));
}
// Cursor hasn't been updated but points at the end of erroneous node already
// This means that error node start position == end position
// Therefore ensure that cursor has moved to the end of erroneous node bu adding its length to the cursor
// Example `/pet` results in 2 erroeneous nodes: `/` and `pet`. The `/` node would have start and end position the
// same from the JC compiler.
if (endPos(t) == cursor && rightPadded.getElement() instanceof J.Erroneous) {
cursor += ((J.Erroneous) rightPadded.getElement()).getText().length();
} else {
cursor(max(endPos(t), cursor)); // if there is a non-empty suffix, the cursor may have already moved past it
}
return rightPadded;
}

private <J2 extends J> J.Erroneous getErroneous(List<JRightPadded<J2>> converted) {
PrintOutputCapture p = new PrintOutputCapture<>(0);
new JavaPrinter<>().visitContainer(JContainer.build(EMPTY, converted, Markers.EMPTY), JContainer.Location.METHOD_INVOCATION_ARGUMENTS, p);
return new J.Erroneous(
org.openrewrite.Tree.randomId(),
EMPTY,
Markers.EMPTY,
p.getOut()
);
}

private static int findFirstNonWhitespaceChar(String s) {
for (int i = 0; i < s.length(); i++) {
if (!Character.isWhitespace(s.charAt(i))) {
return i;
}
}
return -1;
}

private long lineNumber(Tree tree) {
return source.substring(0, ((JCTree) tree).getStartPosition()).chars().filter(c -> c == '\n').count() + 1;
}
Expand Down Expand Up @@ -1783,25 +1845,50 @@ private <J2 extends J> List<JRightPadded<J2>> convertAll(List<? extends Tree> tr

private Space statementDelim(@Nullable Tree t) {
switch (t.getKind()) {
case CONTINUE:
case RETURN:
case BREAK:
case ASSERT:
case ASSIGNMENT:
case BREAK:
case CONTINUE:
case DO_WHILE_LOOP:
case IMPORT:
case METHOD_INVOCATION:
case NEW_CLASS:
case RETURN:
case THROW:
return sourceBefore(";");
case EXPRESSION_STATEMENT:
ExpressionTree expTree = ((ExpressionStatementTree) t).getExpression();
if (expTree instanceof ErroneousTree) {
return Space.build(source.substring(((JCTree) expTree).getEndPosition(endPosTable),((JCTree) t).getEndPosition(endPosTable)), Collections.emptyList());
} else {
return sourceBefore(";");
}
case VARIABLE:
JCTree.JCVariableDecl varTree = (JCTree.JCVariableDecl) t;
if ("<error>".contentEquals(varTree.getName())) {
int start = varTree.vartype.getEndPosition(endPosTable);
int end = varTree.getEndPosition(endPosTable);
String whitespace = source.substring(start, end);
if (whitespace.contains("\n")) {
return EMPTY;
} else {
return Space.build(source.substring(start, end), Collections.emptyList());
}
}
return sourceBefore(";");
case YIELD:
return sourceBefore(";");
case LABELED_STATEMENT:
return statementDelim(((JCLabeledStatement) t).getStatement());
case METHOD:
JCMethodDecl m = (JCMethodDecl) t;
return sourceBefore(m.body == null || m.defaultValue != null ? ";" : "");
if (m.body == null || m.defaultValue != null) {
String suffix = source.substring(cursor, positionOfNext(";", null));
int idx = findFirstNonWhitespaceChar(suffix);
return sourceBefore(idx >= 0 ? "" : ";");
} else {
return sourceBefore("");
}
default:
return t instanceof JCAssignOp || t instanceof JCUnary ? sourceBefore(";") : EMPTY;
}
Expand Down Expand Up @@ -1829,6 +1916,10 @@ private List<JRightPadded<Statement>> convertStatements(@Nullable List<? extends
List<JRightPadded<Statement>> converted = new ArrayList<>(treesGroupedByStartPosition.size());
for (List<? extends Tree> treeGroup : treesGroupedByStartPosition.values()) {
if (treeGroup.size() == 1) {
Tree t = treeGroup.get(0);
int startPosition = ((JCTree) t).getStartPosition();
if (cursor > startPosition)
continue;
converted.add(convert(treeGroup.get(0), suffix));
} else {
// multi-variable declarations are split into independent overlapping JCVariableDecl's by the OpenJDK AST
Expand Down
Loading

0 comments on commit e5d9337

Please sign in to comment.