From b889851bc7647a85c503df10484e5248824fda01 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 2 Feb 2024 23:45:41 +0100 Subject: [PATCH 1/3] Skip method references and lambdas in body --- build.gradle.kts | 9 ++--- .../processor/RefasterTemplateProcessor.java | 14 ++++++++ .../RefasterTemplateProcessorTest.java | 36 ++++++++++++++----- .../java/template/TemplateProcessorTest.java | 14 ++++---- .../refaster/StringIsEmptyPredicate.java | 33 +++++++++++++++++ 5 files changed, 87 insertions(+), 19 deletions(-) create mode 100644 src/test/resources/refaster/StringIsEmptyPredicate.java diff --git a/build.gradle.kts b/build.gradle.kts index 9662c55b..5aec76af 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -130,14 +130,15 @@ configure { suppressPomMetadataWarningsFor("runtimeElements") pom.withXml { - (asElement().getElementsByTagName("dependencies").item(0) as org.w3c.dom.Element?)?.let { dependencies -> + (asElement().getElementsByTagName("dependencies") + .item(0) as org.w3c.dom.Element?)?.let { dependencies -> dependencies.getElementsByTagName("dependency").let { dependencyList -> var i = 0 var length = dependencyList.length while (i < length) { (dependencyList.item(i) as org.w3c.dom.Element).let { dependency -> if ((dependency.getElementsByTagName("scope") - .item(0) as org.w3c.dom.Element).textContent == "provided" + .item(0) as org.w3c.dom.Element).textContent == "provided" ) { dependencies.removeChild(dependency) i-- @@ -156,12 +157,12 @@ configure { val signingKey: String? by project val signingPassword: String? by project val requireSigning = project.hasProperty("forceSigning") || project.hasProperty("releasing") -if(signingKey != null && signingPassword != null) { +if (signingKey != null && signingPassword != null) { signing { isRequired = requireSigning useInMemoryPgpKeys(signingKey, signingPassword) sign(publishing.publications["nebula"]) } -} else if(requireSigning) { +} else if (requireSigning) { throw RuntimeException("Artifact signing is required, but signingKey and/or signingPassword are null") } 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 34a9ba86..8932fd84 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -106,6 +106,10 @@ public boolean process(Set annotations, RoundEnvironment maybeGenerateTemplateSources(jcCompilationUnit); } } + + // Inform how many rules were skipped and why; useful for debugging, but not enabled by default + //printedMessages.entrySet().forEach(entry -> processingEnv.getMessager().printMessage(Kind.NOTE, entry.toString())); + // Give other annotation processors a chance to process the same annotations, for dual use of Refaster templates return false; } @@ -784,6 +788,16 @@ private boolean validateTemplateMethod(JCTree.JCMethodDecl template) { printNoteOnce("If statements are currently not supported", classDecl.sym); return false; } + if (template.body.stats.get(0) instanceof JCTree.JCReturn) { + JCTree.JCExpression expr = ((JCTree.JCReturn) template.body.stats.get(0)).expr; + if (expr instanceof JCTree.JCLambda) { + printNoteOnce("Lambdas are currently not supported", classDecl.sym); + return false; + } else if (expr instanceof JCTree.JCMemberReference) { + printNoteOnce("Method references are currently not supported", classDecl.sym); + return false; + } + } return new TreeScanner() { boolean valid = true; diff --git a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java index 1c7b63f6..579da269 100644 --- a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java +++ b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java @@ -19,12 +19,14 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate; import com.google.testing.compile.Compilation; import com.google.testing.compile.JavaFileObjects; +import org.intellij.lang.annotations.Language; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; import org.openrewrite.java.template.processor.RefasterTemplateProcessor; import org.openrewrite.java.template.processor.TypeAwareProcessor; +import javax.tools.JavaFileObject; import java.io.File; import java.net.URL; import java.util.Arrays; @@ -45,7 +47,7 @@ class RefasterTemplateProcessorTest { "SimplifyBooleans", }) void generateRecipe(String recipeName) { - Compilation compilation = compile("refaster/" + recipeName + ".java"); + Compilation compilation = compileResource("refaster/" + recipeName + ".java"); assertThat(compilation).succeeded(); assertThat(compilation).hadNoteCount(0); assertThat(compilation) @@ -55,7 +57,7 @@ void generateRecipe(String recipeName) { @Test void generateRecipeInDefaultPackage() { - Compilation compilation = compile("refaster/UnnamedPackage.java"); + Compilation compilation = compileResource("refaster/UnnamedPackage.java"); assertThat(compilation).succeeded(); assertThat(compilation).hadNoteCount(0); assertThat(compilation) @@ -69,7 +71,7 @@ void generateRecipeInDefaultPackage() { "RefasterAnyOf", }) void skipRecipeGeneration(String recipeName) { - Compilation compilation = compile("refaster/" + recipeName + ".java"); + Compilation compilation = compileResource("refaster/" + recipeName + ".java"); assertThat(compilation).succeeded(); assertEquals(0, compilation.generatedSourceFiles().size(), "Should not generate recipe for " + recipeName); } @@ -85,7 +87,7 @@ void skipRecipeGeneration(String recipeName) { "SimplifyTernary", }) void nestedRecipes(String recipeName) { - Compilation compilation = compile("refaster/" + recipeName + ".java"); + Compilation compilation = compileResource("refaster/" + recipeName + ".java"); assertThat(compilation).succeeded(); assertThat(compilation).hadNoteCount(0); assertThat(compilation) // Recipes (plural) @@ -93,12 +95,30 @@ void nestedRecipes(String recipeName) { .hasSourceEquivalentTo(JavaFileObjects.forResource("refaster/" + recipeName + "Recipes.java")); } - private static Compilation compile(String resourceName) { - return compile(resourceName, new RefasterTemplateProcessor()); + @Test + void stringIsEmptyPredicate() { + Compilation compilation = compileResource("refaster/StringIsEmptyPredicate.java"); + assertThat(compilation).succeeded(); + assertThat(compilation).hadNoteCount(2); + assertThat(compilation).hadNoteContaining("Lambdas are currently not supported"); + assertThat(compilation).hadNoteContaining("Method references are currently not supported"); + assertEquals(0, compilation.generatedSourceFiles().size(), "Not yet supported"); + } + + private static Compilation compileResource(String resourceName) { + return compileResource(resourceName, new RefasterTemplateProcessor()); } - static Compilation compile(String resourceName, TypeAwareProcessor processor) { + static Compilation compileResource(String resourceName, TypeAwareProcessor processor) { // 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 compileResource(JavaFileObjects.forResource(resourceName), processor); + } + + static Compilation compileSource(String fqn, @Language("java") String source, TypeAwareProcessor processor) { + return compileResource(JavaFileObjects.forSourceString(fqn, source), processor); + } + + static Compilation compileResource(JavaFileObject javaFileObject, TypeAwareProcessor processor) { return javac() .withProcessors(processor) .withClasspath(Arrays.asList( @@ -110,7 +130,7 @@ static Compilation compile(String resourceName, TypeAwareProcessor processor) { fileForClass(org.slf4j.Logger.class), fileForClass(Primitive.class) )) - .compile(JavaFileObjects.forResource(resourceName)); + .compile(javaFileObject); } // 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 diff --git a/src/test/java/org/openrewrite/java/template/TemplateProcessorTest.java b/src/test/java/org/openrewrite/java/template/TemplateProcessorTest.java index 3139cd3d..0d1ec116 100644 --- a/src/test/java/org/openrewrite/java/template/TemplateProcessorTest.java +++ b/src/test/java/org/openrewrite/java/template/TemplateProcessorTest.java @@ -23,7 +23,7 @@ import org.openrewrite.java.template.processor.TemplateProcessor; import static com.google.testing.compile.CompilationSubject.assertThat; -import static org.openrewrite.java.template.RefasterTemplateProcessorTest.compile; +import static org.openrewrite.java.template.RefasterTemplateProcessorTest.compileResource; class TemplateProcessorTest { @ParameterizedTest @@ -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/ShouldAddClasspathRecipes.java", new TemplateProcessor()); + Compilation compilation = compileResource("template/ShouldAddClasspathRecipes.java", new TemplateProcessor()); assertThat(compilation).succeeded(); assertThat(compilation) .generatedSourceFile("foo/ShouldAddClasspathRecipes$" + qualifier + "Recipe$1_before") @@ -47,7 +47,7 @@ void qualification(String qualifier) { @Test void parameterReuse() { - Compilation compilation = compile("template/ParameterReuseRecipe.java", new TemplateProcessor()); + Compilation compilation = compileResource("template/ParameterReuseRecipe.java", new TemplateProcessor()); assertThat(compilation).succeeded(); assertThat(compilation) .generatedSourceFile("foo/ParameterReuseRecipe$1_before") @@ -56,7 +56,7 @@ void parameterReuse() { @Test void parserClasspath() { - Compilation compilation = compile("template/LoggerRecipe.java", new TemplateProcessor()); + Compilation compilation = compileResource("template/LoggerRecipe.java", new TemplateProcessor()); assertThat(compilation).succeeded(); assertThat(compilation) .generatedSourceFile("template/LoggerRecipe$1_logger") @@ -68,7 +68,7 @@ void parserClasspath() { @Test void generics() { - Compilation compilation = compile("template/Generics.java", new TemplateProcessor()); + Compilation compilation = compileResource("template/Generics.java", new TemplateProcessor()); assertThat(compilation).succeeded(); assertThat(compilation) .generatedSourceFile("template/Generics$1_before") @@ -80,7 +80,7 @@ void generics() { @Test void throwNew() { - Compilation compilation = compile("template/ThrowNewRecipe.java", new TemplateProcessor()); + Compilation compilation = compileResource("template/ThrowNewRecipe.java", new TemplateProcessor()); assertThat(compilation).succeeded(); assertThat(compilation) .generatedSourceFile("template/ThrowNewRecipe$1_template") @@ -89,7 +89,7 @@ void throwNew() { @Test void unnamedPackage() { - Compilation compilation = compile("template/UnnamedPackage.java", new TemplateProcessor()); + Compilation compilation = compileResource("template/UnnamedPackage.java", new TemplateProcessor()); assertThat(compilation).succeeded(); assertThat(compilation) .generatedSourceFile("UnnamedPackage$1_message") diff --git a/src/test/resources/refaster/StringIsEmptyPredicate.java b/src/test/resources/refaster/StringIsEmptyPredicate.java new file mode 100644 index 00000000..aa2182e5 --- /dev/null +++ b/src/test/resources/refaster/StringIsEmptyPredicate.java @@ -0,0 +1,33 @@ +/* + * Copyright 2024 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 refaster; + +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; + +import java.util.function.Predicate; + +class StringIsEmptyPredicate { + @BeforeTemplate + Predicate before() { + return s -> s.isEmpty(); + } + + @AfterTemplate + Predicate after() { + return String::isEmpty; + } +} From 08bd95a386796299ceeca83159adc62b245cea64 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 2 Feb 2024 23:53:46 +0100 Subject: [PATCH 2/3] Skip after the first invalid template is found --- .../java/template/processor/RefasterTemplateProcessor.java | 4 ++-- .../java/template/RefasterTemplateProcessorTest.java | 3 +-- 2 files changed, 3 insertions(+), 4 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 8932fd84..e33f20b7 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -754,9 +754,9 @@ private TemplateDescriptor validate(Context context, JCCompilationUnit cu) { boolean valid = resolve(context, cu); if (valid) { for (JCTree.JCMethodDecl template : beforeTemplates) { - valid &= validateTemplateMethod(template); + valid = valid && validateTemplateMethod(template); } - valid &= validateTemplateMethod(afterTemplate); + valid = valid && validateTemplateMethod(afterTemplate); } return valid ? this : null; } diff --git a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java index 579da269..842e6b75 100644 --- a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java +++ b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java @@ -99,9 +99,8 @@ void nestedRecipes(String recipeName) { void stringIsEmptyPredicate() { Compilation compilation = compileResource("refaster/StringIsEmptyPredicate.java"); assertThat(compilation).succeeded(); - assertThat(compilation).hadNoteCount(2); + assertThat(compilation).hadNoteCount(1); assertThat(compilation).hadNoteContaining("Lambdas are currently not supported"); - assertThat(compilation).hadNoteContaining("Method references are currently not supported"); assertEquals(0, compilation.generatedSourceFiles().size(), "Not yet supported"); } From 8286412e1cc70a38e09b02b2ee22d255b20a0a10 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 3 Feb 2024 00:20:47 +0100 Subject: [PATCH 3/3] Sort reported messages --- .../java/template/processor/RefasterTemplateProcessor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 e33f20b7..d3ee5bfb 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -108,7 +108,8 @@ public boolean process(Set annotations, RoundEnvironment } // Inform how many rules were skipped and why; useful for debugging, but not enabled by default - //printedMessages.entrySet().forEach(entry -> processingEnv.getMessager().printMessage(Kind.NOTE, entry.toString())); + //printedMessages.entrySet().stream().sorted(Map.Entry.comparingByValue()) + // .forEach(entry -> processingEnv.getMessager().printMessage(Kind.NOTE, entry.toString())); // Give other annotation processors a chance to process the same annotations, for dual use of Refaster templates return false;