Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle erroneous nodes in open rewrite #4412

Merged
merged 27 commits into from
Dec 19, 2024
Merged

Handle erroneous nodes in open rewrite #4412

Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
997b797
Handle erroneous nodes in a tree
vudayani Aug 1, 2024
8969dc5
Add visitErroneous to all java parser visitors
vudayani Aug 1, 2024
3414438
Override the visitVariable to handle erroneous identifier names set b…
vudayani Aug 2, 2024
da52622
retain name and suffix for erroneous varDecl
vudayani Aug 9, 2024
db8e069
override the visitVariable to handle error identifiers in all java pa…
vudayani Aug 13, 2024
79961ad
Remove sysout
jkschneider Aug 13, 2024
7d894bd
Update rewrite-java-test/src/test/java/org/openrewrite/java/JavaParse…
jkschneider Aug 13, 2024
5191acd
Update rewrite-java-test/src/test/java/org/openrewrite/java/JavaParse…
jkschneider Aug 13, 2024
ac32851
handle errors in method params, variable declarations, fix tests
vudayani Sep 6, 2024
df8fb83
Add missing license headers
timtebeek Sep 6, 2024
5c0dadd
fix compilation error
vudayani Sep 16, 2024
a29b0b0
fix compilation error in Java8ParserVisitor
vudayani Sep 18, 2024
c393fc0
Apply code suggestions from bot
timtebeek Sep 21, 2024
611d18b
fix cases for statementDelim
vudayani Sep 24, 2024
d60543f
fix block statement template generator to handle adding semicolon
vudayani Oct 3, 2024
7754d25
fix ChangeStaticFieldToMethod recipe
vudayani Oct 4, 2024
41420b6
Merge branch 'main' into handle-erroneous-nodes
timtebeek Oct 6, 2024
fb22682
Record compiler errors from erroneous LST nodes
BoykoAlex Oct 7, 2024
9951a4b
Merge branch 'main' into handle-erroneous-nodes
timtebeek Oct 22, 2024
1cf92eb
Merge branch 'main' into handle-erroneous-nodes
timtebeek Oct 29, 2024
b0af9b9
Merge branch 'main' into handle-erroneous-nodes
timtebeek Nov 3, 2024
078b799
Adjustments for comments
BoykoAlex Nov 22, 2024
bbdef52
Merge branch 'main' into handle-erroneous-nodes
BoykoAlex Nov 23, 2024
b1bb87e
Java 17 parser adjustment alos in 8, 11 and 21
BoykoAlex Nov 23, 2024
4461dce
Merge branch 'main' into handle-erroneous-nodes
timtebeek Nov 25, 2024
c8afbc5
Merge branch 'main' into handle-erroneous-nodes
timtebeek Dec 18, 2024
8a1d9c9
Add `FindCompileErrorsTest` & move away from deprecated `print()`
timtebeek Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 93 additions & 5 deletions ...te-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11ParserVisitor.java
View file
Open in desktop
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,38 @@ 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)));
}
if (endPos(t) == cursor && rightPadded.getElement() instanceof J.Erroneous) {
cursor++;
} 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 +1743,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 +1808,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
95 changes: 90 additions & 5 deletions ...te-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java
View file
Open in desktop
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.CompactConstructor;
import org.openrewrite.java.marker.OmitParentheses;
Expand Down Expand Up @@ -198,6 +200,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 @@ -1520,6 +1532,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 @@ -1710,10 +1738,38 @@ 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)));
}
if (endPos(t) == cursor && rightPadded.getElement() instanceof J.Erroneous) {
cursor++;
Copy link
Contributor

Choose a reason for hiding this comment

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

This part is unclear to me. Why advance the cursor a single character? Can we add a comment explaining this? Is there a specific test case which would demonstrate this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd have to dig into this to recall why this was done. Hopefully a test would fail if I comment this out. Will get back on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Added a comment

} 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 @@ -1768,25 +1824,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();
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
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 All @@ -1811,6 +1892,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