Skip to content

Commit c615f61

Browse files
sullissullis
authored
Merge branch 'main' into cleanup-try-with-resources
2 parents e14f36f + f357fcb commit c615f61

6 files changed

Lines changed: 228 additions & 14 deletions

File tree

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

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import org.openrewrite.staticanalysis.javascript.JavascriptFileChecker;
3030

3131
import java.util.ArrayList;
32+
import java.util.Collections;
33+
import java.util.IdentityHashMap;
3234
import java.util.List;
3335
import java.util.Set;
3436

@@ -80,15 +82,15 @@ private J.VariableDeclarations maybeAddTypeExpression(J.VariableDeclarations mul
8082
// if the type expression is null, it implies the types on the lambda arguments are implicit.
8183
if (multiVariable.getTypeExpression() == null) {
8284
J.VariableDeclarations.NamedVariable nv = multiVariable.getVariables().get(0);
83-
TypeTree typeExpression = buildTypeTree(nv.getType(), Space.EMPTY);
85+
TypeTree typeExpression = buildTypeTree(nv.getType(), Space.EMPTY, newRecursionGuard());
8486
if (typeExpression != null) {
8587
// "? extends Foo" is not a valid type definition on its own. Unwrap wildcard and replace with its bound
8688
if (typeExpression instanceof J.Wildcard) {
8789
J.Wildcard wildcard = (J.Wildcard) typeExpression;
8890
if (wildcard.getBoundedType() == null) {
8991
return multiVariable;
9092
}
91-
typeExpression = buildTypeTree(wildcard.getBoundedType().getType(), Space.EMPTY);
93+
typeExpression = buildTypeTree(wildcard.getBoundedType().getType(), Space.EMPTY, newRecursionGuard());
9294
}
9395
multiVariable = multiVariable.withTypeExpression(typeExpression);
9496
multiVariable = multiVariable.withVariables(ListUtils.map(multiVariable.getVariables(), (index, variable) -> {
@@ -102,10 +104,25 @@ private J.VariableDeclarations maybeAddTypeExpression(J.VariableDeclarations mul
102104
return multiVariable;
103105
}
104106

105-
private @Nullable TypeTree buildTypeTree(@Nullable JavaType type, Space space) {
107+
private Set<JavaType> newRecursionGuard() {
108+
return Collections.newSetFromMap(new IdentityHashMap<>());
109+
}
110+
111+
private @Nullable TypeTree buildTypeTree(@Nullable JavaType type, Space space, Set<JavaType> recursionGuard) {
106112
if (type == null || type instanceof JavaType.Unknown) {
107113
return null;
108114
}
115+
if (!recursionGuard.add(type)) {
116+
return null;
117+
}
118+
try {
119+
return buildTypeTree0(type, space, recursionGuard);
120+
} finally {
121+
recursionGuard.remove(type);
122+
}
123+
}
124+
125+
private @Nullable TypeTree buildTypeTree0(JavaType type, Space space, Set<JavaType> recursionGuard) {
109126
if (type instanceof JavaType.Primitive) {
110127
return new J.Primitive(
111128
Tree.randomId(),
@@ -128,7 +145,7 @@ private J.VariableDeclarations maybeAddTypeExpression(J.VariableDeclarations mul
128145
);
129146

130147
if (!fq.getTypeParameters().isEmpty()) {
131-
JContainer<Expression> typeParameters = buildTypeParameters(fq.getTypeParameters());
148+
JContainer<Expression> typeParameters = buildTypeParameters(fq.getTypeParameters(), recursionGuard);
132149
if (typeParameters == null) {
133150
//If there is a problem resolving one of the type parameters, then do not return a type
134151
//expression for the fully-qualified type.
@@ -156,7 +173,7 @@ private J.VariableDeclarations maybeAddTypeExpression(J.VariableDeclarations mul
156173
}
157174

158175
// Build the base type expression
159-
TypeTree result = buildTypeTree(elemType, space);
176+
TypeTree result = buildTypeTree(elemType, space, recursionGuard);
160177
if (result == null) {
161178
return null;
162179
}
@@ -178,7 +195,7 @@ private J.VariableDeclarations maybeAddTypeExpression(J.VariableDeclarations mul
178195
return result;
179196
}
180197
if (type instanceof JavaType.Variable) {
181-
return buildTypeTree(((JavaType.Variable) type).getType(), space);
198+
return buildTypeTree(((JavaType.Variable) type).getType(), space, recursionGuard);
182199
}
183200
if (type instanceof JavaType.GenericTypeVariable) {
184201
JavaType.GenericTypeVariable genericType = (JavaType.GenericTypeVariable) type;
@@ -202,7 +219,7 @@ private J.VariableDeclarations maybeAddTypeExpression(J.VariableDeclarations mul
202219
}
203220

204221
if (!genericType.getBounds().isEmpty()) {
205-
boundedType = buildTypeTree(genericType.getBounds().get(0), Space.format(" "));
222+
boundedType = buildTypeTree(genericType.getBounds().get(0), Space.format(" "), recursionGuard);
206223
if (boundedType == null) {
207224
return null;
208225
}
@@ -219,11 +236,11 @@ private J.VariableDeclarations maybeAddTypeExpression(J.VariableDeclarations mul
219236
return null;
220237
}
221238

222-
private @Nullable JContainer<Expression> buildTypeParameters(List<JavaType> typeParameters) {
239+
private @Nullable JContainer<Expression> buildTypeParameters(List<JavaType> typeParameters, Set<JavaType> recursionGuard) {
223240
List<JRightPadded<Expression>> typeExpressions = new ArrayList<>();
224241

225242
for (JavaType type : typeParameters) {
226-
Expression typeParameterExpression = (Expression) buildTypeTree(type, Space.EMPTY);
243+
Expression typeParameterExpression = (Expression) buildTypeTree(type, Space.EMPTY, recursionGuard);
227244
if (typeParameterExpression == null) {
228245
return null;
229246
}

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,22 @@
1919
import org.openrewrite.ExecutionContext;
2020
import org.openrewrite.Recipe;
2121
import org.openrewrite.internal.ListUtils;
22+
import org.openrewrite.internal.ReflectionUtils;
2223
import org.openrewrite.java.JavaIsoVisitor;
24+
import org.openrewrite.java.tree.Comment;
2325
import org.openrewrite.java.tree.J;
2426
import org.openrewrite.java.tree.Space;
2527
import org.openrewrite.java.tree.Statement;
28+
import org.openrewrite.python.tree.Py;
29+
30+
import java.util.List;
2631

2732
import static org.openrewrite.java.format.ShiftFormat.indent;
2833

2934
public class SimplifyElseBranch extends Recipe {
3035

36+
private static final boolean IS_PYTHON_AVAILABLE = ReflectionUtils.isClassAvailable("org.openrewrite.python.tree.Py");
37+
3138
@Getter
3239
final String displayName = "Simplify `else` branch if it only has a single `if`";
3340

@@ -46,10 +53,20 @@ public J.If.Else visitElse(J.If.Else else_, ExecutionContext ctx) {
4653
if (block.getStatements().size() == 1) {
4754
Statement firstStatement = block.getStatements().get(0);
4855
if (firstStatement instanceof J.If) {
56+
List<Comment> comments = ListUtils.concatAll(block.getComments(), firstStatement.getComments());
57+
if (IS_PYTHON_AVAILABLE && getCursor().firstEnclosing(Py.CompilationUnit.class) != null) {
58+
// Python renders `else` + `if` as a single `elif` keyword, so the `if` keeps an empty
59+
// prefix and any comments move ahead of it, onto the `elif` line
60+
Space elsePrefix = elseStatement.getPrefix();
61+
Space withComments = elsePrefix.withComments(ListUtils.concatAll(elsePrefix.getComments(),
62+
ListUtils.map(comments, c -> c.withSuffix(elsePrefix.getWhitespace()))));
63+
J.If ifStatement = firstStatement.withPrefix(Space.EMPTY);
64+
return elseStatement.withPrefix(withComments).withBody(indent(ifStatement, getCursor(), -1));
65+
}
4966
// Combine comments from the block and the if statement
5067
J.If ifStatement = firstStatement
5168
.withPrefix(Space.SINGLE_SPACE)
52-
.withComments(ListUtils.concatAll(block.getComments(), firstStatement.getComments()));
69+
.withComments(comments);
5370
return elseStatement.withBody(indent(ifStatement, getCursor(), -1));
5471
}
5572
}

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,11 @@ private boolean methodArgumentRequiresCast(J.Lambda lambda, MethodCall method, i
246246
if (methodType == null) {
247247
return false;
248248
}
249-
if (!TypeUtils.isOfClassType(methodType.getParameterTypes().get(argumentIndex), lambdaFqn)) {
249+
JavaType parameterType = parameterTypeAt(methodType, argumentIndex);
250+
if (parameterType == null) {
251+
return false;
252+
}
253+
if (!TypeUtils.isOfClassType(parameterType, lambdaFqn)) {
250254
return true;
251255
}
252256

@@ -256,8 +260,8 @@ private boolean methodArgumentRequiresCast(J.Lambda lambda, MethodCall method, i
256260
if (methodType.getName().equals(maybeAmbiguous.getName()) &&
257261
methodType.getParameterTypes().size() == maybeAmbiguous.getParameterTypes().size()) {
258262
if (areMethodsAmbiguous(
259-
getSamCompatible(methodType.getParameterTypes().get(argumentIndex)),
260-
getSamCompatible(maybeAmbiguous.getParameterTypes().get(argumentIndex)))) {
263+
getSamCompatible(parameterType),
264+
getSamCompatible(parameterTypeAt(maybeAmbiguous, argumentIndex)))) {
261265
count++;
262266
}
263267
}
@@ -269,6 +273,22 @@ private boolean methodArgumentRequiresCast(J.Lambda lambda, MethodCall method, i
269273
return hasGenerics(lambda);
270274
}
271275

276+
private @Nullable JavaType parameterTypeAt(JavaType.Method methodType, int argumentIndex) {
277+
List<JavaType> parameterTypes = methodType.getParameterTypes();
278+
if (parameterTypes.isEmpty()) {
279+
return null;
280+
}
281+
int index = Math.min(argumentIndex, parameterTypes.size() - 1);
282+
JavaType parameterType = parameterTypes.get(index);
283+
if (index == parameterTypes.size() - 1) {
284+
JavaType.Array array = TypeUtils.asArray(parameterType);
285+
if (array != null) {
286+
return array.getElemType();
287+
}
288+
}
289+
return parameterType;
290+
}
291+
272292
private boolean areMethodsAmbiguous(JavaType.@Nullable Method m1, JavaType.@Nullable Method m2) {
273293
if (m1 == null || m2 == null || m1.getParameterTypes().size() != m2.getParameterTypes().size()) {
274294
return false;

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.junit.jupiter.api.Test;
1919
import org.openrewrite.DocumentExample;
2020
import org.openrewrite.Issue;
21+
import org.openrewrite.java.JavaParser;
2122
import org.openrewrite.test.RecipeSpec;
2223
import org.openrewrite.test.RewriteTest;
2324
import org.openrewrite.test.TypeValidation;
@@ -700,6 +701,35 @@ void foo(List<? extends A> a) {
700701
);
701702
}
702703

704+
@Test
705+
void doNotStackOverflowOnRecursiveGenericType() {
706+
rewriteRun(
707+
spec -> spec.parser(JavaParser.fromJavaVersion().classpath("assertj-core"))
708+
.typeValidationOptions(TypeValidation.none()),
709+
//language=java
710+
java(
711+
"""
712+
import org.assertj.core.api.AbstractStringAssert;
713+
714+
class Test {
715+
static <C> void inject(C carrier, Setter<C> setter) {
716+
}
717+
718+
interface Setter<C> {
719+
void set(C carrier);
720+
}
721+
722+
static void method(AbstractStringAssert<?> assertion) {
723+
inject(assertion, a -> {
724+
a.isNotNull();
725+
});
726+
}
727+
}
728+
"""
729+
)
730+
);
731+
}
732+
703733
@Test
704734
void doNotFailOnTypeScriptArrowFunction() {
705735
rewriteRun(

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

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.openrewrite.test.RewriteTest;
2323

2424
import static org.openrewrite.java.Assertions.java;
25+
import static org.openrewrite.python.Assertions.python;
2526

2627
class SimplifyElseBranchTest implements RewriteTest {
2728

@@ -344,4 +345,71 @@ else if (password.length() > 12)
344345
"""
345346
)
346347
);
347-
}}
348+
}
349+
350+
@Test
351+
void simplifyElseBranchPython() {
352+
rewriteRun(
353+
//language=python
354+
python(
355+
"""
356+
def a(password):
357+
if len(password) < 6:
358+
print("Password is too short.")
359+
else:
360+
if len(password) > 12:
361+
print("Password is too long.")
362+
""",
363+
"""
364+
def a(password):
365+
if len(password) < 6:
366+
print("Password is too short.")
367+
elif len(password) > 12:
368+
print("Password is too long.")
369+
"""
370+
)
371+
);
372+
}
373+
374+
@Test
375+
void simplifyElseBranchPythonWithComments() {
376+
rewriteRun(
377+
//language=python
378+
python(
379+
"""
380+
def a(password):
381+
if len(password) < 6:
382+
print("Password is too short.")
383+
else:
384+
# Comment 1
385+
if len(password) > 12:
386+
print("Password is too long.")
387+
""",
388+
"""
389+
def a(password):
390+
if len(password) < 6:
391+
print("Password is too short.")
392+
# Comment 1
393+
elif len(password) > 12:
394+
print("Password is too long.")
395+
"""
396+
)
397+
);
398+
}
399+
400+
@Test
401+
void doNotChangeExistingElifPython() {
402+
rewriteRun(
403+
//language=python
404+
python(
405+
"""
406+
def a(password):
407+
if len(password) < 6:
408+
print("Password is too short.")
409+
elif len(password) > 12:
410+
print("Password is too long.")
411+
"""
412+
)
413+
);
414+
}
415+
}

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,68 @@ void test() {
116116
);
117117
}
118118

119+
@Test
120+
void varargsArgumentAfterFixedArguments() {
121+
rewriteRun(
122+
//language=java
123+
java(
124+
"""
125+
class Test {
126+
interface I { int run(); }
127+
static void of(Object... args) {}
128+
void test(Object first) {
129+
of(first, new I() {
130+
@Override public int run() {
131+
return 0;
132+
}
133+
});
134+
}
135+
}
136+
""",
137+
"""
138+
class Test {
139+
interface I { int run(); }
140+
static void of(Object... args) {}
141+
void test(Object first) {
142+
of(first, (I) () -> 0);
143+
}
144+
}
145+
"""
146+
)
147+
);
148+
}
149+
150+
@Test
151+
void functionalInterfaceVarargsNeedsNoCast() {
152+
rewriteRun(
153+
//language=java
154+
java(
155+
"""
156+
class Test {
157+
interface I { int run(); }
158+
static void of(I... args) {}
159+
void test() {
160+
of(new I() {
161+
@Override public int run() {
162+
return 0;
163+
}
164+
});
165+
}
166+
}
167+
""",
168+
"""
169+
class Test {
170+
interface I { int run(); }
171+
static void of(I... args) {}
172+
void test() {
173+
of(() -> 0);
174+
}
175+
}
176+
"""
177+
)
178+
);
179+
}
180+
119181
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/194")
120182
@SuppressWarnings("ConstantConditions")
121183
@Test

0 commit comments

Comments
 (0)