Fix ClassCastException on J.Ternary visitor#3861
Fix ClassCastException on J.Ternary visitor#3861ammachado wants to merge 1 commit intoopenrewrite:mainfrom
ClassCastException on J.Ternary visitor#3861Conversation
|
Interesting. I don't think we should have to change the |
|
Figured quickly share what recipe classes are generated for SimplifyTernaryRecipesimport org.openrewrite.ExecutionContext;
import org.openrewrite.Preconditions;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.lang.NonNullApi;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.search.*;
import org.openrewrite.java.template.Primitive;
import org.openrewrite.java.template.Semantics;
import org.openrewrite.java.template.function.*;
import org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor;
import org.openrewrite.java.tree.*;
import java.util.*;
import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.*;
/**
* OpenRewrite recipes created for Refaster template {@code org.openrewrite.java.migrate.SimplifyTernary}.
*/
@SuppressWarnings("all")
public class SimplifyTernaryRecipes extends Recipe {
/**
* Instantiates a new instance.
*/
public SimplifyTernaryRecipes() {}
@Override
public String getDisplayName() {
return "`SimplifyTernary` Refaster recipes";
}
@Override
public String getDescription() {
return "Refaster template recipes for `org.openrewrite.java.migrate.SimplifyTernary`.";
}
@Override
public List<Recipe> getRecipeList() {
return Arrays.asList(
new SimplifyTernaryTrueFalseRecipe()
);
}
/**
* OpenRewrite recipe created for Refaster template {@code SimplifyTernary.SimplifyTernaryTrueFalse}.
*/
@SuppressWarnings("all")
@NonNullApi
public static class SimplifyTernaryTrueFalseRecipe extends Recipe {
/**
* Instantiates a new instance.
*/
public SimplifyTernaryTrueFalseRecipe() {}
@Override
public String getDisplayName() {
return "Simplify ternaries";
}
@Override
public String getDescription() {
return "Simplify `expr ? true : false` to `expr`.";
}
@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
JavaVisitor<ExecutionContext> javaVisitor = new AbstractRefasterJavaVisitor() {
final JavaTemplate before = Semantics.expression(this, "before", (@Primitive Boolean expr) -> expr ? true : false).build();
final JavaTemplate after = Semantics.expression(this, "after", (@Primitive Boolean expr) -> expr).build();
@Override
public J visitExpression(Expression elem, ExecutionContext ctx) {
JavaTemplate.Matcher matcher;
if ((matcher = before.matcher(getCursor())).find()) {
return embed(
after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)),
getCursor(),
ctx,
SHORTEN_NAMES, SIMPLIFY_BOOLEANS
);
}
return super.visitExpression(elem, ctx);
}
};
return javaVisitor;
}
}
}SimplifyTernaryRecipes$SimplifyTernaryTrueFalseRecipe$1_beforeimport org.openrewrite.java.*;
/**
* OpenRewrite `before` template created for {@code org.openrewrite.java.migrate.SimplifyTernaryRecipes$SimplifyTernaryTrueFalseRecipe$1}.
*/
@SuppressWarnings("all")
public class SimplifyTernaryRecipes$SimplifyTernaryTrueFalseRecipe$1_before {
/**
* Instantiates a new instance.
*/
public SimplifyTernaryRecipes$SimplifyTernaryTrueFalseRecipe$1_before() {}
/**
* Get the {@code JavaTemplate.Builder} to match or replace.
* @return the JavaTemplate builder.
*/
public static JavaTemplate.Builder getTemplate() {
return JavaTemplate
.builder("#{expr:any(boolean)} ? true : false");
}
}SimplifyTernaryRecipes$SimplifyTernaryTrueFalseRecipe$1_afterimport org.openrewrite.java.*;
/**
* OpenRewrite `after` template created for {@code org.openrewrite.java.migrate.SimplifyTernaryRecipes$SimplifyTernaryTrueFalseRecipe$1}.
*/
@SuppressWarnings("all")
public class SimplifyTernaryRecipes$SimplifyTernaryTrueFalseRecipe$1_after {
/**
* Instantiates a new instance.
*/
public SimplifyTernaryRecipes$SimplifyTernaryTrueFalseRecipe$1_after() {}
/**
* Get the {@code JavaTemplate.Builder} to match or replace.
* @return the JavaTemplate builder.
*/
public static JavaTemplate.Builder getTemplate() {
return JavaTemplate
.builder("#{expr:any(boolean)}");
}
}Hadn't expected any of this to trigger a class cast exception indeed, as I still think this issue is a good candidate for Refaster. |
| t = t.withPrefix(visitSpace(t.getPrefix(), Space.Location.TERNARY_PREFIX, p)); | ||
| t = t.withMarkers(visitMarkers(t.getMarkers(), p)); | ||
| Expression temp = (Expression) visitExpression(t, p); | ||
| J temp = visitExpression(t, p); |
There was a problem hiding this comment.
This is not required, since J.Literal implements Expression
|
This should help: #3867 |
|
@ammachado Maybe you can check if #3867 fixes the problem for you and then we could close this PR. |
|
This fixes it. Thanks. |
|
Thanks for confirming! Guess we now can indeed add rewrite-templating to rewrite-static-analysis to implement this recipe. |
What's changed?
Changed
JavaVisitor#visitTernaryto allow simplification of ternary expressions.What's your motivation?
I tried to implement this issue to explore
rewrite-templating, and used the following implementation:With the following test case:
Anything in particular you'd like reviewers to focus on?
Test passes, but the change might break something else.
Anyone you would like to review specifically?
No one.
Have you considered any alternatives or workarounds?
Implement the rule as a regular visitor, instead of a Refaster template.
Any additional context
Here's the exception trace for the test:
Checklist