Skip to content

Commit

Permalink
Fix Control Flow when types are absent (openrewrite#2096)
Browse files Browse the repository at this point in the history
  • Loading branch information
JLLeitschuh authored Aug 3, 2022
1 parent f120b50 commit b9a96a2
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public static Optional<Guard> from(Cursor cursor) {
return Optional.empty();
}
}
return getTypeSafe(e)
return getTypeSafe(cursor, e)
.map(type -> {
if (TypeUtils.isAssignableTo(JavaType.Primitive.Boolean, type)) {
return new Guard(cursor, e);
Expand Down Expand Up @@ -104,7 +104,7 @@ private static boolean isGuardType(Expression e) {
(e instanceof J.Unary && ((J.Unary) e).getOperator() == J.Unary.Type.Not);
}

private static Optional<JavaType> getTypeSafe(Expression e) {
private static Optional<JavaType> getTypeSafe(Cursor c, Expression e) {
JavaType type = e.getType();
if (type != null && !JavaType.Unknown.getInstance().equals(type)) {
return Optional.of(type);
Expand All @@ -116,12 +116,74 @@ private static Optional<JavaType> getTypeSafe(Expression e) {
case Or:
case Equal:
case NotEqual:
case LessThan:
case LessThanOrEqual:
case GreaterThan:
case GreaterThanOrEqual:
return Optional.of(JavaType.Primitive.Boolean);
default:
return Optional.empty();
break;
}
} else if (e instanceof J.InstanceOf) {
return Optional.of(JavaType.Primitive.Boolean);
} else if (e instanceof J.Unary) {
J.Unary unary = (J.Unary) e;
if (unary.getOperator() == J.Unary.Type.Not) {
return Optional.of(JavaType.Primitive.Boolean);
}
} else if (e instanceof J.MethodInvocation) {
J.MethodInvocation methodInvocation = (J.MethodInvocation) e;
if (methodInvocation.getSimpleName().equals("equals")) {
return Optional.of(JavaType.Primitive.Boolean);
}
}
J firstEnclosing = c.getParentOrThrow().firstEnclosing(J.class);
if (firstEnclosing instanceof J.Binary) {
J.Binary binary = (J.Binary) firstEnclosing;
if (binary.getLeft() == e || binary.getRight() == e) {
switch (binary.getOperator()) {
case And:
case Or:
return Optional.of(JavaType.Primitive.Boolean);
default:
break;
}
}
} else if (firstEnclosing instanceof J.Unary) {
J.Unary unary = (J.Unary) firstEnclosing;
if (unary.getExpression() == e && unary.getOperator() == J.Unary.Type.Not) {
return Optional.of(JavaType.Primitive.Boolean);
}
} else if (firstEnclosing instanceof J.Ternary) {
J.Ternary ternary = (J.Ternary) firstEnclosing;
if (ternary.getCondition() == e) {
return Optional.of(JavaType.Primitive.Boolean);
}
} else if (firstEnclosing instanceof J.ControlParentheses) {
J.ControlParentheses<?> controlParentheses = (J.ControlParentheses<?>) firstEnclosing;
J.If ifStatement = c.getParentOrThrow().firstEnclosing(J.If.class);
if (controlParentheses.getTree() == e && ifStatement != null && ifStatement.getIfCondition() == controlParentheses) {
return Optional.of(JavaType.Primitive.Boolean);
} else {
Cursor parent = c.dropParentUntil(J.class::isInstance);
return getTypeSafe(parent, parent.getValue());
}
} else if (firstEnclosing instanceof J.Parentheses) {
J.Parentheses<?> parentheses = (J.Parentheses<?>) firstEnclosing;
if (parentheses.getTree() == e) {
Cursor parent = c.dropParentUntil(J.class::isInstance);
return getTypeSafe(parent, parent.getValue());
}
} else if (firstEnclosing instanceof J.VariableDeclarations.NamedVariable) {
J.VariableDeclarations.NamedVariable namedVariable = (J.VariableDeclarations.NamedVariable) firstEnclosing;
if (namedVariable.getInitializer() == e) {
return Optional.ofNullable(namedVariable.getType());
}
} else if (firstEnclosing instanceof J.Assignment) {
J.Assignment assignment = (J.Assignment) firstEnclosing;
if (assignment.getAssignment() == e) {
return Optional.ofNullable(assignment.getType());
}
}
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package org.openrewrite.java.controlflow
import org.junit.jupiter.api.Test
import org.openrewrite.ExecutionContext
import org.openrewrite.java.JavaIsoVisitor
import org.openrewrite.java.TypeValidation
import org.openrewrite.java.tree.Expression
import org.openrewrite.test.RecipeSpec
import org.openrewrite.test.RewriteTest
Expand Down Expand Up @@ -177,4 +178,103 @@ interface GuardTest : RewriteTest {
"""
)
)

@Test
fun `identifies guards with missing type information`() = rewriteRun(
{ spec -> spec.typeValidationOptions(TypeValidation.none()) },
java(
"""
class Test {
void test() {
if (potato) {
// ...
}
if ((potato)) {
// ...
}
if (potato && turnip) {
// ...
}
if (potato && turnip || squash) {
// ...
}
int a = 1, b = 2;
if ((a = turnip) == b) {
// ..
}
horse.equals(donkey);
boolean farmFresh = tomato;
boolean farmFreshAndFancyFree = (chicken);
boolean farmFreshEggs = true;
farmFreshEggs = chicken.layEggs();
}
}
""".trimIndent(),
"""
class Test {
void test() {
if (/*~~>*/potato) {
// ...
}
if (/*~~>*/(/*~~>*/potato)) {
// ...
}
if (/*~~>*//*~~>*/potato && /*~~>*/turnip) {
// ...
}
if (/*~~>*//*~~>*//*~~>*/potato && /*~~>*/turnip || /*~~>*/squash) {
// ...
}
int a = 1, b = 2;
if (/*~~>*/(a = turnip) == b) {
// ..
}
/*~~>*/horse.equals(donkey);
/*~~>*/boolean /*~~>*/farmFresh = /*~~>*/tomato;
/*~~>*/boolean /*~~>*/farmFreshAndFancyFree = /*~~>*/(/*~~>*/chicken);
/*~~>*/boolean /*~~>*/farmFreshEggs = /*~~>*/true;
/*~~>*//*~~>*/farmFreshEggs = /*~~>*/chicken.layEggs();
}
}
""".trimIndent()
)
)

@Test
fun `identifies guards for control parentheses with missing type information`() = rewriteRun(
java(
"""
class Test {
void test() {
if ((potato)) {
// ...
}
}
}
""".trimIndent(),
"""
class Test {
void test() {
if (/*~~>*/(/*~~>*/potato)) {
// ...
}
}
}
""".trimIndent()
)
)

@Test
fun `does not flag arbitrary parentheses as guards`() = rewriteRun(
java(
"""
class Test {
void test() {
int a = (potato);
int b = (a = turnip);
}
}
""".trimIndent()
)
)
}

0 comments on commit b9a96a2

Please sign in to comment.