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.

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

Inline Javatemplate in RefasterTemplateProcessor #57

Merged
merged 30 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
2f49868
Inline JavaTemplate into RefasterTemplateProcessor
timtebeek Dec 27, 2023
10d7be3
Adapt Refaster input and output tests
timtebeek Dec 27, 2023
57c8e52
Remove unused method statementType
timtebeek Dec 27, 2023
6ce5044
Resolve parameters, which now triggers NPE
timtebeek Dec 29, 2023
a835294
Merge branch 'main' into inline-javatemplate-in-refaster-template-pro…
timtebeek Jan 2, 2024
0ae56a8
Revert JavacResolution for template parameters
timtebeek Jan 2, 2024
dfe8a22
Change input to ParameterReuseRecipe
timtebeek Jan 2, 2024
5fb131c
Adapt three out of four remaining tests
timtebeek Jan 2, 2024
9bf620b
Fix remaining test
timtebeek Jan 2, 2024
9085931
Merge branch 'main' into inline-javatemplate-in-refaster-template-pro…
timtebeek Jan 2, 2024
01cef4b
Update new test too
timtebeek Jan 2, 2024
ef3d59f
Merge branch 'main' into inline-javatemplate-in-refaster-template-pro…
timtebeek Jan 6, 2024
4491363
Merge branch 'main' into inline-javatemplate-in-refaster-template-pro…
timtebeek Jan 9, 2024
5058d1c
Merge branch 'main' into inline-javatemplate-in-refaster-template-pro…
timtebeek Jan 27, 2024
6d449d7
Merge branch 'main' into inline-javatemplate-in-refaster-template-pro…
timtebeek Feb 2, 2024
a21d6db
Add TODO
timtebeek Jan 6, 2024
676a188
Quick stash
timtebeek Feb 2, 2024
41e95fb
Merge branch 'main' into inline-javatemplate-in-refaster-template-pro…
timtebeek Feb 11, 2024
7cb1bbd
Fix compilation issue
timtebeek Feb 11, 2024
e93e148
Add an inline test for RefaterTemplateProcessor as well
timtebeek Feb 11, 2024
f7c954a
Add FIXME
timtebeek Feb 20, 2024
f31fb31
Merge branch 'main' into inline-javatemplate-in-refaster-template-pro…
timtebeek Mar 3, 2024
a7ac909
Add more analysis of what's going wrong
timtebeek Mar 3, 2024
71603fc
First working version
timtebeek Mar 3, 2024
6f7b6cb
Update reference outputs
timtebeek Mar 3, 2024
a726122
Handle recipes without parameters
timtebeek Mar 3, 2024
261ad17
Drop now unused import
timtebeek Mar 3, 2024
12223ca
Drop inline tests again while text blocks are unavailable
timtebeek Mar 3, 2024
0ea5172
Only pass through matching resolved parameter types
timtebeek Mar 3, 2024
50fdf56
Resolve all before templates at once
timtebeek Mar 3, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ public static <T extends JCTree> String process(T tree, List<JCTree.JCVariableDe
}
}

public static String indent(String code, int width) {
char[] indent = new char[width];
Arrays.fill(indent, ' ');
String replacement = "$1" + new String(indent);
return code.replaceAll("(?m)(\\R)", replacement);
}

private static class TemplateCodePrinter extends Pretty {

private static final String PRIMITIVE_ANNOTATION = "org.openrewrite.java.template.Primitive";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
import com.sun.tools.javac.tree.TreeScanner;
import com.sun.tools.javac.util.Context;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.template.internal.FQNPretty;
import org.openrewrite.java.template.internal.ImportDetector;
import org.openrewrite.java.template.internal.JavacResolution;
import org.openrewrite.java.template.internal.TemplateCode;
import org.openrewrite.java.template.internal.UsedMethodDetector;

import javax.annotation.processing.RoundEnvironment;
Expand Down Expand Up @@ -197,24 +197,18 @@ public void visitClassDef(JCTree.JCClassDecl classDecl) {
recipe.append(" public TreeVisitor<?, ExecutionContext> getVisitor() {\n");
recipe.append(" JavaVisitor<ExecutionContext> javaVisitor = new AbstractRefasterJavaVisitor() {\n");
for (Map.Entry<String, JCTree.JCMethodDecl> entry : beforeTemplates.entrySet()) {
JCTree.JCMethodDecl methodDecl = entry.getValue();
recipe.append(" final JavaTemplate ")
.append(entry.getKey())
.append(" = Semantics.")
.append(statementType(entry.getValue()))
.append("(this, \"")
.append(entry.getKey()).append("\", ")
.append(toLambda(entry.getValue()))
.append(").build();\n");
.append(" = ")
.append(toJavaTemplateBuilder(methodDecl))
.append("\n .build();\n");
}
recipe.append(" final JavaTemplate ")
.append(after)
.append(" = Semantics.")
.append(statementType(descriptor.afterTemplate))
.append("(this, \"")
.append(after)
.append("\", ")
.append(toLambda(descriptor.afterTemplate))
.append(").build();\n");
.append(" = ")
.append(toJavaTemplateBuilder(descriptor.afterTemplate))
.append("\n .build();\n");
recipe.append("\n");

List<String> lstTypes = LST_TYPE_MAP.get(getType(descriptor.beforeTemplates.get(0)));
Expand Down Expand Up @@ -312,6 +306,7 @@ public void visitClassDef(JCTree.JCClassDecl classDecl) {
out.write("import org.openrewrite.Recipe;\n");
out.write("import org.openrewrite.TreeVisitor;\n");
out.write("import org.openrewrite.internal.lang.NonNullApi;\n");
out.write("import org.openrewrite.java.JavaParser;\n");
out.write("import org.openrewrite.java.JavaTemplate;\n");
out.write("import org.openrewrite.java.JavaVisitor;\n");
out.write("import org.openrewrite.java.search.*;\n");
Expand Down Expand Up @@ -381,6 +376,15 @@ public void visitClassDef(JCTree.JCClassDecl classDecl) {
}
}

private String toJavaTemplateBuilder(JCTree.JCMethodDecl methodDecl) {
JCTree tree = methodDecl.getBody().getStatements().get(0);
if (tree instanceof JCTree.JCReturn) {
tree = ((JCTree.JCReturn) tree).getExpression();
}
String javaTemplateBuilder = TemplateCode.process(tree, methodDecl.getParameters(), true);
return TemplateCode.indent(javaTemplateBuilder, 16);
}

private boolean simplifyBooleans(JCTree.JCMethodDecl template) {
if (template.getReturnType().type.getTag() == TypeTag.BOOLEAN) {
return true;
Expand Down Expand Up @@ -631,55 +635,6 @@ private Class<? extends JCTree> getType(JCTree.JCMethodDecl method) {
return type;
}

private String statementType(JCTree.JCMethodDecl method) {
// for now excluding assignment expressions and prefix and postfix -- and ++
Set<Class<? extends JCTree>> expressionStatementTypes = Stream.of(
JCTree.JCMethodInvocation.class,
JCTree.JCNewClass.class).collect(Collectors.toSet());

Class<? extends JCTree> type = getType(method);
if (expressionStatementTypes.contains(type)) {
if (type == JCTree.JCMethodInvocation.class
&& method.getBody().getStatements().last() instanceof JCTree.JCExpressionStatement
&& !(method.getReturnType().type instanceof Type.JCVoidType)) {
return "expression";
}
if (method.restype.type instanceof Type.JCVoidType || !JCTree.JCExpression.class.isAssignableFrom(type)) {
return "statement";
}
}
return "expression";
}

private String toLambda(JCTree.JCMethodDecl method) {
StringBuilder builder = new StringBuilder();

StringJoiner joiner = new StringJoiner(", ", "(", ")");
for (JCTree.JCVariableDecl parameter : method.getParameters()) {
String paramType = parameter.getType().type.toString();
if (!getBoxedPrimitive(paramType).equals(paramType)) {
paramType = "@Primitive " + getBoxedPrimitive(paramType);
} else if (paramType.startsWith("java.lang.")) {
paramType = paramType.substring("java.lang.".length());
}
joiner.add(paramType + " " + parameter.getName());
}
builder.append(joiner);
builder.append(" -> ");

JCTree.JCStatement statement = method.getBody().getStatements().get(0);
if (statement instanceof JCTree.JCReturn) {
builder.append(FQNPretty.toString(((JCTree.JCReturn) statement).getExpression()));
} else if (statement instanceof JCTree.JCThrow) {
String string = FQNPretty.toString(statement);
builder.append("{ ").append(string).append(" }");
} else {
String string = FQNPretty.toString(statement);
builder.append(string);
}
return builder.toString();
}

@Nullable
private TemplateDescriptor getTemplateDescriptor(JCTree.JCClassDecl tree, Context context, JCCompilationUnit cu) {
TemplateDescriptor result = new TemplateDescriptor(tree);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public void visitApply(JCTree.JCMethodInvocation tree) {
out.write(" * @return the JavaTemplate builder.\n");
out.write(" */\n");
out.write(" public static JavaTemplate.Builder getTemplate() {\n");
out.write(" return " + indent(templateCode, 12) + ";\n");
out.write(" return " + TemplateCode.indent(templateCode, 12) + ";\n");
out.write(" }\n");
out.write("}\n");
out.flush();
Expand All @@ -181,13 +181,6 @@ public void visitApply(JCTree.JCMethodInvocation tree) {

super.visitApply(tree);
}

private String indent(String code, int width) {
char[] indent = new char[width];
Arrays.fill(indent, ' ');
String replacement = "$1" + new String(indent);
return code.replaceAll("(?m)(\\R)", replacement);
}
}.scan(cu);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class TemplateProcessorTest {
})
void qualification(String qualifier) {
// As per https://github.com/google/compile-testing/blob/v0.21.0/src/main/java/com/google/testing/compile/package-info.java#L53-L55
Compilation compilation = compile("template/ShouldAddClasspath.java");
Compilation compilation = compile("template/ShouldAddClasspathRecipes.java");
assertThat(compilation).succeeded();
assertThat(compilation)
.generatedSourceFile("foo/ShouldAddClasspathRecipes$" + qualifier + "Recipe$1_before")
Expand All @@ -47,7 +47,7 @@ void qualification(String qualifier) {

@Test
void parameterReuse() {
Compilation compilation = compile("template/ParameterReuse.java");
Compilation compilation = compile("template/ParameterReuseRecipe.java");
assertThat(compilation).succeeded();
assertThat(compilation)
.generatedSourceFile("foo/ParameterReuseRecipe$1_before")
Expand Down
19 changes: 15 additions & 4 deletions src/test/resources/refaster/ArraysRecipe.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.lang.NonNullApi;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.search.*;
Expand All @@ -33,12 +34,18 @@

import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.*;


/**
* OpenRewrite recipe created for Refaster template {@code Arrays}.
*/
@SuppressWarnings("all")
@NonNullApi
public class ArraysRecipe extends Recipe {

public ArraysRecipe() {
}
/**
* Instantiates a new instance.
*/
public ArraysRecipe() {}

@Override
public String getDisplayName() {
Expand All @@ -53,8 +60,12 @@ public String getDescription() {
@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
JavaVisitor<ExecutionContext> javaVisitor = new AbstractRefasterJavaVisitor() {
final JavaTemplate before = Semantics.expression(this, "before", (String[] strings) -> String.join(", ", strings)).build();
final JavaTemplate after = Semantics.expression(this, "after", (String[] strings) -> String.join(":", strings)).build();
final JavaTemplate before = JavaTemplate
.builder("String.join(\", \", foo.Arrays.before.strings)")
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
.build();
final JavaTemplate after = JavaTemplate
.builder("String.join(\":\", foo.Arrays.after.strings)")
.build();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only downside I see with this refactoring is that the generated code is now harder to read and that the Semantics mechanism isn't visible anymore. But I think that is acceptable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the main motivator for me was PRs like this one that aim to use Java 9+ constructs in recipes, which previously meant that code was in the generated recipes as well(likely?) bumping the Java version required to run the recipe.

With this change I think we should be good to have the generated recipes included with the other Java 8 compatible classes.


@Override
public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) {
Expand Down
42 changes: 37 additions & 5 deletions src/test/resources/refaster/EscapesRecipes.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package foo;

import 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.JavaParser;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.search.*;
Expand All @@ -36,8 +38,14 @@
import com.sun.tools.javac.util.Convert;
import com.sun.tools.javac.util.Constants;

/**
* OpenRewrite recipes created for Refaster template {@code foo.Escapes}.
*/
@SuppressWarnings("all")
public class EscapesRecipes extends Recipe {
/**
* Instantiates a new instance.
*/
public EscapesRecipes() {}

@Override
Expand All @@ -58,9 +66,16 @@ public List<Recipe> getRecipeList() {
);
}

/**
* OpenRewrite recipe created for Refaster template {@code Escapes.ConstantsFormat}.
*/
@SuppressWarnings("all")
@NonNullApi
public static class ConstantsFormatRecipe extends Recipe {

/**
* Instantiates a new instance.
*/
public ConstantsFormatRecipe() {}

@Override
Expand All @@ -76,8 +91,14 @@ public String getDescription() {
@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
JavaVisitor<ExecutionContext> javaVisitor = new AbstractRefasterJavaVisitor() {
final JavaTemplate before = Semantics.expression(this, "before", (String value) -> String.format("\"%s\"", com.sun.tools.javac.util.Convert.quote(value))).build();
final JavaTemplate after = Semantics.expression(this, "after", (String value) -> com.sun.tools.javac.util.Constants.format(value)).build();
final JavaTemplate before = JavaTemplate
.builder("String.format(\"\\\"%s\\\"\", com.sun.tools.javac.util.Convert.quote(foo.Escapes.ConstantsFormat.before.value))")
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
.javaParser(JavaParser.fromJavaVersion().classpath("tools"))
.build();
final JavaTemplate after = JavaTemplate
.builder("com.sun.tools.javac.util.Constants.format(foo.Escapes.ConstantsFormat.after.value)")
.javaParser(JavaParser.fromJavaVersion().classpath("tools"))
.build();

@Override
public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) {
Expand Down Expand Up @@ -106,9 +127,16 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) {
}
}

/**
* OpenRewrite recipe created for Refaster template {@code Escapes.Split}.
*/
@SuppressWarnings("all")
@NonNullApi
public static class SplitRecipe extends Recipe {

/**
* Instantiates a new instance.
*/
public SplitRecipe() {}

@Override
Expand All @@ -124,8 +152,12 @@ public String getDescription() {
@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
JavaVisitor<ExecutionContext> javaVisitor = new AbstractRefasterJavaVisitor() {
final JavaTemplate before = Semantics.expression(this, "before", (String s) -> s.split("[^\\S]+")).build();
final JavaTemplate after = Semantics.expression(this, "after", (String s) -> s.split("\\s+")).build();
final JavaTemplate before = JavaTemplate
.builder("foo.Escapes.Split.before.s.split(\"[^\\\\S]+\")")
.build();
final JavaTemplate after = JavaTemplate
.builder("foo.Escapes.Split.after.s.split(\"\\\\s+\")")
.build();

@Override
public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) {
Expand All @@ -149,4 +181,4 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) {
}
}

}
}
27 changes: 23 additions & 4 deletions src/test/resources/refaster/MatchingRecipes.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.lang.NonNullApi;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.search.*;
Expand All @@ -33,9 +34,15 @@

import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.*;


/**
* OpenRewrite recipes created for Refaster template {@code foo.Matching}.
*/
@SuppressWarnings("all")
public class MatchingRecipes extends Recipe {

/**
* Instantiates a new instance.
*/
public MatchingRecipes() {}

@Override
Expand All @@ -60,10 +67,16 @@ public List<Recipe> getRecipeList() {
);
}

/**
* OpenRewrite recipe created for Refaster template {@code Matching.StringIsEmpty}.
*/
@SuppressWarnings("all")
@NonNullApi
public static class StringIsEmptyRecipe extends Recipe {

/**
* Instantiates a new instance.
*/
public StringIsEmptyRecipe() {}

@Override
Expand All @@ -84,9 +97,15 @@ public Set<String> getTags() {
@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
JavaVisitor<ExecutionContext> javaVisitor = new AbstractRefasterJavaVisitor() {
final JavaTemplate before = Semantics.expression(this, "before", (@Primitive Integer i, String s) -> s.substring(i).isEmpty()).build();
final JavaTemplate before2 = Semantics.expression(this, "before2", (@Primitive Integer i, String s) -> s.substring(i).isEmpty()).build();
final JavaTemplate after = Semantics.expression(this, "after", (String s) -> (s != null && s.length() == 0)).build();
final JavaTemplate before = JavaTemplate
.builder("foo.Matching.StringIsEmpty.before.s.substring(foo.Matching.StringIsEmpty.before.i).isEmpty()")
.build();
final JavaTemplate before2 = JavaTemplate
.builder("foo.Matching.StringIsEmpty.before2.s.substring(foo.Matching.StringIsEmpty.before2.i).isEmpty()")
.build();
final JavaTemplate after = JavaTemplate
.builder("(foo.Matching.StringIsEmpty.after.s != null && foo.Matching.StringIsEmpty.after.s.length() == 0)")
.build();

@Override
public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) {
Expand Down
Loading