From 97cca687386b910cfc8e7c4fd235ff41beecb09c Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 10 Dec 2023 22:11:42 +0100 Subject: [PATCH 1/5] Print messages once, as to not flood console --- .../processor/RefasterTemplateProcessor.java | 28 +++++++++++-------- .../RefasterTemplateProcessorTest.java | 8 ++++-- .../resources/refaster/MatchingRecipes.java | 2 +- .../refaster/MultipleDereferencesRecipes.java | 2 +- .../ShouldSupportNestedClassesRecipes.java | 4 +-- 5 files changed, 25 insertions(+), 19 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 7cce0749..9016184c 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -33,7 +33,6 @@ import javax.annotation.processing.RoundEnvironment; import javax.annotation.processing.SupportedAnnotationTypes; import javax.lang.model.element.Element; -import javax.lang.model.element.Modifier; import javax.lang.model.element.NestingKind; import javax.lang.model.element.TypeElement; import javax.tools.Diagnostic.Kind; @@ -126,10 +125,6 @@ public void visitClassDef(JCTree.JCClassDecl classDecl) { .collect(Collectors.toList()); JCTree.JCClassDecl copy = treeMaker.ClassDef(classDecl.mods, classDecl.name, classDecl.typarams, classDecl.extending, classDecl.implementing, com.sun.tools.javac.util.List.from(membersWithoutConstructor)); - String classNameIncludingOuter = descriptor.classDecl.sym.packge().toString().isEmpty() ? - descriptor.classDecl.sym.className() : - descriptor.classDecl.sym.className().substring(descriptor.classDecl.sym.packge().toString().length() + 1); - String templateFqn = classDecl.sym.fullname.toString() + "Recipe"; String templateCode = copy.toString().trim(); @@ -685,7 +680,7 @@ private TemplateDescriptor validate(Context context, JCCompilationUnit cu) { 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)) { - processingEnv.getMessager().printMessage(Kind.NOTE, "The @" + annotation.annotationType + " is currently not supported", ((JCTree.JCMethodDecl) member).sym); + printNoteOnce("The @" + annotation.annotationType + " is currently not supported", ((JCTree.JCMethodDecl) member)); valid = false; } } @@ -706,22 +701,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)) { - processingEnv.getMessager().printMessage(Kind.NOTE, "The @" + annotation.annotationType + " is currently not supported", template.sym); + printNoteOnce("The @" + annotation.annotationType + " is currently not supported", template); valid = false; } // TODO: support all Refaster parameter-level annotations for (JCTree.JCVariableDecl parameter : template.getParameters()) { for (JCTree.JCAnnotation annotation : getTemplateAnnotations(parameter, UNSUPPORTED_ANNOTATIONS::contains)) { - processingEnv.getMessager().printMessage(Kind.NOTE, "The @" + annotation.annotationType + " annotation is currently not supported", template.sym); + printNoteOnce("The @" + annotation.annotationType + " annotation is currently not supported", template); valid = false; } if (parameter.vartype instanceof ParameterizedTypeTree || parameter.vartype.type instanceof Type.TypeVar) { - processingEnv.getMessager().printMessage(Kind.NOTE, "Generics are currently not supported", template.sym); + printNoteOnce("Generics are currently not supported", template); valid = false; } } if (template.restype instanceof ParameterizedTypeTree || template.restype.type instanceof Type.TypeVar) { - processingEnv.getMessager().printMessage(Kind.NOTE, "Generics are currently not supported", template.sym); + printNoteOnce("Generics are currently not supported", template); valid = false; } valid &= new TreeScanner() { @@ -735,8 +730,9 @@ boolean validate(JCTree tree) { @Override public void visitIdent(JCTree.JCIdent jcIdent) { if (jcIdent.sym != null - && jcIdent.sym.packge().getQualifiedName().contentEquals("com.google.errorprone.refaster")) { - processingEnv.getMessager().printMessage(Kind.NOTE, jcIdent.type.tsym.getQualifiedName() + " is not supported", template.sym); + && jcIdent.sym.packge().getQualifiedName().contentEquals("com.google.errorprone.refaster") + && !jcIdent.sym.getSimpleName().contentEquals("Refaster")) { + printNoteOnce(jcIdent.type.tsym.getQualifiedName() + " is not supported", template); valid = false; } } @@ -744,6 +740,14 @@ public void visitIdent(JCTree.JCIdent jcIdent) { return valid; } + private final Set printedMessages = new HashSet<>(); + + private void printNoteOnce(String message, JCTree.JCMethodDecl template) { + if (printedMessages.add(message)) { + processingEnv.getMessager().printMessage(Kind.NOTE, message, template.sym); + } + } + public void beforeTemplate(JCTree.JCMethodDecl method) { beforeTemplates.add(method); } diff --git a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java index bf0cdd28..d1382330 100644 --- a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java +++ b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java @@ -47,6 +47,7 @@ void generateRecipe(String recipeName) { .withClasspath(classpath()) .compile(JavaFileObjects.forResource("refaster/" + recipeName + ".java")); assertThat(compilation).succeeded(); + assertThat(compilation).hadNoteCount(0); compilation.generatedSourceFiles().forEach(System.out::println); assertThat(compilation) .generatedSourceFile("foo/" + recipeName + "Recipe") @@ -55,10 +56,10 @@ void generateRecipe(String recipeName) { @ParameterizedTest @ValueSource(strings = { -// "ShouldSupportNestedClasses", + "ShouldSupportNestedClasses", "ShouldAddImports", -// "MultipleDereferences", -// "Matching", + "MultipleDereferences", + "Matching", }) void nestedRecipes(String recipeName) { Compilation compilation = javac() @@ -66,6 +67,7 @@ void nestedRecipes(String recipeName) { .withClasspath(classpath()) .compile(JavaFileObjects.forResource("refaster/" + recipeName + ".java")); assertThat(compilation).succeeded(); + assertThat(compilation).hadNoteCount(0); compilation.generatedSourceFiles().forEach(System.out::println); assertThat(compilation) // Recipes (plural) .generatedSourceFile("foo/" + recipeName + "Recipes") diff --git a/src/test/resources/refaster/MatchingRecipes.java b/src/test/resources/refaster/MatchingRecipes.java index 77c05b4b..0be1e83b 100644 --- a/src/test/resources/refaster/MatchingRecipes.java +++ b/src/test/resources/refaster/MatchingRecipes.java @@ -34,7 +34,7 @@ import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.*; -public final class MatchingRecipes extends Recipe { +public class MatchingRecipes extends Recipe { @Override public String getDisplayName() { return "Static analysis"; diff --git a/src/test/resources/refaster/MultipleDereferencesRecipes.java b/src/test/resources/refaster/MultipleDereferencesRecipes.java index 3c3511a5..a95411ac 100644 --- a/src/test/resources/refaster/MultipleDereferencesRecipes.java +++ b/src/test/resources/refaster/MultipleDereferencesRecipes.java @@ -37,7 +37,7 @@ import java.io.IOException; import java.nio.file.Path; -public final class MultipleDereferencesRecipes extends Recipe { +public class MultipleDereferencesRecipes extends Recipe { @Override public String getDisplayName() { return "`MultipleDereferences` Refaster recipes"; diff --git a/src/test/resources/refaster/ShouldSupportNestedClassesRecipes.java b/src/test/resources/refaster/ShouldSupportNestedClassesRecipes.java index 13b87d32..3b204063 100644 --- a/src/test/resources/refaster/ShouldSupportNestedClassesRecipes.java +++ b/src/test/resources/refaster/ShouldSupportNestedClassesRecipes.java @@ -34,7 +34,7 @@ import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.*; -public final class ShouldSupportNestedClassesRecipes extends Recipe { +public class ShouldSupportNestedClassesRecipes extends Recipe { @Override public String getDisplayName() { return "`ShouldSupportNestedClasses` Refaster recipes"; @@ -95,7 +95,7 @@ public J visitBinary(J.Binary elem, ExecutionContext ctx) { } @NonNullApi - static class AnotherClassRecipe extends Recipe { + public static class AnotherClassRecipe extends Recipe { @Override public String getDisplayName() { From baf8ca60e5b7e627ca7f0c16bab4279ed03a5956 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 10 Dec 2023 22:13:20 +0100 Subject: [PATCH 2/5] Do not allow Refaster just yet --- .../java/template/processor/RefasterTemplateProcessor.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 9016184c..b88a6b0f 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -730,8 +730,7 @@ boolean validate(JCTree tree) { @Override public void visitIdent(JCTree.JCIdent jcIdent) { if (jcIdent.sym != null - && jcIdent.sym.packge().getQualifiedName().contentEquals("com.google.errorprone.refaster") - && !jcIdent.sym.getSimpleName().contentEquals("Refaster")) { + && jcIdent.sym.packge().getQualifiedName().contentEquals("com.google.errorprone.refaster")) { printNoteOnce(jcIdent.type.tsym.getQualifiedName() + " is not supported", template); valid = false; } From 1b87b8db485ff665f861e36c57a947d43e21c9d0 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 10 Dec 2023 22:16:07 +0100 Subject: [PATCH 3/5] Move printedMessages outside `TemplateDescriptor` --- .../processor/RefasterTemplateProcessor.java | 16 ++++++++-------- 1 file changed, 8 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 b88a6b0f..6e7f77d4 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -739,14 +739,6 @@ public void visitIdent(JCTree.JCIdent jcIdent) { return valid; } - private final Set printedMessages = new HashSet<>(); - - private void printNoteOnce(String message, JCTree.JCMethodDecl template) { - if (printedMessages.add(message)) { - processingEnv.getMessager().printMessage(Kind.NOTE, message, template.sym); - } - } - public void beforeTemplate(JCTree.JCMethodDecl method) { beforeTemplates.add(method); } @@ -773,6 +765,14 @@ private boolean resolve(Context context, JCCompilationUnit cu) { } + private final Set printedMessages = new HashSet<>(); + + private void printNoteOnce(String message, JCTree.JCMethodDecl template) { + if (printedMessages.add(message)) { + processingEnv.getMessager().printMessage(Kind.NOTE, message, template.sym); + } + } + private static List getTemplateAnnotations(MethodTree method, Predicate typePredicate) { List result = new ArrayList<>(); for (AnnotationTree annotation : method.getModifiers().getAnnotations()) { From 3167698012e1a277fa11d5dfe7c689fe2e9e5397 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 10 Dec 2023 23:14:12 +0100 Subject: [PATCH 4/5] Exclude Exceptions from UsesType Fixes #30 --- .../processor/RefasterTemplateProcessor.java | 14 ++++++++++---- .../refaster/MultipleDereferencesRecipes.java | 2 -- 2 files changed, 10 insertions(+), 6 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 6e7f77d4..68814657 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -129,10 +129,13 @@ public void visitClassDef(JCTree.JCClassDecl classDecl) { String templateCode = copy.toString().trim(); for (JCTree.JCMethodDecl template : descriptor.beforeTemplates) { + Set thrown = template.thrown.stream().map(exp -> exp.type).collect(Collectors.toSet()); for (Symbol anImport : ImportDetector.imports(template)) { if (anImport instanceof Symbol.ClassSymbol) { - imports.computeIfAbsent(template, k -> new TreeSet<>()) - .add(anImport.getQualifiedName().toString().replace('$', '.')); + if (!thrown.contains(anImport.type)) { + imports.computeIfAbsent(template, k -> new TreeSet<>()) + .add(anImport.getQualifiedName().toString().replace('$', '.')); + } } else if (anImport instanceof Symbol.VarSymbol || anImport instanceof Symbol.MethodSymbol) { staticImports.computeIfAbsent(template, k -> new TreeSet<>()) .add(anImport.owner.getQualifiedName().toString().replace('$', '.') + '.' + anImport.flatName().toString()); @@ -142,9 +145,12 @@ public void visitClassDef(JCTree.JCClassDecl classDecl) { } } for (Symbol anImport : ImportDetector.imports(descriptor.afterTemplate)) { + Set thrown = descriptor.afterTemplate.thrown.stream().map(exp -> exp.type).collect(Collectors.toSet()); if (anImport instanceof Symbol.ClassSymbol) { - imports.computeIfAbsent(descriptor.afterTemplate, k -> new TreeSet<>()) - .add(anImport.getQualifiedName().toString().replace('$', '.')); + if (!thrown.contains(anImport.type)) { + imports.computeIfAbsent(descriptor.afterTemplate, k -> new TreeSet<>()) + .add(anImport.getQualifiedName().toString().replace('$', '.')); + } } else if (anImport instanceof Symbol.VarSymbol || anImport instanceof Symbol.MethodSymbol) { staticImports.computeIfAbsent(descriptor.afterTemplate, k -> new TreeSet<>()) .add(anImport.owner.getQualifiedName().toString().replace('$', '.') + '.' + anImport.flatName().toString()); diff --git a/src/test/resources/refaster/MultipleDereferencesRecipes.java b/src/test/resources/refaster/MultipleDereferencesRecipes.java index a95411ac..55857594 100644 --- a/src/test/resources/refaster/MultipleDereferencesRecipes.java +++ b/src/test/resources/refaster/MultipleDereferencesRecipes.java @@ -34,7 +34,6 @@ import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.*; import java.nio.file.Files; -import java.io.IOException; import java.nio.file.Path; public class MultipleDereferencesRecipes extends Recipe { @@ -93,7 +92,6 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { }; return Preconditions.check( Preconditions.and( - new UsesType<>("java.io.IOException", true), new UsesType<>("java.nio.file.Files", true), new UsesType<>("java.nio.file.Path", true), new UsesMethod<>("java.nio.file.Files delete(..)") From 2820b5066317a1b86b397035ba81931a32e6b400 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 12 Dec 2023 12:19:44 +0100 Subject: [PATCH 5/5] Pull out the `thrown.contains` calls for now --- .../processor/RefasterTemplateProcessor.java | 14 ++++---------- .../refaster/MultipleDereferencesRecipes.java | 2 ++ 2 files changed, 6 insertions(+), 10 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 68814657..6e7f77d4 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -129,13 +129,10 @@ public void visitClassDef(JCTree.JCClassDecl classDecl) { String templateCode = copy.toString().trim(); for (JCTree.JCMethodDecl template : descriptor.beforeTemplates) { - Set thrown = template.thrown.stream().map(exp -> exp.type).collect(Collectors.toSet()); for (Symbol anImport : ImportDetector.imports(template)) { if (anImport instanceof Symbol.ClassSymbol) { - if (!thrown.contains(anImport.type)) { - imports.computeIfAbsent(template, k -> new TreeSet<>()) - .add(anImport.getQualifiedName().toString().replace('$', '.')); - } + imports.computeIfAbsent(template, k -> new TreeSet<>()) + .add(anImport.getQualifiedName().toString().replace('$', '.')); } else if (anImport instanceof Symbol.VarSymbol || anImport instanceof Symbol.MethodSymbol) { staticImports.computeIfAbsent(template, k -> new TreeSet<>()) .add(anImport.owner.getQualifiedName().toString().replace('$', '.') + '.' + anImport.flatName().toString()); @@ -145,12 +142,9 @@ public void visitClassDef(JCTree.JCClassDecl classDecl) { } } for (Symbol anImport : ImportDetector.imports(descriptor.afterTemplate)) { - Set thrown = descriptor.afterTemplate.thrown.stream().map(exp -> exp.type).collect(Collectors.toSet()); if (anImport instanceof Symbol.ClassSymbol) { - if (!thrown.contains(anImport.type)) { - imports.computeIfAbsent(descriptor.afterTemplate, k -> new TreeSet<>()) - .add(anImport.getQualifiedName().toString().replace('$', '.')); - } + imports.computeIfAbsent(descriptor.afterTemplate, k -> new TreeSet<>()) + .add(anImport.getQualifiedName().toString().replace('$', '.')); } else if (anImport instanceof Symbol.VarSymbol || anImport instanceof Symbol.MethodSymbol) { staticImports.computeIfAbsent(descriptor.afterTemplate, k -> new TreeSet<>()) .add(anImport.owner.getQualifiedName().toString().replace('$', '.') + '.' + anImport.flatName().toString()); diff --git a/src/test/resources/refaster/MultipleDereferencesRecipes.java b/src/test/resources/refaster/MultipleDereferencesRecipes.java index 55857594..a95411ac 100644 --- a/src/test/resources/refaster/MultipleDereferencesRecipes.java +++ b/src/test/resources/refaster/MultipleDereferencesRecipes.java @@ -34,6 +34,7 @@ import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.*; import java.nio.file.Files; +import java.io.IOException; import java.nio.file.Path; public class MultipleDereferencesRecipes extends Recipe { @@ -92,6 +93,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { }; return Preconditions.check( Preconditions.and( + new UsesType<>("java.io.IOException", true), new UsesType<>("java.nio.file.Files", true), new UsesType<>("java.nio.file.Path", true), new UsesMethod<>("java.nio.file.Files delete(..)")