Skip to content

Commit 80ecf94

Browse files
authored
Write literals, catches and && chains as the parser builds them
New rewriteRun checks in rewrite-test fail tests of eight recipes here. One check prints a recipe's output, parses it again and compares the trees. A recipe that runs later matches the tree, not the text, so it misses or mistakes a node that parsed code never holds. Another check fails a template that names a type it doesn't declare. - ReplaceTextBlockWithString put the two characters `\n` in a literal's value instead of a newline. The value now holds the newline. The text now escapes backslashes, quotes and line breaks, so a text block holding `\\` no longer comes out as an invalid escape. - IndexOfShouldNotCompareGreaterThanZero changed the text `0` to `1` but kept the value 0. - NewStringBuilderBufferWithCharArgument and PrimitiveWrapperClassConstructorToValueOf turned a char or double literal into a string and kept the old value. NewStringBuilderBufferWithCharArgument also wrote `'"'` as `"""` and `'\''` as `"\""`. It now keeps the character as written and escapes a double quote. - CombineSemanticallyEqualCatchBlocks kept a multi-catch with one type when only one type was left. It now catches that type on its own. - CollapsibleIfStatements built `a && (b && c)` where the parser reads `a && b && c` as `(a && b) && c`. It now groups the merged condition from the left. - RemoveInstanceOfPatternMatch's template named the instanceof type, such as `LocalDate`, without declaring it. It resolved only through the file's imports. The template now imports the types it names. - UseTryWithResources named an unused resource `_` on Java 21. Unnamed variables are a preview in Java 21, and javac 21 rejects the code without `--enable-preview`. The recipe now does this from Java 22, as rewrite-migrate-java's ReplaceUnusedVariablesWithUnderscore does. The tests now expect `_` on Java 22 and the old name on Java 21.
1 parent d807485 commit 80ecf94

12 files changed

Lines changed: 160 additions & 21 deletions

‎src/main/java/org/openrewrite/staticanalysis/CollapsibleIfStatements.java‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,7 @@ public J visitIf(J.If if_, ExecutionContext ctx) {
8181
innerCond = wrapInParens(innerCond);
8282
}
8383

84-
Expression combined = JavaElementFactory.newLogicalExpression(
85-
J.Binary.Type.And,
86-
outerCond,
87-
innerCond.withPrefix(Space.SINGLE_SPACE)
88-
);
84+
Expression combined = and(outerCond, innerCond.withPrefix(Space.SINGLE_SPACE));
8985

9086
J.If merged = outerIf
9187
.withIfCondition(outerIf.getIfCondition().withTree(combined))
@@ -94,6 +90,18 @@ public J visitIf(J.If if_, ExecutionContext ctx) {
9490
return autoFormat(merged, ctx);
9591
}
9692

93+
/**
94+
* Joins two conditions with {@code &&}, grouped from the left as the parser reads {@code a && b && c}.
95+
*/
96+
private Expression and(Expression left, Expression right) {
97+
if (right instanceof J.Binary && ((J.Binary) right).getOperator() == J.Binary.Type.And) {
98+
J.Binary binary = (J.Binary) right;
99+
return binary.withLeft(and(left, binary.getLeft().withPrefix(binary.getPrefix())))
100+
.withPrefix(Space.EMPTY);
101+
}
102+
return JavaElementFactory.newLogicalExpression(J.Binary.Type.And, left, right);
103+
}
104+
97105
private Expression wrapInParens(Expression expr) {
98106
return new J.Parentheses<>(
99107
Tree.randomId(),

‎src/main/java/org/openrewrite/staticanalysis/CombineSemanticallyEqualCatchBlocks.java‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,10 @@ public J visitMultiCatch(J.MultiCatch multiCatch, ExecutionContext ctx) {
180180
J.Try.Catch parent = parentCursor.getValue();
181181
if (parent == scope) {
182182
List<JRightPadded<NameTree>> combinedCatches = combineEquivalentCatches();
183+
TypeTree onlyType = onlyType(combinedCatches);
184+
if (onlyType != null) {
185+
return onlyType.withPrefix(m.getPrefix());
186+
}
183187
m = maybeAutoFormat(m, m.getPadding().withAlternatives(combinedCatches), ctx);
184188
}
185189
}
@@ -192,15 +196,25 @@ public J visitCatch(J.Try.Catch _catch, ExecutionContext ctx) {
192196
if (c == scope && !(c.getParameter().getTree().getTypeExpression() instanceof J.MultiCatch)) {
193197
if (c.getParameter().getTree().getTypeExpression() != null) {
194198
List<JRightPadded<NameTree>> combinedCatches = combineEquivalentCatches();
199+
TypeTree onlyType = onlyType(combinedCatches);
195200
c = maybeAutoFormat(c, c.withParameter(c.getParameter()
196201
.withTree(c.getParameter().getTree()
197-
.withTypeExpression(new J.MultiCatch(Tree.randomId(), EMPTY, Markers.EMPTY, combinedCatches)))),
202+
.withTypeExpression(onlyType != null ? onlyType :
203+
new J.MultiCatch(Tree.randomId(), EMPTY, Markers.EMPTY, combinedCatches)))),
198204
ctx);
199205
}
200206
}
201207
return c;
202208
}
203209

210+
/**
211+
* The one type left to catch, which the parser makes a plain type rather than a multi-catch, or null.
212+
*/
213+
private static @Nullable TypeTree onlyType(List<JRightPadded<NameTree>> combinedCatches) {
214+
return combinedCatches.size() == 1 && combinedCatches.get(0).getElement() instanceof TypeTree ?
215+
(TypeTree) combinedCatches.get(0).getElement() : null;
216+
}
217+
204218
private List<JRightPadded<NameTree>> combineEquivalentCatches() {
205219
Set<NameTree> removeExceptions = new HashSet<>();
206220
List<JRightPadded<NameTree>> combinedCatches = new ArrayList<>();

‎src/main/java/org/openrewrite/staticanalysis/IndexOfShouldNotCompareGreaterThanZero.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public J.Binary visitBinary(J.Binary binary, ExecutionContext ctx) {
6060
J.MethodInvocation mi = (J.MethodInvocation) b.getLeft();
6161
if (STRING_INDEX_MATCHER.matches(mi) || LIST_INDEX_MATCHER.matches(mi)) {
6262
if (b.getRight() instanceof J.Literal && "0".equals(((J.Literal) b.getRight()).getValueSource())) {
63-
b = b.withRight(((J.Literal) b.getRight()).withValueSource("1")).withOperator(J.Binary.Type.GreaterThanOrEqual);
63+
b = b.withRight(((J.Literal) b.getRight()).withValue(1).withValueSource("1")).withOperator(J.Binary.Type.GreaterThanOrEqual);
6464
}
6565
}
6666
}

‎src/main/java/org/openrewrite/staticanalysis/NewStringBuilderBufferWithCharArgument.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,11 @@ public J.NewClass visitNewClass(J.NewClass newClass, ExecutionContext ctx) {
6060
nc = nc.withArguments(ListUtils.mapFirst(nc.getArguments(), arg -> {
6161
if (arg instanceof J.Literal) {
6262
J.Literal l = (J.Literal) arg;
63-
l = l.withType(JavaType.buildType("String"));
63+
l = l.withType(JavaType.buildType("String")).withValue(String.valueOf(l.getValue()));
6464
if (l.getValueSource() != null) {
65-
l = l.withValueSource(l.getValueSource().replace("'", "\""));
65+
// the character as written, which a string holds as is, except a double quote
66+
String character = l.getValueSource().substring(1, l.getValueSource().length() - 1);
67+
l = l.withValueSource("\"" + ("\"".equals(character) ? "\\\"" : character) + "\"");
6668
}
6769
return l;
6870
}

‎src/main/java/org/openrewrite/staticanalysis/PrimitiveWrapperClassConstructorToValueOf.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,9 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) {
9292
break;
9393
case "java.lang.Float":
9494
if (arg instanceof J.Literal && JavaType.Primitive.Double == ((J.Literal) arg).getType()) {
95-
arg = ((J.Literal) arg).withType(JavaType.Primitive.String);
96-
arg = ((J.Literal) arg).withValueSource("\"" + ((J.Literal) arg).getValue() + "\"");
95+
String value = String.valueOf(((J.Literal) arg).getValue());
96+
arg = ((J.Literal) arg).withType(JavaType.Primitive.String).withValue(value)
97+
.withValueSource("\"" + value + "\"");
9798
}
9899

99100
JavaType argType = arg.getType();

‎src/main/java/org/openrewrite/staticanalysis/RemoveInstanceOfPatternMatch.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,13 +194,43 @@ private J.Block addVariableDeclaration(Cursor blockCursor, J.InstanceOf instance
194194
JavaTemplate template = JavaTemplate
195195
.builder("#{} #{} = (#{}) #{any()};")
196196
.contextSensitive()
197+
.imports(typesNamedBy(instanceOf.getClazz()))
197198
.build();
198199
return template.apply(blockCursor, block.getCoordinates().firstStatement(), instanceOf.getClazz().toString(),
199200
((J.Identifier) Objects.requireNonNull(instanceOf.getPattern())).getSimpleName(),
200201
instanceOf.getClazz().toString(),
201202
visit(instanceOf.getExpression(), ctx));
202203
}
203204

205+
/**
206+
* The fully qualified names of the top-level types a type tree names by their simple names.
207+
*
208+
* @param typeTree the type tree
209+
* @return the names for the template to import
210+
*/
211+
private static String[] typesNamedBy(J typeTree) {
212+
Set<String> types = new LinkedHashSet<>();
213+
new JavaIsoVisitor<Set<String>>() {
214+
@Override
215+
public J.FieldAccess visitFieldAccess(J.FieldAccess fieldAccess, Set<String> t) {
216+
// a qualified name is found through its first part
217+
visit(fieldAccess.getTarget(), t);
218+
return fieldAccess;
219+
}
220+
221+
@Override
222+
public J.Identifier visitIdentifier(J.Identifier identifier, Set<String> t) {
223+
JavaType.FullyQualified type = TypeUtils.asFullyQualified(identifier.getType());
224+
if (type != null && !type.getPackageName().isEmpty() &&
225+
identifier.getSimpleName().equals(type.getClassName())) {
226+
t.add(type.getFullyQualifiedName());
227+
}
228+
return identifier;
229+
}
230+
}.visit(typeTree, types);
231+
return types.toArray(new String[0]);
232+
}
233+
204234
/**
205235
* Creates a type cast expression.
206236
*

‎src/main/java/org/openrewrite/staticanalysis/ReplaceTextBlockWithString.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
6565
boolean lastLineIsEmpty = lines[lines.length - 1].isEmpty();
6666
int n = lastLineIsEmpty && lines.length == 1 ? 1 : lines.length - 1;
6767
for (int i = 0; i < n; i++) {
68-
lines[i] += "\\n";
68+
lines[i] += "\n";
6969
}
7070
// Take all lines except the last one if it's empty
7171
// If there is only one line and it's empty, take it as well
@@ -97,7 +97,8 @@ private J.Literal toLiteral(String str) {
9797
}
9898

9999
private String quote(String str) {
100-
return "\"" + str.replace("\"", "\\\"") + "\"";
100+
return "\"" + str.replace("\\", "\\\\").replace("\"", "\\\"")
101+
.replace("\n", "\\n").replace("\r", "\\r") + "\"";
101102
}
102103
});
103104
}

‎src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) {
7171
List<Statement> stmts = b.getStatements();
7272
boolean unnamedAvailable = getCursor().firstEnclosingOrThrow(JavaSourceFile.class)
7373
.getMarkers().findFirst(JavaVersion.class)
74-
.filter(v -> v.getMajorVersion() >= 21)
74+
.filter(v -> v.getMajorVersion() >= 22)
7575
.isPresent();
7676
return b.withStatements(ListUtils.map(stmts, (i, stmt) -> {
7777
if (stmt instanceof J.Try) {
@@ -177,7 +177,7 @@ private static J.Try transform(J.VariableDeclarations varDecl, J.Try tryStmt, bo
177177
String varName = varDecl.getVariables().get(0).getSimpleName();
178178
J.VariableDeclarations resourceDecl = varDecl.withPrefix(Space.EMPTY);
179179
// When the resource is never read inside the try body, name it with the unnamed variable `_`
180-
// (JEP 443 preview in Java 21, finalized as JEP 456 in Java 22) so the rewritten code does
180+
// (JEP 456 in Java 22, a preview in Java 21 that needs --enable-preview) so the rewritten code does
181181
// not emit an unused-variable warning that would fail `-Werror` builds.
182182
if (unnamedAvailable && !isReferenced(varName, tryStmt.getBody())) {
183183
J.VariableDeclarations.NamedVariable namedVar = resourceDecl.getVariables().get(0);

‎src/test/java/org/openrewrite/staticanalysis/CombineSemanticallyEqualCatchBlocksTest.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,40 @@ void method() {
433433
);
434434
}
435435

436+
@Test
437+
void removeRedundantChildClassesOfMultiCatch() {
438+
rewriteRun(
439+
//language=java
440+
java("class A extends BaseException {}"),
441+
//language=java
442+
java("class B extends BaseException {}"),
443+
//language=java
444+
java("class BaseException extends RuntimeException {}"),
445+
//language=java
446+
java(
447+
"""
448+
class Test {
449+
void method() {
450+
try {
451+
} catch (A | B ex) {
452+
} catch (BaseException ex) {
453+
}
454+
}
455+
}
456+
""",
457+
"""
458+
class Test {
459+
void method() {
460+
try {
461+
} catch (BaseException ex) {
462+
}
463+
}
464+
}
465+
"""
466+
)
467+
);
468+
}
469+
436470
@Test
437471
void dontCombineCatchBlocksWithDifferentMethodInvocationParameters() {
438472
rewriteRun(

‎src/test/java/org/openrewrite/staticanalysis/NewStringBuilderBufferWithCharArgumentTest.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,28 @@ class A {
5555
);
5656
}
5757

58+
@SuppressWarnings("NewStringBufferWithCharArgument")
59+
@Test
60+
void quoteCharacters() {
61+
rewriteRun(
62+
//language=java
63+
java(
64+
"""
65+
class A {
66+
StringBuilder doubleQuote = new StringBuilder('"');
67+
StringBuilder singleQuote = new StringBuilder('\\'');
68+
}
69+
""",
70+
"""
71+
class A {
72+
StringBuilder doubleQuote = new StringBuilder("\\"");
73+
StringBuilder singleQuote = new StringBuilder("\\'");
74+
}
75+
"""
76+
)
77+
);
78+
}
79+
5880
@Test
5981
void validStringArg() {
6082
rewriteRun(

0 commit comments

Comments
 (0)