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

Disable Java SpacesVisitor for non-Java sources #3792

Merged
merged 6 commits into from
Dec 8, 2023
Merged

Disable Java SpacesVisitor for non-Java sources #3792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6d29810
Disable java SpacesVisitor to make breaking changes on other code fil…
kunli2 Dec 7, 2023
0583aa2
Preserve prefix in `SimplifyBooleanExpressionVisitor`
knutwannheden Dec 8, 2023
0082680
Add another test case
knutwannheden Dec 8, 2023
bde21ac
Small cleanup to `UnwrapParentheses`
knutwannheden Dec 8, 2023
43c31c1
Further simplify `SimplifyBooleanExpressionVisitor`
knutwannheden Dec 8, 2023
27c44d0
Limit `MethodParamPad` to Java sources
knutwannheden Dec 8, 2023
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
28 changes: 28 additions & 0 deletions ...test/src/test/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,34 @@ public class A {
);
}

@Test
void preserveComments() {
rewriteRun(
java(
"""
public class A {
boolean m(boolean a) {
if (/*a*/!!a) {
return true;
}
return /*a*/!true || !true;
}
}
""",
"""
public class A {
boolean m(boolean a) {
if (/*a*/a) {
return true;
}
return /*a*/false;
}
}
"""
)
);
}

@ParameterizedTest
@Issue("https://github.com/openrewrite/rewrite-templating/issues/28")
// Mimic what would be inserted by a Refaster template using two nullable parameters, with the second one a literal
Expand Down
3 changes: 2 additions & 1 deletion rewrite-java/src/main/java/org/openrewrite/java/UnwrapParentheses.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.openrewrite.Cursor;
import org.openrewrite.Tree;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.Space;

public class UnwrapParentheses<P> extends JavaVisitor<P> {
private final J.Parentheses<?> scope;
Expand All @@ -33,7 +34,7 @@ public <T extends J> J visitParentheses(J.Parentheses<T> parens, P p) {
if (tree.getPrefix().isEmpty()) {
Object parent = getCursor().getParentOrThrow().getValue();
if (parent instanceof J.Return || parent instanceof J.Throw) {
tree = tree.withPrefix(tree.getPrefix().withWhitespace(" "));
tree = tree.withPrefix(Space.SINGLE_SPACE);
}
}
return tree;
Expand Down
28 changes: 5 additions & 23 deletions ...ite-java/src/main/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,16 @@
import org.openrewrite.ExecutionContext;
import org.openrewrite.Tree;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.search.SemanticallyEqual;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.Space;

import java.util.Collections;

public class SimplifyBooleanExpressionVisitor extends JavaVisitor<ExecutionContext> {
private static final String MAYBE_AUTO_FORMAT_ME = "MAYBE_AUTO_FORMAT_ME";

@Override
public J visitBinary(J.Binary binary, ExecutionContext ctx) {
J j = super.visitBinary(binary, ctx);
Expand All @@ -45,8 +42,7 @@ public J visitBinary(J.Binary binary, ExecutionContext ctx) {
j = asBinary.getRight();
} else if (isLiteralTrue(asBinary.getRight())) {
j = asBinary.getLeft().withPrefix(asBinary.getLeft().getPrefix().withWhitespace(""));
} else if (removeAllSpace(asBinary.getLeft()).printTrimmed(getCursor())
.equals(removeAllSpace(asBinary.getRight()).printTrimmed(getCursor()))) {
} else if (SemanticallyEqual.areEqual(asBinary.getLeft(), asBinary.getRight())) {
j = asBinary.getLeft();
}
} else if (asBinary.getOperator() == J.Binary.Type.Or) {
Expand All @@ -58,8 +54,7 @@ public J visitBinary(J.Binary binary, ExecutionContext ctx) {
j = asBinary.getRight();
} else if (isLiteralFalse(asBinary.getRight())) {
j = asBinary.getLeft().withPrefix(asBinary.getLeft().getPrefix().withWhitespace(""));
} else if (removeAllSpace(asBinary.getLeft()).printTrimmed(getCursor())
.equals(removeAllSpace(asBinary.getRight()).printTrimmed(getCursor()))) {
} else if (SemanticallyEqual.areEqual(asBinary.getLeft(), asBinary.getRight())) {
j = asBinary.getLeft();
}
} else if (asBinary.getOperator() == J.Binary.Type.Equal) {
Expand Down Expand Up @@ -88,7 +83,7 @@ public J visitBinary(J.Binary binary, ExecutionContext ctx) {
}
}
if (asBinary != j) {
getCursor().getParentTreeCursor().putMessage(MAYBE_AUTO_FORMAT_ME, "");
j = j.withPrefix(asBinary.getPrefix());
}
return j;
}
Expand All @@ -99,9 +94,6 @@ public J postVisit(J tree, ExecutionContext ctx) {
if (j instanceof J.Parentheses) {
j = new UnnecessaryParenthesesVisitor<>().visit(j, ctx, getCursor().getParentOrThrow());
}
if (j != null && getCursor().pollMessage(MAYBE_AUTO_FORMAT_ME) != null) {
j = autoFormat(j, ctx);
}
return j;
}

Expand Down Expand Up @@ -133,7 +125,7 @@ public J visitUnary(J.Unary unary, ExecutionContext ctx) {
}
}
if (asUnary != j) {
getCursor().getParentTreeCursor().putMessage(MAYBE_AUTO_FORMAT_ME, "");
j = j.withPrefix(asUnary.getPrefix());
}
return j;
}
Expand Down Expand Up @@ -216,16 +208,6 @@ private J.Literal booleanLiteral(J j, boolean value) {
JavaType.Primitive.Boolean);
}

private J removeAllSpace(J j) {
//noinspection ConstantConditions
return new JavaIsoVisitor<Integer>() {
@Override
public Space visitSpace(Space space, Space.Location loc, Integer integer) {
return Space.EMPTY;
}
}.visit(j, 0);
}

/**
* Override this method to disable simplification of equals expressions,
* specifically for Kotlin while that is not yet part of the OpenRewrite/rewrite.
Expand Down
6 changes: 6 additions & 0 deletions rewrite-java/src/main/java/org/openrewrite/java/format/MethodParamPad.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ private static class MethodParamPadVisitor extends JavaIsoVisitor<ExecutionConte
SpacesStyle spacesStyle;
MethodParamPadStyle methodParamPadStyle;

@Override
public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
// This visitor causes problems for Groovy sources as the `SpacesVisitor` is not aware of Groovy
return sourceFile instanceof J.CompilationUnit;
}

@Override
public J visit(@Nullable Tree tree, ExecutionContext ctx) {
if (tree instanceof JavaSourceFile) {
Expand Down
6 changes: 6 additions & 0 deletions rewrite-java/src/main/java/org/openrewrite/java/format/SpacesVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.openrewrite.java.format;

import org.openrewrite.SourceFile;
import org.openrewrite.Tree;
import org.openrewrite.internal.ListUtils;
import org.openrewrite.internal.StringUtils;
Expand Down Expand Up @@ -54,6 +55,11 @@ public SpacesVisitor(SpacesStyle style, @Nullable EmptyForInitializerPadStyle em
this.stopAfter = stopAfter;
}

@Override
public boolean isAcceptable(SourceFile sourceFile, P p) {
return sourceFile instanceof J.CompilationUnit;
}

<T extends J> T spaceBefore(T j, boolean spaceBefore) {
if (!j.getComments().isEmpty()) {
return j;
Expand Down