From 492802360708ad80eed9523d0a88829bc5d07397 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 16 Dec 2023 16:56:56 +0100 Subject: [PATCH 1/4] Test problematic OrOrElseThrow --- .../RefasterTemplateProcessorTest.java | 1 + .../resources/refaster/OrOrElseThrow.java | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 src/test/resources/refaster/OrOrElseThrow.java diff --git a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java index 07e12fc7..19b8d80c 100644 --- a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java +++ b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java @@ -57,6 +57,7 @@ void generateRecipe(String recipeName) { @ParameterizedTest @ValueSource(strings = { "OrElseGetGet", + "OrOrElseThrow", "RefasterAnyOf", }) void skipRecipeGeneration(String recipeName){ diff --git a/src/test/resources/refaster/OrOrElseThrow.java b/src/test/resources/refaster/OrOrElseThrow.java new file mode 100644 index 00000000..bdd6211a --- /dev/null +++ b/src/test/resources/refaster/OrOrElseThrow.java @@ -0,0 +1,33 @@ +/* + * Copyright 2023 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package foo; + +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; + +import java.util.Optional; + +class OrOrElseThrow { + @BeforeTemplate + T before(Optional o1, Optional o2) { + return o1.orElseGet(() -> o2.orElseThrow()); + } + + @AfterTemplate + T after(Optional o1, Optional o2) { + return o1.or(() -> o2).orElseThrow(); + } +} \ No newline at end of file From dbaf3d7febb18229ef95080264e76b889bd7758e Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 16 Dec 2023 18:50:06 +0100 Subject: [PATCH 2/4] Extract common methods to compile per test class --- .../RefasterTemplateProcessorTest.java | 33 ++++++++++--------- .../java/template/TemplateProcessorTest.java | 19 +++++------ 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java index 19b8d80c..96d309d4 100644 --- a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java +++ b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java @@ -19,6 +19,7 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate; import com.google.testing.compile.Compilation; import com.google.testing.compile.JavaFileObjects; +import org.jetbrains.annotations.NotNull; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; import org.openrewrite.java.template.processor.RefasterTemplateProcessor; @@ -30,6 +31,7 @@ import static com.google.testing.compile.CompilationSubject.assertThat; import static com.google.testing.compile.Compiler.javac; +import static org.junit.jupiter.api.Assertions.assertEquals; class RefasterTemplateProcessorTest { @ParameterizedTest @@ -42,11 +44,7 @@ class RefasterTemplateProcessorTest { "SimplifyBooleans", }) void generateRecipe(String recipeName) { - // 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 = javac() - .withProcessors(new RefasterTemplateProcessor()) - .withClasspath(classpath()) - .compile(JavaFileObjects.forResource("refaster/" + recipeName + ".java")); + Compilation compilation = compile("refaster/" + recipeName + ".java"); assertThat(compilation).succeeded(); assertThat(compilation).hadNoteCount(0); assertThat(compilation) @@ -60,13 +58,10 @@ void generateRecipe(String recipeName) { "OrOrElseThrow", "RefasterAnyOf", }) - void skipRecipeGeneration(String recipeName){ - Compilation compilation = javac() - .withProcessors(new RefasterTemplateProcessor()) - .withClasspath(classpath()) - .compile(JavaFileObjects.forResource("refaster/" + recipeName + ".java")); + void skipRecipeGeneration(String recipeName) { + Compilation compilation = compile("refaster/" + recipeName + ".java"); assertThat(compilation).succeeded(); - assert compilation.generatedSourceFiles().isEmpty(); + assertEquals(0, compilation.generatedSourceFiles().size(), "Should not generate recipe for " + recipeName); } @ParameterizedTest @@ -77,10 +72,7 @@ void skipRecipeGeneration(String recipeName){ "Matching", }) void nestedRecipes(String recipeName) { - Compilation compilation = javac() - .withProcessors(new RefasterTemplateProcessor()) - .withClasspath(classpath()) - .compile(JavaFileObjects.forResource("refaster/" + recipeName + ".java")); + Compilation compilation = compile("refaster/" + recipeName + ".java"); assertThat(compilation).succeeded(); assertThat(compilation).hadNoteCount(0); assertThat(compilation) // Recipes (plural) @@ -88,6 +80,15 @@ void nestedRecipes(String recipeName) { .hasSourceEquivalentTo(JavaFileObjects.forResource("refaster/" + recipeName + "Recipes.java")); } + @NotNull + private static Compilation compile(String resourceName) { + // As per https://github.com/google/compile-testing/blob/v0.21.0/src/main/java/com/google/testing/compile/package-info.java#L53-L55 + return javac() + .withProcessors(new RefasterTemplateProcessor()) + .withClasspath(classpath()) + .compile(JavaFileObjects.forResource(resourceName)); + } + static Collection classpath() { return Arrays.asList( fileForClass(BeforeTemplate.class), @@ -101,7 +102,7 @@ static Collection classpath() { } // As per https://github.com/google/auto/blob/auto-value-1.10.2/factory/src/test/java/com/google/auto/factory/processor/AutoFactoryProcessorTest.java#L99 - static File fileForClass(Class c) { + private static File fileForClass(Class c) { URL url = c.getProtectionDomain().getCodeSource().getLocation(); assert url.getProtocol().equals("file") || url.getProtocol().equals("jrt") : "Unexpected URL: " + url; return new File(url.getPath()); diff --git a/src/test/java/org/openrewrite/java/template/TemplateProcessorTest.java b/src/test/java/org/openrewrite/java/template/TemplateProcessorTest.java index 333eb814..be9ffb8d 100644 --- a/src/test/java/org/openrewrite/java/template/TemplateProcessorTest.java +++ b/src/test/java/org/openrewrite/java/template/TemplateProcessorTest.java @@ -37,12 +37,8 @@ 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 = javac() - .withProcessors(new RefasterTemplateProcessor(), new TemplateProcessor()) - .withClasspath(classpath()) - .compile(JavaFileObjects.forResource("template/ShouldAddClasspath.java")); + Compilation compilation = compile("template/ShouldAddClasspath.java"); assertThat(compilation).succeeded(); - compilation.generatedSourceFiles().forEach(System.out::println); assertThat(compilation) .generatedSourceFile("foo/ShouldAddClasspathRecipes$" + qualifier + "Recipe$1_before") .hasSourceEquivalentTo(JavaFileObjects.forResource("template/ShouldAddClasspathRecipe$" + qualifier + "Recipe$1_before.java")); @@ -53,14 +49,17 @@ void qualification(String qualifier) { @Test void parameterReuse() { - Compilation compilation = javac() - .withProcessors(new RefasterTemplateProcessor(), new TemplateProcessor()) - .withClasspath(classpath()) - .compile(JavaFileObjects.forResource("template/ParameterReuse.java")); + Compilation compilation = compile("template/ParameterReuse.java"); assertThat(compilation).succeeded(); - compilation.generatedSourceFiles().forEach(System.out::println); assertThat(compilation) .generatedSourceFile("foo/ParameterReuseRecipe$1_before") .hasSourceEquivalentTo(JavaFileObjects.forResource("template/ParameterReuseRecipe$1_before.java")); } + + private static Compilation compile(String resourceName) { + return javac() + .withProcessors(new RefasterTemplateProcessor(), new TemplateProcessor()) + .withClasspath(classpath()) + .compile(JavaFileObjects.forResource(resourceName)); + } } From 8ec310c162f8c323b3e50547a61db46cd16ec0e0 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 16 Dec 2023 23:14:39 +0100 Subject: [PATCH 3/4] Return early when class has generics --- .../processor/RefasterTemplateProcessor.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java index 373f394d..81cb5eaa 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -678,11 +678,16 @@ private TemplateDescriptor validate(Context context, JCCompilationUnit cu) { return null; } + if (classDecl.typarams != null && !classDecl.typarams.isEmpty()) { + printNoteOnce("Generics are currently not supported", classDecl.sym); + return null; + } + boolean valid = true; for (JCTree member : classDecl.getMembers()) { if (member instanceof JCTree.JCMethodDecl && !beforeTemplates.contains(member) && member != afterTemplate) { for (JCTree.JCAnnotation annotation : getTemplateAnnotations(((JCTree.JCMethodDecl) member), UNSUPPORTED_ANNOTATIONS::contains)) { - printNoteOnce("The @" + annotation.annotationType + " is currently not supported", ((JCTree.JCMethodDecl) member)); + printNoteOnce("The @" + annotation.annotationType + " is currently not supported", ((JCTree.JCMethodDecl) member).sym); valid = false; } } @@ -703,22 +708,22 @@ private boolean validateTemplateMethod(JCTree.JCMethodDecl template) { boolean valid = true; // TODO: support all Refaster method-level annotations for (JCTree.JCAnnotation annotation : getTemplateAnnotations(template, UNSUPPORTED_ANNOTATIONS::contains)) { - printNoteOnce("The @" + annotation.annotationType + " is currently not supported", template); + printNoteOnce("The @" + annotation.annotationType + " is currently not supported", template.sym); valid = false; } // TODO: support all Refaster parameter-level annotations for (JCTree.JCVariableDecl parameter : template.getParameters()) { for (JCTree.JCAnnotation annotation : getTemplateAnnotations(parameter, UNSUPPORTED_ANNOTATIONS::contains)) { - printNoteOnce("The @" + annotation.annotationType + " annotation is currently not supported", template); + printNoteOnce("The @" + annotation.annotationType + " annotation is currently not supported", template.sym); valid = false; } if (parameter.vartype instanceof ParameterizedTypeTree || parameter.vartype.type instanceof Type.TypeVar) { - printNoteOnce("Generics are currently not supported", template); + printNoteOnce("Generics are currently not supported", template.sym); valid = false; } } if (template.restype instanceof ParameterizedTypeTree || template.restype.type instanceof Type.TypeVar) { - printNoteOnce("Generics are currently not supported", template); + printNoteOnce("Generics are currently not supported", template.sym); valid = false; } valid &= new TreeScanner() { @@ -733,7 +738,7 @@ boolean validate(JCTree tree) { public void visitIdent(JCTree.JCIdent jcIdent) { if (jcIdent.sym != null && jcIdent.sym.packge().getQualifiedName().contentEquals("com.google.errorprone.refaster")) { - printNoteOnce(jcIdent.type.tsym.getQualifiedName() + " is not supported", template); + printNoteOnce(jcIdent.type.tsym.getQualifiedName() + " is not supported", template.sym); valid = false; } } @@ -769,9 +774,9 @@ private boolean resolve(Context context, JCCompilationUnit cu) { private final Set printedMessages = new HashSet<>(); - private void printNoteOnce(String message, JCTree.JCMethodDecl template) { + private void printNoteOnce(String message, Symbol symbol) { if (printedMessages.add(message)) { - processingEnv.getMessager().printMessage(Kind.NOTE, message, template.sym); + processingEnv.getMessager().printMessage(Kind.NOTE, message, symbol); } } From cf1092c14318fea9f3cb4a5c258052d35dee0a3f Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 16 Dec 2023 23:20:58 +0100 Subject: [PATCH 4/4] Delete test again that required Java 11 --- .../RefasterTemplateProcessorTest.java | 1 - .../resources/refaster/OrOrElseThrow.java | 33 ------------------- 2 files changed, 34 deletions(-) delete mode 100644 src/test/resources/refaster/OrOrElseThrow.java diff --git a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java index d633662d..b05d60c5 100644 --- a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java +++ b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java @@ -53,7 +53,6 @@ void generateRecipe(String recipeName) { @ParameterizedTest @ValueSource(strings = { "OrElseGetGet", - "OrOrElseThrow", "RefasterAnyOf", }) void skipRecipeGeneration(String recipeName) { diff --git a/src/test/resources/refaster/OrOrElseThrow.java b/src/test/resources/refaster/OrOrElseThrow.java deleted file mode 100644 index bdd6211a..00000000 --- a/src/test/resources/refaster/OrOrElseThrow.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright 2023 the original author or authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - *

- * https://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package foo; - -import com.google.errorprone.refaster.annotation.AfterTemplate; -import com.google.errorprone.refaster.annotation.BeforeTemplate; - -import java.util.Optional; - -class OrOrElseThrow { - @BeforeTemplate - T before(Optional o1, Optional o2) { - return o1.orElseGet(() -> o2.orElseThrow()); - } - - @AfterTemplate - T after(Optional o1, Optional o2) { - return o1.or(() -> o2).orElseThrow(); - } -} \ No newline at end of file