From a28be2722c35e3992babe571daf5b1b3a3fad17f Mon Sep 17 00:00:00 2001 From: lingenj Date: Wed, 11 Dec 2024 17:40:40 +0100 Subject: [PATCH 01/35] Starting point: PreConditionsVerifier --- .../RefasterTemplateProcessorTest.java | 5 +- .../refaster/PreConditionsVerifier.java | 112 +++++ .../refaster/PreConditionsVerifierRecipe.java | 402 ++++++++++++++++++ 3 files changed, 517 insertions(+), 2 deletions(-) create mode 100644 src/test/resources/refaster/PreConditionsVerifier.java create mode 100644 src/test/resources/refaster/PreConditionsVerifierRecipe.java diff --git a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java index c659e2d3..f5c8f4ef 100644 --- a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java +++ b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java @@ -44,7 +44,8 @@ class RefasterTemplateProcessorTest { @ParameterizedTest @ValueSource(strings = { - "Arrays", + "PreConditionsVerifier", + /*"Arrays", "CharacterEscapeAnnotation", "MethodThrows", "NestedPreconditions", @@ -52,7 +53,7 @@ class RefasterTemplateProcessorTest { "UseStringIsEmpty", "SimplifyBooleans", "TwoVisitMethods", - "FindListAdd", + "FindListAdd",*/ }) void generateRecipe(String recipeName) { Compilation compilation = compileResource("refaster/" + recipeName + ".java"); diff --git a/src/test/resources/refaster/PreConditionsVerifier.java b/src/test/resources/refaster/PreConditionsVerifier.java new file mode 100644 index 00000000..6c5880d8 --- /dev/null +++ b/src/test/resources/refaster/PreConditionsVerifier.java @@ -0,0 +1,112 @@ +/* + * 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 java.util.List; +import java.util.Map; + +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; + +/** + * A refaster template to test when a `UsesType`and Preconditions.or should or should not be applied to the Preconditions check. + */ +public class PreConditionsVerifier { + public static class NoUsesTypeWhenBeforeTemplateContainsPrimitive { + @BeforeTemplate + void before(int actual) { + System.out.println(actual); + } + + @BeforeTemplate + void beforeTwo(int actual) { + System.out.println(actual); + } + + @AfterTemplate + void after(Object actual) { + System.out.println("Changed: " + actual); + } + } + + public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType { + @BeforeTemplate + void before(int actual) { + System.out.println(actual); + } + + @BeforeTemplate + void before(Map actual) { + System.out.println(actual); + } + + @AfterTemplate + void after(Object actual) { + System.out.println("Changed: " + actual); + } + } + + public static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType { + @BeforeTemplate + void before(String actual) { + System.out.println(actual); + } + + @BeforeTemplate + void before(Map actual) { + System.out.println(actual); + } + + @AfterTemplate + void after(Object actual) { + System.out.println("Changed: " + actual); + } + } + + public static class UsesTypeMapWhenBeforeTemplateContainsMap { + @BeforeTemplate + void before(Map actual) { + System.out.println(actual); + } + + @BeforeTemplate + void beforeTwo(Map actual) { + System.out.println(actual); + } + + @AfterTemplate + void after(Object actual) { + System.out.println("Changed: " + actual); + } + } + + public static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList { + @BeforeTemplate + void before(List actual) { + System.out.println(actual); + } + + @BeforeTemplate + void beforeTwo(Map actual) { + System.out.println(actual); + } + + @AfterTemplate + void after(Object actual) { + System.out.println("Changed: " + actual); + } + } +} diff --git a/src/test/resources/refaster/PreConditionsVerifierRecipe.java b/src/test/resources/refaster/PreConditionsVerifierRecipe.java new file mode 100644 index 00000000..3fe8db01 --- /dev/null +++ b/src/test/resources/refaster/PreConditionsVerifierRecipe.java @@ -0,0 +1,402 @@ +package foo; + +import org.jspecify.annotations.NullMarked; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.search.*; +import org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor; +import org.openrewrite.java.tree.*; + +import javax.annotation.Generated; +import java.util.*; + +import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.*; + +/** + * OpenRewrite recipes created for Refaster template {@code foo.PreConditionsVerifier}. + */ +@SuppressWarnings("all") +@Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") +public class PreConditionsVerifierRecipes extends Recipe { + /** + * Instantiates a new instance. + */ + public PreConditionsVerifierRecipes() {} + + @Override + public String getDisplayName() { + return "A refaster template to test when a `UsesType`and Preconditions.or should or should not be applied to the Preconditions check"; + } + + @Override + public String getDescription() { + return "Refaster template recipes for `foo.PreConditionsVerifier`."; + } + + @Override + public List getRecipeList() { + return Arrays.asList( + new NoUsesTypeWhenBeforeTemplateContainsPrimitiveRecipe(), + new NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherTypeRecipe(), + new NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherTypeRecipe(), + new UsesTypeMapWhenBeforeTemplateContainsMapRecipe(), + new UsesTypeMapOrListWhenBeforeTemplateContainsMapAndListRecipe() + ); + } + + /** + * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitive}. + */ + @SuppressWarnings("all") + @NullMarked + @Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") + public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveRecipe extends Recipe { + + /** + * Instantiates a new instance. + */ + public NoUsesTypeWhenBeforeTemplateContainsPrimitiveRecipe() {} + + @Override + public String getDisplayName() { + return "Refaster template `PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitive`"; + } + + @Override + public String getDescription() { + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitive {\n \n @BeforeTemplate()\n void before(int actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void beforeTwo(int actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + } + + @Override + public TreeVisitor getVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + final JavaTemplate before = JavaTemplate + .builder("System.out.println(#{actual:any(int)});") + .build(); + final JavaTemplate beforeTwo = JavaTemplate + .builder("System.out.println(#{actual:any(int)});") + .build(); + final JavaTemplate after = JavaTemplate + .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") + .build(); + + @Override + public J visitMethodInvocation(J.MethodInvocation 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 + ); + } + if ((matcher = beforeTwo.matcher(getCursor())).find()) { + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + return super.visitMethodInvocation(elem, ctx); + } + + }; + return Preconditions.check( + new UsesMethod<>("java.io.PrintStream println(..)", true), + javaVisitor + ); + } + } + + /** + * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType}. + */ + @SuppressWarnings("all") + @NullMarked + @Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") + public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherTypeRecipe extends Recipe { + + /** + * Instantiates a new instance. + */ + public NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherTypeRecipe() {} + + @Override + public String getDisplayName() { + return "Refaster template `PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType`"; + } + + @Override + public String getDescription() { + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType {\n \n @BeforeTemplate()\n void before(int actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void before(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + } + + @Override + public TreeVisitor getVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + final JavaTemplate before = JavaTemplate + .builder("System.out.println(#{actual:any(int)});") + .build(); + final JavaTemplate before0 = JavaTemplate + .builder("System.out.println(#{actual:any(java.util.Map)});") + .build(); + final JavaTemplate after = JavaTemplate + .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") + .build(); + + @Override + public J visitMethodInvocation(J.MethodInvocation 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 + ); + } + if ((matcher = before0.matcher(getCursor())).find()) { + maybeRemoveImport("java.util.Map"); + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + return super.visitMethodInvocation(elem, ctx); + } + + }; + return Preconditions.check( + Preconditions.and( + new UsesType<>("java.util.Map", true), + new UsesMethod<>("java.io.PrintStream println(..)", true) + ), + javaVisitor + ); + } + } + + /** + * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType}. + */ + @SuppressWarnings("all") + @NullMarked + @Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") + public static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherTypeRecipe extends Recipe { + + /** + * Instantiates a new instance. + */ + public NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherTypeRecipe() {} + + @Override + public String getDisplayName() { + return "Refaster template `PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType`"; + } + + @Override + public String getDescription() { + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType {\n \n @BeforeTemplate()\n void before(String actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void before(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + } + + @Override + public TreeVisitor getVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + final JavaTemplate before = JavaTemplate + .builder("System.out.println(#{actual:any(java.lang.String)});") + .build(); + final JavaTemplate before0 = JavaTemplate + .builder("System.out.println(#{actual:any(java.util.Map)});") + .build(); + final JavaTemplate after = JavaTemplate + .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") + .build(); + + @Override + public J visitMethodInvocation(J.MethodInvocation 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 + ); + } + if ((matcher = before0.matcher(getCursor())).find()) { + maybeRemoveImport("java.util.Map"); + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + return super.visitMethodInvocation(elem, ctx); + } + + }; + return Preconditions.check( + Preconditions.and( + new UsesType<>("java.util.Map", true), + new UsesMethod<>("java.io.PrintStream println(..)", true) + ), + javaVisitor + ); + } + } + + /** + * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.UsesTypeMapWhenBeforeTemplateContainsMap}. + */ + @SuppressWarnings("all") + @NullMarked + @Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") + public static class UsesTypeMapWhenBeforeTemplateContainsMapRecipe extends Recipe { + + /** + * Instantiates a new instance. + */ + public UsesTypeMapWhenBeforeTemplateContainsMapRecipe() {} + + @Override + public String getDisplayName() { + return "Refaster template `PreConditionsVerifier.UsesTypeMapWhenBeforeTemplateContainsMap`"; + } + + @Override + public String getDescription() { + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapWhenBeforeTemplateContainsMap {\n \n @BeforeTemplate()\n void before(Map actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void beforeTwo(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + } + + @Override + public TreeVisitor getVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + final JavaTemplate before = JavaTemplate + .builder("System.out.println(#{actual:any(java.util.Map)});") + .build(); + final JavaTemplate beforeTwo = JavaTemplate + .builder("System.out.println(#{actual:any(java.util.Map)});") + .build(); + final JavaTemplate after = JavaTemplate + .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") + .build(); + + @Override + public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { + JavaTemplate.Matcher matcher; + if ((matcher = before.matcher(getCursor())).find()) { + maybeRemoveImport("java.util.Map"); + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + if ((matcher = beforeTwo.matcher(getCursor())).find()) { + maybeRemoveImport("java.util.Map"); + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + return super.visitMethodInvocation(elem, ctx); + } + + }; + return Preconditions.check( + Preconditions.and( + new UsesType<>("java.util.Map", true), + new UsesMethod<>("java.io.PrintStream println(..)", true) + ), + javaVisitor + ); + } + } + + /** + * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList}. + */ + @SuppressWarnings("all") + @NullMarked + @Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") + public static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndListRecipe extends Recipe { + + /** + * Instantiates a new instance. + */ + public UsesTypeMapOrListWhenBeforeTemplateContainsMapAndListRecipe() {} + + @Override + public String getDisplayName() { + return "Refaster template `PreConditionsVerifier.UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList`"; + } + + @Override + public String getDescription() { + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList {\n \n @BeforeTemplate()\n void before(List actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void beforeTwo(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + } + + @Override + public TreeVisitor getVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + final JavaTemplate before = JavaTemplate + .builder("System.out.println(#{actual:any()});") + .build(); + final JavaTemplate beforeTwo = JavaTemplate + .builder("System.out.println(#{actual:any(java.util.Map)});") + .build(); + final JavaTemplate after = JavaTemplate + .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") + .build(); + + @Override + public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { + JavaTemplate.Matcher matcher; + if ((matcher = before.matcher(getCursor())).find()) { + maybeRemoveImport("List"); + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + if ((matcher = beforeTwo.matcher(getCursor())).find()) { + maybeRemoveImport("java.util.Map"); + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + return super.visitMethodInvocation(elem, ctx); + } + + }; + return Preconditions.check( + Preconditions.and( + new UsesMethod<>("java.io.PrintStream println(..)", true), + Preconditions.or( + new UsesType<>("List", true), + new UsesType<>("java.util.Map", true) + ) + ), + javaVisitor + ); + } + } + +} From 495a43ee08f08dad3dfae6c33504d72bc7c1e0a9 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 12 Dec 2024 08:45:52 +0100 Subject: [PATCH 02/35] Starting point: PreConditionsVerifier --- .../java/template/RefasterTemplateProcessorTest.java | 6 +++--- ...fierRecipe.java => PreConditionsVerifierRecipes.java} | 9 ++++++--- 2 files changed, 9 insertions(+), 6 deletions(-) rename src/test/resources/refaster/{PreConditionsVerifierRecipe.java => PreConditionsVerifierRecipes.java} (98%) diff --git a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java index f5c8f4ef..3fe8f5b4 100644 --- a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java +++ b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java @@ -44,8 +44,7 @@ class RefasterTemplateProcessorTest { @ParameterizedTest @ValueSource(strings = { - "PreConditionsVerifier", - /*"Arrays", + "Arrays", "CharacterEscapeAnnotation", "MethodThrows", "NestedPreconditions", @@ -53,7 +52,7 @@ class RefasterTemplateProcessorTest { "UseStringIsEmpty", "SimplifyBooleans", "TwoVisitMethods", - "FindListAdd",*/ + "FindListAdd", }) void generateRecipe(String recipeName) { Compilation compilation = compileResource("refaster/" + recipeName + ".java"); @@ -97,6 +96,7 @@ void skipRecipeGeneration(String recipeName) { "SimplifyTernary", "RefasterAnyOf", "Parameters", + "PreConditionsVerifier", }) void nestedRecipes(String recipeName) { Compilation compilation = compileResource("refaster/" + recipeName + ".java"); diff --git a/src/test/resources/refaster/PreConditionsVerifierRecipe.java b/src/test/resources/refaster/PreConditionsVerifierRecipes.java similarity index 98% rename from src/test/resources/refaster/PreConditionsVerifierRecipe.java rename to src/test/resources/refaster/PreConditionsVerifierRecipes.java index 3fe8db01..735274e1 100644 --- a/src/test/resources/refaster/PreConditionsVerifierRecipe.java +++ b/src/test/resources/refaster/PreConditionsVerifierRecipes.java @@ -5,9 +5,12 @@ import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaParser; 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.function.*; import org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor; import org.openrewrite.java.tree.*; @@ -352,7 +355,7 @@ public String getDescription() { public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate before = JavaTemplate - .builder("System.out.println(#{actual:any()});") + .builder("System.out.println(#{actual:any(java.util.List)});") .build(); final JavaTemplate beforeTwo = JavaTemplate .builder("System.out.println(#{actual:any(java.util.Map)});") @@ -365,7 +368,7 @@ public TreeVisitor getVisitor() { public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; if ((matcher = before.matcher(getCursor())).find()) { - maybeRemoveImport("List"); + maybeRemoveImport("java.util.List"); return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), @@ -390,7 +393,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { Preconditions.and( new UsesMethod<>("java.io.PrintStream println(..)", true), Preconditions.or( - new UsesType<>("List", true), + new UsesType<>("java.util.List", true), new UsesType<>("java.util.Map", true) ) ), From 481e08444588fca020c29bac66f632bad538d47e Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 12 Dec 2024 10:37:39 +0100 Subject: [PATCH 03/35] Starting point: PreConditionsVerifier --- .../refaster/PreConditionsVerifier.java | 12 +++---- .../PreConditionsVerifierRecipes.java | 36 +++++++++---------- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/src/test/resources/refaster/PreConditionsVerifier.java b/src/test/resources/refaster/PreConditionsVerifier.java index 6c5880d8..9a3af55e 100644 --- a/src/test/resources/refaster/PreConditionsVerifier.java +++ b/src/test/resources/refaster/PreConditionsVerifier.java @@ -25,14 +25,14 @@ * A refaster template to test when a `UsesType`and Preconditions.or should or should not be applied to the Preconditions check. */ public class PreConditionsVerifier { - public static class NoUsesTypeWhenBeforeTemplateContainsPrimitive { + public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString { @BeforeTemplate - void before(int actual) { + void before(double actual, int ignore) { System.out.println(actual); } @BeforeTemplate - void beforeTwo(int actual) { + void beforeTwo(String actual, String ignore) { System.out.println(actual); } @@ -78,17 +78,17 @@ void after(Object actual) { public static class UsesTypeMapWhenBeforeTemplateContainsMap { @BeforeTemplate - void before(Map actual) { + void withGeneric(Map actual) { System.out.println(actual); } @BeforeTemplate - void beforeTwo(Map actual) { + void withGenericTwo(Map actual) { System.out.println(actual); } @AfterTemplate - void after(Object actual) { + void withoutGeneric(Map actual) { System.out.println("Changed: " + actual); } } diff --git a/src/test/resources/refaster/PreConditionsVerifierRecipes.java b/src/test/resources/refaster/PreConditionsVerifierRecipes.java index 735274e1..20fb2407 100644 --- a/src/test/resources/refaster/PreConditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreConditionsVerifierRecipes.java @@ -43,7 +43,7 @@ public String getDescription() { @Override public List getRecipeList() { return Arrays.asList( - new NoUsesTypeWhenBeforeTemplateContainsPrimitiveRecipe(), + new NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringRecipe(), new NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherTypeRecipe(), new NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherTypeRecipe(), new UsesTypeMapWhenBeforeTemplateContainsMapRecipe(), @@ -52,36 +52,36 @@ public List getRecipeList() { } /** - * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitive}. + * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString}. */ @SuppressWarnings("all") @NullMarked @Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") - public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveRecipe extends Recipe { + public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringRecipe extends Recipe { /** * Instantiates a new instance. */ - public NoUsesTypeWhenBeforeTemplateContainsPrimitiveRecipe() {} + public NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringRecipe() {} @Override public String getDisplayName() { - return "Refaster template `PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitive`"; + return "Refaster template `PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString`"; } @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitive {\n \n @BeforeTemplate()\n void before(int actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void beforeTwo(int actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString {\n \n @BeforeTemplate()\n void before(double actual, int ignore) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void beforeTwo(String actual, String ignore) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate before = JavaTemplate - .builder("System.out.println(#{actual:any(int)});") + .builder("System.out.println(#{actual:any(double)});") .build(); final JavaTemplate beforeTwo = JavaTemplate - .builder("System.out.println(#{actual:any(int)});") + .builder("System.out.println(#{actual:any(java.lang.String)});") .build(); final JavaTemplate after = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") @@ -277,38 +277,36 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapWhenBeforeTemplateContainsMap {\n \n @BeforeTemplate()\n void before(Map actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void beforeTwo(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapWhenBeforeTemplateContainsMap {\n \n @BeforeTemplate()\n void withGeneric(Map actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void withGenericTwo(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void withoutGeneric(Map actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { - final JavaTemplate before = JavaTemplate + final JavaTemplate withGeneric = JavaTemplate .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); - final JavaTemplate beforeTwo = JavaTemplate + final JavaTemplate withGenericTwo = JavaTemplate .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); - final JavaTemplate after = JavaTemplate - .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") + final JavaTemplate withoutGeneric = JavaTemplate + .builder("System.out.println(\"Changed: \" + #{actual:any(java.util.Map)});") .build(); @Override public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; - if ((matcher = before.matcher(getCursor())).find()) { - maybeRemoveImport("java.util.Map"); + if ((matcher = withGenericTwo.matcher(getCursor())).find()) { return embed( - after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + withoutGeneric.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), ctx, SHORTEN_NAMES ); } - if ((matcher = beforeTwo.matcher(getCursor())).find()) { - maybeRemoveImport("java.util.Map"); + if ((matcher = withGeneric.matcher(getCursor())).find()) { return embed( - after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + withoutGeneric.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), ctx, SHORTEN_NAMES From c3ceb416fd8eeb4b1e7c5a1555f94af2d4e06eb3 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 12 Dec 2024 11:29:41 +0100 Subject: [PATCH 04/35] Starting point: PreConditionsVerifier (failing tests as intended) --- .../refaster/PreConditionsVerifier.java | 62 ++++- .../PreConditionsVerifierRecipes.java | 220 ++++++++++++++---- 2 files changed, 229 insertions(+), 53 deletions(-) diff --git a/src/test/resources/refaster/PreConditionsVerifier.java b/src/test/resources/refaster/PreConditionsVerifier.java index 9a3af55e..6a112dbf 100644 --- a/src/test/resources/refaster/PreConditionsVerifier.java +++ b/src/test/resources/refaster/PreConditionsVerifier.java @@ -20,6 +20,7 @@ import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate; +import com.sun.tools.javac.util.Convert; /** * A refaster template to test when a `UsesType`and Preconditions.or should or should not be applied to the Preconditions check. @@ -27,12 +28,49 @@ public class PreConditionsVerifier { public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString { @BeforeTemplate - void before(double actual, int ignore) { + void doubleAndInt(double actual, int ignore) { System.out.println(actual); } @BeforeTemplate - void beforeTwo(String actual, String ignore) { + void stringAndString(String actual, String ignore) { + System.out.println(actual); + } + + @AfterTemplate + void after(Object actual) { + System.out.println("Changed: " + actual); + } + } + + public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody { + @BeforeTemplate + void doubleAndInt(double actual, String value) { + Convert.quote(value); + System.out.println(actual); + } + + @BeforeTemplate + void stringAndString(String actual, String value) { + System.out.println(actual); + } + + @AfterTemplate + void after(Object actual) { + System.out.println("Changed: " + actual); + } + } + + public static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody { + @BeforeTemplate + void doubleAndInt(double actual, String value) { + Convert.quote(value); + System.out.println(actual); + } + + @BeforeTemplate + void stringAndString(String actual, String value) { + Convert.quote(value); System.out.println(actual); } @@ -44,12 +82,12 @@ void after(Object actual) { public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType { @BeforeTemplate - void before(int actual) { + void _int(int actual) { System.out.println(actual); } @BeforeTemplate - void before(Map actual) { + void map(Map actual) { System.out.println(actual); } @@ -61,12 +99,12 @@ void after(Object actual) { public static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType { @BeforeTemplate - void before(String actual) { + void string(String actual) { System.out.println(actual); } @BeforeTemplate - void before(Map actual) { + void map(Map actual) { System.out.println(actual); } @@ -76,31 +114,31 @@ void after(Object actual) { } } - public static class UsesTypeMapWhenBeforeTemplateContainsMap { + public static class UsesTypeMapWhenAllBeforeTemplatesContainsMap { @BeforeTemplate - void withGeneric(Map actual) { + void mapWithGeneric(Map actual) { System.out.println(actual); } @BeforeTemplate - void withGenericTwo(Map actual) { + void mapWithGenericTwo(Map actual) { System.out.println(actual); } @AfterTemplate - void withoutGeneric(Map actual) { + void mapWithoutGeneric(Map actual) { System.out.println("Changed: " + actual); } } public static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList { @BeforeTemplate - void before(List actual) { + void list(List actual) { System.out.println(actual); } @BeforeTemplate - void beforeTwo(Map actual) { + void map(Map actual) { System.out.println(actual); } diff --git a/src/test/resources/refaster/PreConditionsVerifierRecipes.java b/src/test/resources/refaster/PreConditionsVerifierRecipes.java index 20fb2407..df538dfc 100644 --- a/src/test/resources/refaster/PreConditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreConditionsVerifierRecipes.java @@ -44,9 +44,11 @@ public String getDescription() { public List getRecipeList() { return Arrays.asList( new NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringRecipe(), + new NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBodyRecipe(), + new UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBodyRecipe(), new NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherTypeRecipe(), new NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherTypeRecipe(), - new UsesTypeMapWhenBeforeTemplateContainsMapRecipe(), + new UsesTypeMapWhenAllBeforeTemplatesContainsMapRecipe(), new UsesTypeMapOrListWhenBeforeTemplateContainsMapAndListRecipe() ); } @@ -71,16 +73,16 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString {\n \n @BeforeTemplate()\n void before(double actual, int ignore) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void beforeTwo(String actual, String ignore) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, int ignore) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String ignore) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { - final JavaTemplate before = JavaTemplate + final JavaTemplate doubleAndInt = JavaTemplate .builder("System.out.println(#{actual:any(double)});") .build(); - final JavaTemplate beforeTwo = JavaTemplate + final JavaTemplate stringAndString = JavaTemplate .builder("System.out.println(#{actual:any(java.lang.String)});") .build(); final JavaTemplate after = JavaTemplate @@ -90,7 +92,7 @@ public TreeVisitor getVisitor() { @Override public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; - if ((matcher = before.matcher(getCursor())).find()) { + if ((matcher = doubleAndInt.matcher(getCursor())).find()) { return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), @@ -98,7 +100,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - if ((matcher = beforeTwo.matcher(getCursor())).find()) { + if ((matcher = stringAndString.matcher(getCursor())).find()) { return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), @@ -117,6 +119,148 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { } } + /** + * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody}. + */ + @SuppressWarnings("all") + @NullMarked + @Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") + public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBodyRecipe extends Recipe { + + /** + * Instantiates a new instance. + */ + public NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBodyRecipe() {} + + @Override + public String getDisplayName() { + return "Refaster template `PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody`"; + } + + @Override + public String getDescription() { + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, String value) {\n Convert.quote(value);\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String value) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + } + + @Override + public TreeVisitor getVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + final JavaTemplate doubleAndInt = JavaTemplate + .builder("com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)});") + .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) + .build(); + final JavaTemplate stringAndString = JavaTemplate + .builder("System.out.println(#{actual:any(java.lang.String)});") + .build(); + final JavaTemplate after = JavaTemplate + .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") + .build(); + + @Override + public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { + JavaTemplate.Matcher matcher; + if ((matcher = doubleAndInt.matcher(getCursor())).find()) { + maybeRemoveImport("com.sun.tools.javac.util.Convert"); + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(1)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + if ((matcher = stringAndString.matcher(getCursor())).find()) { + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + return super.visitMethodInvocation(elem, ctx); + } + + }; + return Preconditions.check( + new UsesMethod<>("java.io.PrintStream println(..)", true), + javaVisitor + ); + } + } + + /** + * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody}. + */ + @SuppressWarnings("all") + @NullMarked + @Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") + public static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBodyRecipe extends Recipe { + + /** + * Instantiates a new instance. + */ + public UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBodyRecipe() {} + + @Override + public String getDisplayName() { + return "Refaster template `PreConditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody`"; + } + + @Override + public String getDescription() { + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, String value) {\n Convert.quote(value);\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String value) {\n Convert.quote(value);\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + } + + @Override + public TreeVisitor getVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + final JavaTemplate doubleAndInt = JavaTemplate + .builder("com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)});") + .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) + .build(); + final JavaTemplate stringAndString = JavaTemplate + .builder("com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)});") + .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) + .build(); + final JavaTemplate after = JavaTemplate + .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") + .build(); + + @Override + public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { + JavaTemplate.Matcher matcher; + if ((matcher = doubleAndInt.matcher(getCursor())).find()) { + maybeRemoveImport("com.sun.tools.javac.util.Convert"); + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(1)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + if ((matcher = stringAndString.matcher(getCursor())).find()) { + maybeRemoveImport("com.sun.tools.javac.util.Convert"); + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(1)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + return super.visitMethodInvocation(elem, ctx); + } + + }; + return Preconditions.check( + Preconditions.and( + new UsesType<>("com.sun.tools.javac.util.Convert", true), + new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true), + new UsesMethod<>("java.io.PrintStream println(..)", true) + ), + javaVisitor + ); + } + } + /** * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType}. */ @@ -137,16 +281,16 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType {\n \n @BeforeTemplate()\n void before(int actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void before(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType {\n \n @BeforeTemplate()\n void _int(int actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { - final JavaTemplate before = JavaTemplate + final JavaTemplate _int = JavaTemplate .builder("System.out.println(#{actual:any(int)});") .build(); - final JavaTemplate before0 = JavaTemplate + final JavaTemplate map = JavaTemplate .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); final JavaTemplate after = JavaTemplate @@ -156,7 +300,7 @@ public TreeVisitor getVisitor() { @Override public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; - if ((matcher = before.matcher(getCursor())).find()) { + if ((matcher = _int.matcher(getCursor())).find()) { return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), @@ -164,7 +308,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - if ((matcher = before0.matcher(getCursor())).find()) { + if ((matcher = map.matcher(getCursor())).find()) { maybeRemoveImport("java.util.Map"); return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), @@ -178,10 +322,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { }; return Preconditions.check( - Preconditions.and( - new UsesType<>("java.util.Map", true), - new UsesMethod<>("java.io.PrintStream println(..)", true) - ), + new UsesMethod<>("java.io.PrintStream println(..)", true), javaVisitor ); } @@ -207,16 +348,16 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType {\n \n @BeforeTemplate()\n void before(String actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void before(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType {\n \n @BeforeTemplate()\n void string(String actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { - final JavaTemplate before = JavaTemplate + final JavaTemplate string = JavaTemplate .builder("System.out.println(#{actual:any(java.lang.String)});") .build(); - final JavaTemplate before0 = JavaTemplate + final JavaTemplate map = JavaTemplate .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); final JavaTemplate after = JavaTemplate @@ -226,7 +367,7 @@ public TreeVisitor getVisitor() { @Override public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; - if ((matcher = before.matcher(getCursor())).find()) { + if ((matcher = string.matcher(getCursor())).find()) { return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), @@ -234,7 +375,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - if ((matcher = before0.matcher(getCursor())).find()) { + if ((matcher = map.matcher(getCursor())).find()) { maybeRemoveImport("java.util.Map"); return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), @@ -248,65 +389,62 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { }; return Preconditions.check( - Preconditions.and( - new UsesType<>("java.util.Map", true), - new UsesMethod<>("java.io.PrintStream println(..)", true) - ), + new UsesMethod<>("java.io.PrintStream println(..)", true), javaVisitor ); } } /** - * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.UsesTypeMapWhenBeforeTemplateContainsMap}. + * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.UsesTypeMapWhenAllBeforeTemplatesContainsMap}. */ @SuppressWarnings("all") @NullMarked @Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") - public static class UsesTypeMapWhenBeforeTemplateContainsMapRecipe extends Recipe { + public static class UsesTypeMapWhenAllBeforeTemplatesContainsMapRecipe extends Recipe { /** * Instantiates a new instance. */ - public UsesTypeMapWhenBeforeTemplateContainsMapRecipe() {} + public UsesTypeMapWhenAllBeforeTemplatesContainsMapRecipe() {} @Override public String getDisplayName() { - return "Refaster template `PreConditionsVerifier.UsesTypeMapWhenBeforeTemplateContainsMap`"; + return "Refaster template `PreConditionsVerifier.UsesTypeMapWhenAllBeforeTemplatesContainsMap`"; } @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapWhenBeforeTemplateContainsMap {\n \n @BeforeTemplate()\n void withGeneric(Map actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void withGenericTwo(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void withoutGeneric(Map actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapWhenAllBeforeTemplatesContainsMap {\n \n @BeforeTemplate()\n void mapWithGeneric(Map actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void mapWithGenericTwo(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void mapWithoutGeneric(Map actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { - final JavaTemplate withGeneric = JavaTemplate + final JavaTemplate mapWithGeneric = JavaTemplate .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); - final JavaTemplate withGenericTwo = JavaTemplate + final JavaTemplate mapWithGenericTwo = JavaTemplate .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); - final JavaTemplate withoutGeneric = JavaTemplate + final JavaTemplate mapWithoutGeneric = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.util.Map)});") .build(); @Override public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; - if ((matcher = withGenericTwo.matcher(getCursor())).find()) { + if ((matcher = mapWithGeneric.matcher(getCursor())).find()) { return embed( - withoutGeneric.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + mapWithoutGeneric.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), ctx, SHORTEN_NAMES ); } - if ((matcher = withGeneric.matcher(getCursor())).find()) { + if ((matcher = mapWithGenericTwo.matcher(getCursor())).find()) { return embed( - withoutGeneric.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + mapWithoutGeneric.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), ctx, SHORTEN_NAMES @@ -346,16 +484,16 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList {\n \n @BeforeTemplate()\n void before(List actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void beforeTwo(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList {\n \n @BeforeTemplate()\n void list(List actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { - final JavaTemplate before = JavaTemplate + final JavaTemplate list = JavaTemplate .builder("System.out.println(#{actual:any(java.util.List)});") .build(); - final JavaTemplate beforeTwo = JavaTemplate + final JavaTemplate map = JavaTemplate .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); final JavaTemplate after = JavaTemplate @@ -365,7 +503,7 @@ public TreeVisitor getVisitor() { @Override public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; - if ((matcher = before.matcher(getCursor())).find()) { + if ((matcher = list.matcher(getCursor())).find()) { maybeRemoveImport("java.util.List"); return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), @@ -374,7 +512,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - if ((matcher = beforeTwo.matcher(getCursor())).find()) { + if ((matcher = map.matcher(getCursor())).find()) { maybeRemoveImport("java.util.Map"); return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), From db6081f6371dab41ab2fea52396eef0c8eb8f3ef Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 12 Dec 2024 12:24:09 +0100 Subject: [PATCH 05/35] Starting point: PreConditionsVerifier (failing tests as intended) --- .../processor/RefasterTemplateProcessor.java | 8 +++++++- .../refaster/PreConditionsVerifier.java | 9 +++------ .../PreConditionsVerifierRecipes.java | 20 +++++++++---------- 3 files changed, 20 insertions(+), 17 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 154dfd68..20053a96 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -688,7 +688,13 @@ public void scan(JCTree jcTree) { jcIdent.sym.owner instanceof Symbol.MethodSymbol && ((Symbol.MethodSymbol) jcIdent.sym.owner).params.contains(jcIdent.sym) && seenParams.add(jcIdent.sym)) { - afterParams.add(beforeParamOrder.get(((Symbol.MethodSymbol) jcIdent.sym.owner).params.indexOf(jcIdent.sym))); + Integer idx = beforeParamOrder.get(((Symbol.MethodSymbol) jcIdent.sym.owner).params.indexOf(jcIdent.sym)); + // TODO fix ugly hack + if (idx == null && beforeParamOrder.size() == 1) { + afterParams.add(0); + } else { + afterParams.add(idx); + } } } super.scan(jcTree); diff --git a/src/test/resources/refaster/PreConditionsVerifier.java b/src/test/resources/refaster/PreConditionsVerifier.java index 6a112dbf..ad111b02 100644 --- a/src/test/resources/refaster/PreConditionsVerifier.java +++ b/src/test/resources/refaster/PreConditionsVerifier.java @@ -46,8 +46,7 @@ void after(Object actual) { public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody { @BeforeTemplate void doubleAndInt(double actual, String value) { - Convert.quote(value); - System.out.println(actual); + System.out.println(Convert.quote(value)); } @BeforeTemplate @@ -64,14 +63,12 @@ void after(Object actual) { public static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody { @BeforeTemplate void doubleAndInt(double actual, String value) { - Convert.quote(value); - System.out.println(actual); + System.out.println(Convert.quote(value)); } @BeforeTemplate void stringAndString(String actual, String value) { - Convert.quote(value); - System.out.println(actual); + System.out.println(Convert.quote(actual)); } @AfterTemplate diff --git a/src/test/resources/refaster/PreConditionsVerifierRecipes.java b/src/test/resources/refaster/PreConditionsVerifierRecipes.java index df538dfc..d23e3c86 100644 --- a/src/test/resources/refaster/PreConditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreConditionsVerifierRecipes.java @@ -139,14 +139,14 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, String value) {\n Convert.quote(value);\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String value) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, String value) {\n System.out.println(Convert.quote(value));\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String value) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate doubleAndInt = JavaTemplate - .builder("com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)});") + .builder("System.out.println(com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)}));") .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) .build(); final JavaTemplate stringAndString = JavaTemplate @@ -162,7 +162,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { if ((matcher = doubleAndInt.matcher(getCursor())).find()) { maybeRemoveImport("com.sun.tools.javac.util.Convert"); return embed( - after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(1)), + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), ctx, SHORTEN_NAMES @@ -207,18 +207,18 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, String value) {\n Convert.quote(value);\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String value) {\n Convert.quote(value);\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, String value) {\n System.out.println(Convert.quote(value));\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String value) {\n System.out.println(Convert.quote(actual));\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate doubleAndInt = JavaTemplate - .builder("com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)});") + .builder("System.out.println(com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)}));") .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) .build(); final JavaTemplate stringAndString = JavaTemplate - .builder("com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)});") + .builder("System.out.println(com.sun.tools.javac.util.Convert.quote(#{actual:any(java.lang.String)}));") .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) .build(); final JavaTemplate after = JavaTemplate @@ -231,7 +231,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { if ((matcher = doubleAndInt.matcher(getCursor())).find()) { maybeRemoveImport("com.sun.tools.javac.util.Convert"); return embed( - after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(1)), + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), ctx, SHORTEN_NAMES @@ -240,7 +240,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { if ((matcher = stringAndString.matcher(getCursor())).find()) { maybeRemoveImport("com.sun.tools.javac.util.Convert"); return embed( - after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(1)), + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), ctx, SHORTEN_NAMES @@ -253,8 +253,8 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { return Preconditions.check( Preconditions.and( new UsesType<>("com.sun.tools.javac.util.Convert", true), - new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true), - new UsesMethod<>("java.io.PrintStream println(..)", true) + new UsesMethod<>("java.io.PrintStream println(..)", true), + new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true) ), javaVisitor ); From d5559a9d1abacb7a259f9f92c55a6666210aafab Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 12 Dec 2024 13:44:28 +0100 Subject: [PATCH 06/35] Fix unit test --- .../refaster/PreConditionsVerifier.java | 25 +++++------ .../PreConditionsVerifierRecipes.java | 41 +++++++++---------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/test/resources/refaster/PreConditionsVerifier.java b/src/test/resources/refaster/PreConditionsVerifier.java index ad111b02..5b6ec4ed 100644 --- a/src/test/resources/refaster/PreConditionsVerifier.java +++ b/src/test/resources/refaster/PreConditionsVerifier.java @@ -20,6 +20,7 @@ import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate; +import com.sun.tools.javac.util.Constants; import com.sun.tools.javac.util.Convert; /** @@ -45,35 +46,35 @@ void after(Object actual) { public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody { @BeforeTemplate - void doubleAndInt(double actual, String value) { - System.out.println(Convert.quote(value)); + String string(String value) { + return Convert.quote(value); } @BeforeTemplate - void stringAndString(String actual, String value) { - System.out.println(actual); + String _int(int value) { + return String.valueOf(value); } @AfterTemplate - void after(Object actual) { - System.out.println("Changed: " + actual); + Object after(Object actual) { + return Convert.quote(String.valueOf(actual)); } } public static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody { @BeforeTemplate - void doubleAndInt(double actual, String value) { - System.out.println(Convert.quote(value)); + String string(String value) { + return Convert.quote(value); } @BeforeTemplate - void stringAndString(String actual, String value) { - System.out.println(Convert.quote(actual)); + String _int(int value) { + return Convert.quote(String.valueOf(value)); } @AfterTemplate - void after(Object actual) { - System.out.println("Changed: " + actual); + Object after(Object actual) { + return Convert.quote(String.valueOf(actual)); } } diff --git a/src/test/resources/refaster/PreConditionsVerifierRecipes.java b/src/test/resources/refaster/PreConditionsVerifierRecipes.java index d23e3c86..c55586a3 100644 --- a/src/test/resources/refaster/PreConditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreConditionsVerifierRecipes.java @@ -139,28 +139,28 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, String value) {\n System.out.println(Convert.quote(value));\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String value) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody {\n \n @BeforeTemplate()\n String string(String value) {\n return Convert.quote(value);\n }\n \n @BeforeTemplate()\n String _int(int value) {\n return String.valueOf(value);\n }\n \n @AfterTemplate()\n Object after(Object actual) {\n return Convert.quote(String.valueOf(actual));\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { - final JavaTemplate doubleAndInt = JavaTemplate - .builder("System.out.println(com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)}));") + final JavaTemplate string = JavaTemplate + .builder("com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)})") .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) .build(); - final JavaTemplate stringAndString = JavaTemplate - .builder("System.out.println(#{actual:any(java.lang.String)});") + final JavaTemplate _int = JavaTemplate + .builder("String.valueOf(#{value:any(int)})") .build(); final JavaTemplate after = JavaTemplate - .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") + .builder("com.sun.tools.javac.util.Convert.quote(String.valueOf(#{actual:any(java.lang.Object)}))") + .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) .build(); @Override public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; - if ((matcher = doubleAndInt.matcher(getCursor())).find()) { - maybeRemoveImport("com.sun.tools.javac.util.Convert"); + if ((matcher = string.matcher(getCursor())).find()) { return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), @@ -168,7 +168,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - if ((matcher = stringAndString.matcher(getCursor())).find()) { + if ((matcher = _int.matcher(getCursor())).find()) { return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), @@ -207,29 +207,29 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, String value) {\n System.out.println(Convert.quote(value));\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String value) {\n System.out.println(Convert.quote(actual));\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody {\n \n @BeforeTemplate()\n String string(String value) {\n return Convert.quote(value);\n }\n \n @BeforeTemplate()\n String _int(int value) {\n return Convert.quote(String.valueOf(value));\n }\n \n @AfterTemplate()\n Object after(Object actual) {\n return Convert.quote(String.valueOf(actual));\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { - final JavaTemplate doubleAndInt = JavaTemplate - .builder("System.out.println(com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)}));") + final JavaTemplate string = JavaTemplate + .builder("com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)})") .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) .build(); - final JavaTemplate stringAndString = JavaTemplate - .builder("System.out.println(com.sun.tools.javac.util.Convert.quote(#{actual:any(java.lang.String)}));") + final JavaTemplate _int = JavaTemplate + .builder("com.sun.tools.javac.util.Convert.quote(String.valueOf(#{value:any(int)}))") .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) .build(); final JavaTemplate after = JavaTemplate - .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") + .builder("com.sun.tools.javac.util.Convert.quote(String.valueOf(#{actual:any(java.lang.Object)}))") + .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) .build(); @Override public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; - if ((matcher = doubleAndInt.matcher(getCursor())).find()) { - maybeRemoveImport("com.sun.tools.javac.util.Convert"); + if ((matcher = string.matcher(getCursor())).find()) { return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), @@ -237,8 +237,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - if ((matcher = stringAndString.matcher(getCursor())).find()) { - maybeRemoveImport("com.sun.tools.javac.util.Convert"); + if ((matcher = _int.matcher(getCursor())).find()) { return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), getCursor(), @@ -253,8 +252,8 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { return Preconditions.check( Preconditions.and( new UsesType<>("com.sun.tools.javac.util.Convert", true), - new UsesMethod<>("java.io.PrintStream println(..)", true), - new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true) + new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true), + new UsesMethod<>("java.lang.String valueOf(..)", true) ), javaVisitor ); From 51c5146fc9c9cd28011c151428698383fc32c9c3 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 12 Dec 2024 15:46:22 +0100 Subject: [PATCH 07/35] Implementation --- .../processor/RefasterTemplateProcessor.java | 32 +++++++++++++++---- .../PreConditionsVerifierRecipes.java | 9 ++---- 2 files changed, 27 insertions(+), 14 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 20053a96..dda7b679 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -50,6 +50,7 @@ import static java.util.Collections.singletonList; import static java.util.Objects.requireNonNull; +import static java.util.function.Function.identity; import static java.util.stream.Collectors.*; import static org.openrewrite.java.template.processor.RefasterTemplateProcessor.AFTER_TEMPLATE; import static org.openrewrite.java.template.processor.RefasterTemplateProcessor.BEFORE_TEMPLATE; @@ -614,6 +615,8 @@ private Set getImportsAsStrings(Map> imp } } + prunePreconditions(preconditions, beforeTemplates); + if (preconditions.size() == 1) { return joinPreconditions(preconditions.values().iterator().next(), "and", indent + 4); } else if (preconditions.size() > 1) { @@ -639,6 +642,27 @@ private Set getImportsAsStrings(Map> imp return null; } + /** + * If there are multiple @BeforeTemplates, it means that one of the beforeTemplates actually corresponds to a piece of code. + * If one @BeforeTemplate uses a type argument and another @BeforeTemplate has at least some primitives or strings as arguments, + * then the latter template will not be executed because it is inadvertently disabled by the type precondition of the former. + * So in that case prune the preconditions, because we don't want to use preconditions to which not all @BeforeTemplates apply. + */ + private void prunePreconditions(Map> preconditions, List beforeTemplates) { + if (beforeTemplates.size() > 1 && beforeTemplates.stream().anyMatch(it -> it.method.params.stream().anyMatch( + vd -> vd.sym.type.isPrimitive() || vd.sym.type.toString().equals("java.lang.String")) + )) { + List preconditionsValidForAllMethods = preconditions.values().stream() + .flatMap(Set::stream) + .collect(groupingBy(identity(), counting())) + .entrySet().stream() + .filter(it -> it.getValue() == preconditions.size()) + .map(Map.Entry::getKey) + .collect(toList()); + preconditions.values().forEach(set -> set.removeIf(value -> !preconditionsValidForAllMethods.contains(value))); + } + } + private String joinPreconditions(Collection preconditions, String op, int indent) { if (preconditions.isEmpty()) { return null; @@ -688,13 +712,7 @@ public void scan(JCTree jcTree) { jcIdent.sym.owner instanceof Symbol.MethodSymbol && ((Symbol.MethodSymbol) jcIdent.sym.owner).params.contains(jcIdent.sym) && seenParams.add(jcIdent.sym)) { - Integer idx = beforeParamOrder.get(((Symbol.MethodSymbol) jcIdent.sym.owner).params.indexOf(jcIdent.sym)); - // TODO fix ugly hack - if (idx == null && beforeParamOrder.size() == 1) { - afterParams.add(0); - } else { - afterParams.add(idx); - } + afterParams.add(beforeParamOrder.get(((Symbol.MethodSymbol) jcIdent.sym.owner).params.indexOf(jcIdent.sym))); } } super.scan(jcTree); diff --git a/src/test/resources/refaster/PreConditionsVerifierRecipes.java b/src/test/resources/refaster/PreConditionsVerifierRecipes.java index c55586a3..68fb6ab0 100644 --- a/src/test/resources/refaster/PreConditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreConditionsVerifierRecipes.java @@ -144,7 +144,7 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { - JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + return new AbstractRefasterJavaVisitor() { final JavaTemplate string = JavaTemplate .builder("com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)})") .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) @@ -180,10 +180,6 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { } }; - return Preconditions.check( - new UsesMethod<>("java.io.PrintStream println(..)", true), - javaVisitor - ); } } @@ -252,8 +248,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { return Preconditions.check( Preconditions.and( new UsesType<>("com.sun.tools.javac.util.Convert", true), - new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true), - new UsesMethod<>("java.lang.String valueOf(..)", true) + new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true) ), javaVisitor ); From 3e4bfebc6b1d3cca627f4943257bea31ce74d418 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 12 Dec 2024 16:34:16 +0100 Subject: [PATCH 08/35] Implementation --- .../processor/RefasterTemplateProcessor.java | 46 +++++++++++++------ .../refaster/PreConditionsVerifier.java | 2 +- .../PreConditionsVerifierRecipes.java | 42 +++++++++++++---- 3 files changed, 67 insertions(+), 23 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 dda7b679..1bdfa2c8 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -615,7 +615,9 @@ private Set getImportsAsStrings(Map> imp } } - prunePreconditions(preconditions, beforeTemplates); + if (shouldPrune(beforeTemplates)) { + prunePreconditions(preconditions, beforeTemplates); + } if (preconditions.size() == 1) { return joinPreconditions(preconditions.values().iterator().next(), "and", indent + 4); @@ -648,19 +650,35 @@ private Set getImportsAsStrings(Map> imp * then the latter template will not be executed because it is inadvertently disabled by the type precondition of the former. * So in that case prune the preconditions, because we don't want to use preconditions to which not all @BeforeTemplates apply. */ - private void prunePreconditions(Map> preconditions, List beforeTemplates) { - if (beforeTemplates.size() > 1 && beforeTemplates.stream().anyMatch(it -> it.method.params.stream().anyMatch( - vd -> vd.sym.type.isPrimitive() || vd.sym.type.toString().equals("java.lang.String")) - )) { - List preconditionsValidForAllMethods = preconditions.values().stream() - .flatMap(Set::stream) - .collect(groupingBy(identity(), counting())) - .entrySet().stream() - .filter(it -> it.getValue() == preconditions.size()) - .map(Map.Entry::getKey) - .collect(toList()); - preconditions.values().forEach(set -> set.removeIf(value -> !preconditionsValidForAllMethods.contains(value))); + private boolean shouldPrune(List beforeTemplates) { + if (beforeTemplates.size() < 2) { + return false; + } + + boolean hasType = false; + boolean hasPrimitiveOrString = false; + for (TemplateDescriptor beforeTemplate : beforeTemplates) { + for (JCTree.JCVariableDecl vd : beforeTemplate.method.params) { + boolean check = vd.sym.type.isPrimitive() || vd.sym.type.toString().equals("java.lang.String"); + hasType |= !check; + hasPrimitiveOrString |= check; + if (hasType && hasPrimitiveOrString) { + return true; + } + } } + return false; + } + + private void prunePreconditions(Map> preconditions, List beforeTemplates) { + List preconditionsValidForAllMethods = preconditions.values().stream() + .flatMap(Set::stream) + .collect(groupingBy(identity(), counting())) + .entrySet().stream() + .filter(it -> it.getValue() == preconditions.size()) + .map(Map.Entry::getKey) + .collect(toList()); + preconditions.values().forEach(set -> set.removeIf(value -> !preconditionsValidForAllMethods.contains(value))); } private String joinPreconditions(Collection preconditions, String op, int indent) { @@ -775,7 +793,7 @@ public RuleDescriptor(JCTree.JCClassDecl classDecl, JCCompilationUnit cu, Contex this.context = context; } - private RefasterTemplateProcessor.@Nullable RuleDescriptor validate() { + private @Nullable RuleDescriptor validate() { if (beforeTemplates.isEmpty()) { return null; } diff --git a/src/test/resources/refaster/PreConditionsVerifier.java b/src/test/resources/refaster/PreConditionsVerifier.java index 5b6ec4ed..61a7f832 100644 --- a/src/test/resources/refaster/PreConditionsVerifier.java +++ b/src/test/resources/refaster/PreConditionsVerifier.java @@ -44,7 +44,7 @@ void after(Object actual) { } } - public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody { + public static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody { @BeforeTemplate String string(String value) { return Convert.quote(value); diff --git a/src/test/resources/refaster/PreConditionsVerifierRecipes.java b/src/test/resources/refaster/PreConditionsVerifierRecipes.java index 68fb6ab0..8d8a6f45 100644 --- a/src/test/resources/refaster/PreConditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreConditionsVerifierRecipes.java @@ -1,3 +1,18 @@ +/* + * 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 foo; import org.jspecify.annotations.NullMarked; @@ -44,7 +59,7 @@ public String getDescription() { public List getRecipeList() { return Arrays.asList( new NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringRecipe(), - new NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBodyRecipe(), + new UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBodyRecipe(), new UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBodyRecipe(), new NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherTypeRecipe(), new NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherTypeRecipe(), @@ -120,31 +135,31 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { } /** - * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody}. + * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody}. */ @SuppressWarnings("all") @NullMarked @Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") - public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBodyRecipe extends Recipe { + public static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBodyRecipe extends Recipe { /** * Instantiates a new instance. */ - public NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBodyRecipe() {} + public UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBodyRecipe() {} @Override public String getDisplayName() { - return "Refaster template `PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody`"; + return "Refaster template `PreConditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody`"; } @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody {\n \n @BeforeTemplate()\n String string(String value) {\n return Convert.quote(value);\n }\n \n @BeforeTemplate()\n String _int(int value) {\n return String.valueOf(value);\n }\n \n @AfterTemplate()\n Object after(Object actual) {\n return Convert.quote(String.valueOf(actual));\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody {\n \n @BeforeTemplate()\n String string(String value) {\n return Convert.quote(value);\n }\n \n @BeforeTemplate()\n String _int(int value) {\n return String.valueOf(value);\n }\n \n @AfterTemplate()\n Object after(Object actual) {\n return Convert.quote(String.valueOf(actual));\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { - return new AbstractRefasterJavaVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate string = JavaTemplate .builder("com.sun.tools.javac.util.Convert.quote(#{value:any(java.lang.String)})") .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())) @@ -180,6 +195,16 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { } }; + return Preconditions.check( + Preconditions.or( + Preconditions.and( + new UsesType<>("com.sun.tools.javac.util.Convert", true), + new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true) + ), + new UsesMethod<>("java.lang.String valueOf(..)", true) + ), + javaVisitor + ); } } @@ -248,7 +273,8 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { return Preconditions.check( Preconditions.and( new UsesType<>("com.sun.tools.javac.util.Convert", true), - new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true) + new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true), + new UsesMethod<>("java.lang.String valueOf(..)", true) ), javaVisitor ); From c4528dfd318c14c3d2b08483a6270cc7f1de8b20 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 12 Dec 2024 16:47:55 +0100 Subject: [PATCH 09/35] improvement --- .../java/template/processor/RefasterTemplateProcessor.java | 4 ++-- 1 file changed, 2 insertions(+), 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 1bdfa2c8..84f7a492 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -616,7 +616,7 @@ private Set getImportsAsStrings(Map> imp } if (shouldPrune(beforeTemplates)) { - prunePreconditions(preconditions, beforeTemplates); + prunePreconditions(preconditions); } if (preconditions.size() == 1) { @@ -670,7 +670,7 @@ private boolean shouldPrune(List beforeTemplates) { return false; } - private void prunePreconditions(Map> preconditions, List beforeTemplates) { + private void prunePreconditions(Map> preconditions) { List preconditionsValidForAllMethods = preconditions.values().stream() .flatMap(Set::stream) .collect(groupingBy(identity(), counting())) From 0202ed37ceebbcc5959a40f8a9a5bbc72080af12 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 12 Dec 2024 16:59:16 +0100 Subject: [PATCH 10/35] improvement --- .../processor/RefasterTemplateProcessor.java | 119 +++++++++--------- 1 file changed, 59 insertions(+), 60 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 84f7a492..62cbf073 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -48,6 +48,7 @@ import java.util.function.Predicate; import java.util.stream.Stream; +import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static java.util.Objects.requireNonNull; import static java.util.function.Function.identity; @@ -309,11 +310,11 @@ private void writeRecipeClass(JCTree.JCClassDecl classDecl, boolean outerClassRe .collect(joining(",\n")); out.write( " @Override\n" + - " public List getRecipeList() {\n" + - " return Arrays.asList(\n" + - recipesAsList + '\n' + - " );\n" + - " }\n\n"); + " public List getRecipeList() {\n" + + " return Arrays.asList(\n" + + recipesAsList + '\n' + + " );\n" + + " }\n\n"); for (String r : recipes.values()) { out.write(r.replaceAll("(?m)^(.+)$", " $1")); @@ -415,7 +416,7 @@ private String generateVisitMethod(Map beforeTemplat List embedOptions = new ArrayList<>(); JCTree.JCExpression afterReturn = getReturnExpression(descriptor.afterTemplate.method); if (afterReturn instanceof JCTree.JCParens || - afterReturn instanceof JCTree.JCUnary && ((JCTree.JCUnary) afterReturn).getExpression() instanceof JCTree.JCParens) { + afterReturn instanceof JCTree.JCUnary && ((JCTree.JCUnary) afterReturn).getExpression() instanceof JCTree.JCParens) { embedOptions.add("REMOVE_PARENS"); } // TODO check if after template contains type or member references @@ -538,28 +539,28 @@ private String recipeDescriptor(JCTree.JCClassDecl classDecl, String defaultDisp } String recipeDescriptor = " @Override\n" + - " public String getDisplayName() {\n" + - " return \"" + displayName + "\";\n" + - " }\n" + - "\n" + - " @Override\n" + - " public String getDescription() {\n" + - " return \"" + description + "\";\n" + - " }\n" + - "\n"; + " public String getDisplayName() {\n" + + " return \"" + displayName + "\";\n" + + " }\n" + + "\n" + + " @Override\n" + + " public String getDescription() {\n" + + " return \"" + description + "\";\n" + + " }\n" + + "\n"; if (tags.size() == 1) { recipeDescriptor += " @Override\n" + - " public Set getTags() {\n" + - " return Collections.singleton(\"" + String.join("\", \"", tags) + "\");\n" + - " }\n" + - "\n"; + " public Set getTags() {\n" + + " return Collections.singleton(\"" + String.join("\", \"", tags) + "\");\n" + + " }\n" + + "\n"; } else if (tags.size() > 1) { recipeDescriptor += " @Override\n" + - " public Set getTags() {\n" + - " return new HashSet<>(Arrays.asList(\"" + String.join("\", \"", tags) + "\"));\n" + - " }\n" + - "\n"; + " public Set getTags() {\n" + + " return new HashSet<>(Arrays.asList(\"" + String.join("\", \"", tags) + "\"));\n" + + " }\n" + + "\n"; } return recipeDescriptor; @@ -615,27 +616,26 @@ private Set getImportsAsStrings(Map> imp } } - if (shouldPrune(beforeTemplates)) { - prunePreconditions(preconditions); - } + Collection> values = + shouldPrune(beforeTemplates) ? prune(preconditions) : preconditions.values(); - if (preconditions.size() == 1) { - return joinPreconditions(preconditions.values().iterator().next(), "and", indent + 4); - } else if (preconditions.size() > 1) { + if (values.size() == 1) { + return joinPreconditions(values.iterator().next(), "and", indent + 4); + } else if (values.size() > 1) { Set common = new LinkedHashSet<>(); - for (String dep : preconditions.values().iterator().next()) { - if (preconditions.values().stream().allMatch(v -> v.contains(dep))) { + for (String dep : values.iterator().next()) { + if (values.stream().allMatch(v -> v.contains(dep))) { common.add(dep); } } - common.forEach(dep -> preconditions.values().forEach(v -> v.remove(dep))); - preconditions.values().removeIf(Collection::isEmpty); + common.forEach(dep -> values.forEach(v -> v.remove(dep))); + values.removeIf(Collection::isEmpty); if (common.isEmpty()) { - return joinPreconditions(preconditions.values().stream().map(v -> joinPreconditions(v, "and", indent + 4)).collect(toList()), "or", indent + 4); + return joinPreconditions(values.stream().map(v -> joinPreconditions(v, "and", indent + 4)).collect(toList()), "or", indent + 4); } else { if (!preconditions.isEmpty()) { - String uniqueConditions = joinPreconditions(preconditions.values().stream().map(v -> joinPreconditions(v, "and", indent + 12)).collect(toList()), "or", indent + 8); + String uniqueConditions = joinPreconditions(values.stream().map(v -> joinPreconditions(v, "and", indent + 12)).collect(toList()), "or", indent + 8); common.add(uniqueConditions); } return joinPreconditions(common, "and", indent + 4); @@ -670,15 +670,14 @@ private boolean shouldPrune(List beforeTemplates) { return false; } - private void prunePreconditions(Map> preconditions) { - List preconditionsValidForAllMethods = preconditions.values().stream() + private Collection> prune(Map> preconditions) { + return singleton(preconditions.values().stream() .flatMap(Set::stream) .collect(groupingBy(identity(), counting())) .entrySet().stream() .filter(it -> it.getValue() == preconditions.size()) .map(Map.Entry::getKey) - .collect(toList()); - preconditions.values().forEach(set -> set.removeIf(value -> !preconditionsValidForAllMethods.contains(value))); + .collect(toSet())); } private String joinPreconditions(Collection preconditions, String op, int indent) { @@ -710,9 +709,9 @@ public void scan(JCTree jcTree) { if (jcTree instanceof JCTree.JCIdent) { JCTree.JCIdent jcIdent = (JCTree.JCIdent) jcTree; if (jcIdent.sym instanceof Symbol.VarSymbol && - jcIdent.sym.owner instanceof Symbol.MethodSymbol && - ((Symbol.MethodSymbol) jcIdent.sym.owner).params.contains(jcIdent.sym) && - seenParams.add(jcIdent.sym)) { + jcIdent.sym.owner instanceof Symbol.MethodSymbol && + ((Symbol.MethodSymbol) jcIdent.sym.owner).params.contains(jcIdent.sym) && + seenParams.add(jcIdent.sym)) { beforeParamOrder.put(((Symbol.MethodSymbol) jcIdent.sym.owner).params.indexOf(jcIdent.sym), beforeParameterOccurrence.getAndIncrement()); } } @@ -727,9 +726,9 @@ public void scan(JCTree jcTree) { if (jcTree instanceof JCTree.JCIdent) { JCTree.JCIdent jcIdent = (JCTree.JCIdent) jcTree; if (jcIdent.sym instanceof Symbol.VarSymbol && - jcIdent.sym.owner instanceof Symbol.MethodSymbol && - ((Symbol.MethodSymbol) jcIdent.sym.owner).params.contains(jcIdent.sym) && - seenParams.add(jcIdent.sym)) { + jcIdent.sym.owner instanceof Symbol.MethodSymbol && + ((Symbol.MethodSymbol) jcIdent.sym.owner).params.contains(jcIdent.sym) && + seenParams.add(jcIdent.sym)) { afterParams.add(beforeParamOrder.get(((Symbol.MethodSymbol) jcIdent.sym.owner).params.indexOf(jcIdent.sym))); } } @@ -805,7 +804,7 @@ public RuleDescriptor(JCTree.JCClassDecl classDecl, JCCompilationUnit cu, Contex for (JCTree member : classDecl.getMembers()) { if (member instanceof JCTree.JCMethodDecl && beforeTemplates.stream().noneMatch(t -> t.method == member) && - (afterTemplate == null || member != afterTemplate.method)) { + (afterTemplate == null || member != afterTemplate.method)) { for (JCTree.JCAnnotation annotation : getTemplateAnnotations(((JCTree.JCMethodDecl) member), UNSUPPORTED_ANNOTATIONS::contains)) { printNoteOnce("@" + annotation.annotationType + " is currently not supported", classDecl.sym); return null; @@ -884,7 +883,7 @@ private boolean isAnyOfCall(JCTree.JCMethodInvocation call) { if (meth instanceof JCTree.JCFieldAccess) { JCTree.JCFieldAccess fieldAccess = (JCTree.JCFieldAccess) meth; return fieldAccess.name.toString().equals("anyOf") && - ((JCTree.JCIdent) fieldAccess.selected).name.toString().equals("Refaster"); + ((JCTree.JCIdent) fieldAccess.selected).name.toString().equals("Refaster"); } return false; } @@ -988,7 +987,7 @@ boolean validate(JCTree tree) { @Override public void visitSelect(JCTree.JCFieldAccess jcFieldAccess) { if (jcFieldAccess.selected.type.tsym.toString().equals("com.google.errorprone.refaster.Refaster") && - jcFieldAccess.name.toString().equals("anyOf")) { + jcFieldAccess.name.toString().equals("anyOf")) { // exception for `Refaster.anyOf()` if (++anyOfCount > 1) { printNoteOnce("Refaster.anyOf() can only be used once per template", classDecl.sym); @@ -1002,8 +1001,8 @@ public void visitSelect(JCTree.JCFieldAccess jcFieldAccess) { @Override public void visitIdent(JCTree.JCIdent jcIdent) { if (valid && - jcIdent.sym != null && - jcIdent.sym.packge().getQualifiedName().contentEquals("com.google.errorprone.refaster")) { + jcIdent.sym != null && + jcIdent.sym.packge().getQualifiedName().contentEquals("com.google.errorprone.refaster")) { printNoteOnce(jcIdent.type.tsym.getQualifiedName() + " is currently not supported", classDecl.sym); valid = false; } @@ -1122,16 +1121,16 @@ private static List getTemplateAnnotations(MethodTree metho for (AnnotationTree annotation : method.getModifiers().getAnnotations()) { Tree type = annotation.getAnnotationType(); if (type.getKind() == Tree.Kind.IDENTIFIER && ((JCTree.JCIdent) type).sym != null && - typePredicate.test(((JCTree.JCIdent) type).sym.getQualifiedName().toString())) { + typePredicate.test(((JCTree.JCIdent) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } else if (type.getKind() == Tree.Kind.IDENTIFIER && ((JCTree.JCAnnotation) annotation).attribute != null && - ((JCTree.JCAnnotation) annotation).attribute.type instanceof Type.ClassType && - ((JCTree.JCAnnotation) annotation).attribute.type.tsym != null && - typePredicate.test(((JCTree.JCAnnotation) annotation).attribute.type.tsym.getQualifiedName().toString())) { + ((JCTree.JCAnnotation) annotation).attribute.type instanceof Type.ClassType && + ((JCTree.JCAnnotation) annotation).attribute.type.tsym != null && + typePredicate.test(((JCTree.JCAnnotation) annotation).attribute.type.tsym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } else if (type.getKind() == Tree.Kind.MEMBER_SELECT && type instanceof JCTree.JCFieldAccess && - ((JCTree.JCFieldAccess) type).sym != null && - typePredicate.test(((JCTree.JCFieldAccess) type).sym.getQualifiedName().toString())) { + ((JCTree.JCFieldAccess) type).sym != null && + typePredicate.test(((JCTree.JCFieldAccess) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } } @@ -1143,12 +1142,12 @@ private static List getTemplateAnnotations(VariableTree par for (AnnotationTree annotation : parameter.getModifiers().getAnnotations()) { Tree type = annotation.getAnnotationType(); if (type.getKind() == Tree.Kind.IDENTIFIER && - ((JCTree.JCIdent) type).sym != null && - typePredicate.test(((JCTree.JCIdent) type).sym.getQualifiedName().toString())) { + ((JCTree.JCIdent) type).sym != null && + typePredicate.test(((JCTree.JCIdent) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } else if (type.getKind() == Tree.Kind.MEMBER_SELECT && type instanceof JCTree.JCFieldAccess && - ((JCTree.JCFieldAccess) type).sym != null && - typePredicate.test(((JCTree.JCFieldAccess) type).sym.getQualifiedName().toString())) { + ((JCTree.JCFieldAccess) type).sym != null && + typePredicate.test(((JCTree.JCFieldAccess) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } } From 6714f48bce30068b2daa7d495c5820e28385117f Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 12 Dec 2024 17:12:59 +0100 Subject: [PATCH 11/35] Fix test --- src/test/resources/refaster/PreConditionsVerifierRecipes.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/resources/refaster/PreConditionsVerifierRecipes.java b/src/test/resources/refaster/PreConditionsVerifierRecipes.java index 8d8a6f45..e9159e04 100644 --- a/src/test/resources/refaster/PreConditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreConditionsVerifierRecipes.java @@ -273,8 +273,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { return Preconditions.check( Preconditions.and( new UsesType<>("com.sun.tools.javac.util.Convert", true), - new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true), - new UsesMethod<>("java.lang.String valueOf(..)", true) + new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true) ), javaVisitor ); From f8dd36dcdd9ab16730ae6ac70ded47b3e501e121 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 12 Dec 2024 17:28:45 +0100 Subject: [PATCH 12/35] Add extra info about pruning --- .../processor/RefasterTemplateProcessor.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) 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 62cbf073..cfdb54a3 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -655,6 +655,39 @@ private boolean shouldPrune(List beforeTemplates) { return false; } + /* + If the types of the body (beforeTemplate.usedTypes) are not all different, like: + ```java + @BeforeTemplate + String string(String value) { + return Convert.quote(value); + } + + @BeforeTemplate + String _int(int value) { + return Convert.quote(String.valueOf(value)); + } + ``` + then a prune should be applied as well (both use `Convert` type, but `String` type is only used in second @BeforeTemplate). + + + So + ```java + @BeforeTemplate + Map hashMap(int size) { + return new HashMap(size); + } + + @BeforeTemplate + Map linkedHashMap(int size) { + return new LinkedHashMap(size); + } + ```java + should not be pruned (first @BeforeTemplate uses HashMap type, second @BeforeTemplate uses `LinkedHashMap` type).! + + */ + + boolean hasType = false; boolean hasPrimitiveOrString = false; for (TemplateDescriptor beforeTemplate : beforeTemplates) { From 1c9d98ec3059430decd9efbb67a56e5953ac14d5 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 13 Dec 2024 11:31:48 +0100 Subject: [PATCH 13/35] Start new implementation, introduce PreCondition objects --- .../java/template/processor/PreCondition.java | 58 +++++++++ .../processor/RefasterTemplateProcessor.java | 123 ++---------------- 2 files changed, 70 insertions(+), 111 deletions(-) create mode 100644 src/main/java/org/openrewrite/java/template/processor/PreCondition.java diff --git a/src/main/java/org/openrewrite/java/template/processor/PreCondition.java b/src/main/java/org/openrewrite/java/template/processor/PreCondition.java new file mode 100644 index 00000000..695ea23e --- /dev/null +++ b/src/main/java/org/openrewrite/java/template/processor/PreCondition.java @@ -0,0 +1,58 @@ +package org.openrewrite.java.template.processor; + +import lombok.RequiredArgsConstructor; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Set; + +import static java.util.stream.Collectors.toSet; + +@RequiredArgsConstructor +public class PreCondition { + + @RequiredArgsConstructor + public static class Rule extends PreCondition { + final String rule; + + @Override + public String toString() { + return rule; + } + } + + @RequiredArgsConstructor + public static class Or extends PreCondition { + final Set rules; + final int indent; + + @Override + public String toString() { + return joinPreconditions(rules, "or", indent); + } + } + + @RequiredArgsConstructor + public static class And extends PreCondition { + final Set rules; + final int indent; + + @Override + public String toString() { + return joinPreconditions(rules, "and", indent); + } + } + + private static String joinPreconditions(Collection rules, String op, int indent) { + if (rules.isEmpty()) { + return ""; + } else if (rules.size() == 1) { + return rules.iterator().next().toString(); + } + char[] indentChars = new char[indent]; + Arrays.fill(indentChars, ' '); + String indentStr = new String(indentChars); + Set preconditions = rules.stream().map(Object::toString).collect(toSet()); + return "Preconditions." + op + "(\n" + indentStr + String.join(",\n" + indentStr, preconditions) + "\n" + indentStr.substring(0, indent - 4) + ')'; + } +} 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 cfdb54a3..d2355867 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -228,7 +228,7 @@ public void visitClassDef(JCTree.JCClassDecl classDecl) { String javaVisitor = newAbstractRefasterJavaVisitor(beforeTemplates, after, descriptor); - String preconditions = generatePreconditions(descriptor.beforeTemplates, 16); + PreCondition preconditions = generatePreconditions(descriptor.beforeTemplates, 16); if (preconditions == null) { recipe.append(String.format(" return %s;\n", javaVisitor)); } else { @@ -589,18 +589,17 @@ private Set getImportsAsStrings(Map> imp } /* Generate the minimal precondition that would allow to match each before template individually. */ - @SuppressWarnings("SameParameterValue") - private @Nullable String generatePreconditions(List beforeTemplates, int indent) { - Map> preconditions = new LinkedHashMap<>(); + private @Nullable PreCondition generatePreconditions(List beforeTemplates, int indent) { + Map> preconditions = new LinkedHashMap<>(); for (TemplateDescriptor beforeTemplate : beforeTemplates) { int arity = beforeTemplate.getArity(); for (int i = 0; i < arity; i++) { - Set usesVisitors = new LinkedHashSet<>(); + Set usesVisitors = new LinkedHashSet<>(); for (Symbol.ClassSymbol usedType : beforeTemplate.usedTypes(i)) { String name = usedType.getQualifiedName().toString().replace('$', '.'); if (!name.startsWith("java.lang.") && !name.startsWith("com.google.errorprone.refaster.")) { - usesVisitors.add("new UsesType<>(\"" + name + "\", true)"); + usesVisitors.add(new PreCondition.Rule("new UsesType<>(\"" + name + "\", true)")); } } for (Symbol.MethodSymbol method : beforeTemplate.usedMethods(i)) { @@ -609,120 +608,22 @@ private Set getImportsAsStrings(Map> imp } String methodName = method.name.toString(); methodName = methodName.equals("") ? "" : methodName; - usesVisitors.add("new UsesMethod<>(\"" + method.owner.getQualifiedName().toString() + ' ' + methodName + "(..)\", true)"); + usesVisitors.add(new PreCondition.Rule("new UsesMethod<>(\"" + method.owner.getQualifiedName().toString() + ' ' + methodName + "(..)\", true)")); } preconditions.put(beforeTemplate.method.name.toString() + (arity == 1 ? "" : "$" + i), usesVisitors); } } - Collection> values = - shouldPrune(beforeTemplates) ? prune(preconditions) : preconditions.values(); - - if (values.size() == 1) { - return joinPreconditions(values.iterator().next(), "and", indent + 4); - } else if (values.size() > 1) { - Set common = new LinkedHashSet<>(); - for (String dep : values.iterator().next()) { - if (values.stream().allMatch(v -> v.contains(dep))) { - common.add(dep); - } - } - common.forEach(dep -> values.forEach(v -> v.remove(dep))); - values.removeIf(Collection::isEmpty); - - if (common.isEmpty()) { - return joinPreconditions(values.stream().map(v -> joinPreconditions(v, "and", indent + 4)).collect(toList()), "or", indent + 4); - } else { - if (!preconditions.isEmpty()) { - String uniqueConditions = joinPreconditions(values.stream().map(v -> joinPreconditions(v, "and", indent + 12)).collect(toList()), "or", indent + 8); - common.add(uniqueConditions); - } - return joinPreconditions(common, "and", indent + 4); - } - } - return null; - } - - /** - * If there are multiple @BeforeTemplates, it means that one of the beforeTemplates actually corresponds to a piece of code. - * If one @BeforeTemplate uses a type argument and another @BeforeTemplate has at least some primitives or strings as arguments, - * then the latter template will not be executed because it is inadvertently disabled by the type precondition of the former. - * So in that case prune the preconditions, because we don't want to use preconditions to which not all @BeforeTemplates apply. - */ - private boolean shouldPrune(List beforeTemplates) { - if (beforeTemplates.size() < 2) { - return false; - } - - /* - If the types of the body (beforeTemplate.usedTypes) are not all different, like: - ```java - @BeforeTemplate - String string(String value) { - return Convert.quote(value); - } - - @BeforeTemplate - String _int(int value) { - return Convert.quote(String.valueOf(value)); - } - ``` - then a prune should be applied as well (both use `Convert` type, but `String` type is only used in second @BeforeTemplate). - - - So - ```java - @BeforeTemplate - Map hashMap(int size) { - return new HashMap(size); - } - - @BeforeTemplate - Map linkedHashMap(int size) { - return new LinkedHashMap(size); - } - ```java - should not be pruned (first @BeforeTemplate uses HashMap type, second @BeforeTemplate uses `LinkedHashMap` type).! - - */ - - - boolean hasType = false; - boolean hasPrimitiveOrString = false; - for (TemplateDescriptor beforeTemplate : beforeTemplates) { - for (JCTree.JCVariableDecl vd : beforeTemplate.method.params) { - boolean check = vd.sym.type.isPrimitive() || vd.sym.type.toString().equals("java.lang.String"); - hasType |= !check; - hasPrimitiveOrString |= check; - if (hasType && hasPrimitiveOrString) { - return true; - } - } - } - return false; - } - - private Collection> prune(Map> preconditions) { - return singleton(preconditions.values().stream() - .flatMap(Set::stream) - .collect(groupingBy(identity(), counting())) - .entrySet().stream() - .filter(it -> it.getValue() == preconditions.size()) - .map(Map.Entry::getKey) - .collect(toSet())); - } - - private String joinPreconditions(Collection preconditions, String op, int indent) { if (preconditions.isEmpty()) { return null; - } else if (preconditions.size() == 1) { - return preconditions.iterator().next(); } - char[] indentChars = new char[indent]; - Arrays.fill(indentChars, ' '); - String indentStr = new String(indentChars); - return "Preconditions." + op + "(\n" + indentStr + String.join(",\n" + indentStr, preconditions) + "\n" + indentStr.substring(0, indent - 4) + ')'; + + return new PreCondition.Or( + preconditions.values().stream() + .map(it -> new PreCondition.And(it, indent + 4)) + .collect(toSet()) + , indent + 4); } }.scan(cu); } From 3d9f1a7cc1f364e9c5c7e375279bce896ca608be Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 13 Dec 2024 12:36:07 +0100 Subject: [PATCH 14/35] Start with PreConditionTest --- .../java/template/processor/PreCondition.java | 86 +++++++++++++++++-- .../template/processor/PreConditionTest.java | 56 ++++++++++++ 2 files changed, 134 insertions(+), 8 deletions(-) create mode 100644 src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java diff --git a/src/main/java/org/openrewrite/java/template/processor/PreCondition.java b/src/main/java/org/openrewrite/java/template/processor/PreCondition.java index 695ea23e..dc1391b8 100644 --- a/src/main/java/org/openrewrite/java/template/processor/PreCondition.java +++ b/src/main/java/org/openrewrite/java/template/processor/PreCondition.java @@ -1,6 +1,9 @@ package org.openrewrite.java.template.processor; +import lombok.EqualsAndHashCode; import lombok.RequiredArgsConstructor; +import lombok.Value; +import sun.reflect.generics.reflectiveObjects.NotImplementedException; import java.util.Arrays; import java.util.Collection; @@ -9,11 +12,31 @@ import static java.util.stream.Collectors.toSet; @RequiredArgsConstructor -public class PreCondition { +public abstract class PreCondition { + abstract boolean fitsInto(PreCondition p); + + public PreCondition prune() { + return this; + } + + @Value + @EqualsAndHashCode(callSuper = false) @RequiredArgsConstructor public static class Rule extends PreCondition { - final String rule; + String rule; + + @Override + boolean fitsInto(PreCondition p) { + if (p instanceof Rule) { + return ((Rule) p).rule.equals(rule); + } else if (p instanceof Or) { + return ((Or) p).preConditions.stream().anyMatch(r -> r.fitsInto(this)); + } else if (p instanceof And) { + return ((And) p).preConditions.stream().anyMatch(r -> r.fitsInto(this)); + } + return false; + } @Override public String toString() { @@ -21,25 +44,72 @@ public String toString() { } } + @Value + @EqualsAndHashCode(callSuper = false) @RequiredArgsConstructor public static class Or extends PreCondition { - final Set rules; - final int indent; + Set preConditions; + int indent; + + @Override + boolean fitsInto(PreCondition p) { + throw new NotImplementedException(); + } + + @Override + public PreCondition prune() { + outer: for (PreCondition p : preConditions) { + int matches = 0; + for (PreCondition p2 : preConditions) { + if (p == p2) { + matches++; + } else if (p.fitsInto(p2)) { + matches++; + if (matches == preConditions.size()) { + return p; + } + } else { + break outer; + } + } + } + + return this; + } @Override public String toString() { - return joinPreconditions(rules, "or", indent); + return joinPreconditions(preConditions, "or", indent); } } + @Value + @EqualsAndHashCode(callSuper = false) @RequiredArgsConstructor public static class And extends PreCondition { - final Set rules; - final int indent; + Set preConditions; + int indent; + + @Override + boolean fitsInto(PreCondition p) { + if (p instanceof Rule) { + return preConditions.contains(p); + } else if (p instanceof Or) { + throw new NotImplementedException(); + } else if (p instanceof And) { + if (preConditions.size() < ((And) p).preConditions.size()) { + return false; + } + return preConditions.stream().anyMatch(it -> it.fitsInto(p)); + } + + // TODO not implemented other case yet + return false; + } @Override public String toString() { - return joinPreconditions(rules, "and", indent); + return joinPreconditions(preConditions, "and", indent); } } diff --git a/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java b/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java new file mode 100644 index 00000000..ac5e0bab --- /dev/null +++ b/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java @@ -0,0 +1,56 @@ +package org.openrewrite.java.template.processor; + +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import static com.google.common.truth.Truth.assertThat; + +class PreConditionTest { + @Test + void sameRulesArePrunedAutomatically() { + Set result = setOf(new PreCondition.Rule("A"), new PreCondition.Rule("A")); + + assertThat(result).isEqualTo(setOf(new PreCondition.Rule("A"))); + } + + @Test + void sameRulesArePrunedAutomaticallyInAnOr() { + PreCondition x = new PreCondition.Or( + setOf(new PreCondition.Rule("A"), new PreCondition.Rule("A")), + 4 + ); + + assertThat(x).isEqualTo(new PreCondition.Or( + setOf(new PreCondition.Rule("A")), + 4 + )); + } + + @Test + void pruneOrWithAnds() { + PreCondition result = new PreCondition.Or( + setOf( + new PreCondition.And( + setOf(new PreCondition.Rule("A"), new PreCondition.Rule("B")), + 4 + ), + new PreCondition.And( + setOf(new PreCondition.Rule("A"), new PreCondition.Rule("B"), new PreCondition.Rule("C")), + 4 + ) + ), 4).prune(); + + assertThat(result).isEqualTo(new PreCondition.And( + setOf(new PreCondition.Rule("A"), new PreCondition.Rule("B")), + 4 + )); + } + + @SafeVarargs + private final Set setOf(T... rules) { + return new HashSet<>(Arrays.asList(rules)); + } +} From 8eefef341b7c8f63b126af8097c0971657840b24 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 13 Dec 2024 16:23:27 +0100 Subject: [PATCH 15/35] Start with PreConditionTest --- .../java/template/processor/PreCondition.java | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/openrewrite/java/template/processor/PreCondition.java b/src/main/java/org/openrewrite/java/template/processor/PreCondition.java index dc1391b8..90bc6429 100644 --- a/src/main/java/org/openrewrite/java/template/processor/PreCondition.java +++ b/src/main/java/org/openrewrite/java/template/processor/PreCondition.java @@ -30,12 +30,9 @@ public static class Rule extends PreCondition { boolean fitsInto(PreCondition p) { if (p instanceof Rule) { return ((Rule) p).rule.equals(rule); - } else if (p instanceof Or) { - return ((Or) p).preConditions.stream().anyMatch(r -> r.fitsInto(this)); - } else if (p instanceof And) { - return ((And) p).preConditions.stream().anyMatch(r -> r.fitsInto(this)); + } else { + return p.fitsInto(this); } - return false; } @Override @@ -58,18 +55,14 @@ boolean fitsInto(PreCondition p) { @Override public PreCondition prune() { - outer: for (PreCondition p : preConditions) { + for (PreCondition p : preConditions) { int matches = 0; for (PreCondition p2 : preConditions) { - if (p == p2) { - matches++; - } else if (p.fitsInto(p2)) { + if (p == p2 || p.fitsInto(p2)) { matches++; - if (matches == preConditions.size()) { - return p; - } - } else { - break outer; + } + if (matches == preConditions.size()) { + return p; } } } @@ -97,14 +90,12 @@ boolean fitsInto(PreCondition p) { } else if (p instanceof Or) { throw new NotImplementedException(); } else if (p instanceof And) { - if (preConditions.size() < ((And) p).preConditions.size()) { + if (preConditions.size() > ((And) p).preConditions.size()) { return false; } - return preConditions.stream().anyMatch(it -> it.fitsInto(p)); + return preConditions.stream().allMatch(it -> it.fitsInto(p)); } - - // TODO not implemented other case yet - return false; + throw new IllegalArgumentException("Type is not supported: " + p.getClass()); } @Override From 9266f9e57435b6c3fc6dc4fc9c1e1a7909890fb9 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 13 Dec 2024 16:47:44 +0100 Subject: [PATCH 16/35] improvement --- .../processor/RefasterTemplateProcessor.java | 7 +++++-- .../java/template/processor/PreConditionTest.java | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 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 d2355867..8b208a21 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -611,7 +611,9 @@ private Set getImportsAsStrings(Map> imp usesVisitors.add(new PreCondition.Rule("new UsesMethod<>(\"" + method.owner.getQualifiedName().toString() + ' ' + methodName + "(..)\", true)")); } - preconditions.put(beforeTemplate.method.name.toString() + (arity == 1 ? "" : "$" + i), usesVisitors); + if (!usesVisitors.isEmpty()) { + preconditions.put(beforeTemplate.method.name.toString() + (arity == 1 ? "" : "$" + i), usesVisitors); + } } } @@ -623,7 +625,8 @@ private Set getImportsAsStrings(Map> imp preconditions.values().stream() .map(it -> new PreCondition.And(it, indent + 4)) .collect(toSet()) - , indent + 4); + , indent + 4) + .prune(); } }.scan(cu); } diff --git a/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java b/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java index ac5e0bab..53b271cd 100644 --- a/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java +++ b/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java @@ -49,6 +49,20 @@ void pruneOrWithAnds() { )); } + @Test + void pruneOrWithAndAndRule() { + PreCondition result = new PreCondition.Or( + setOf( + new PreCondition.And( + setOf(new PreCondition.Rule("A"), new PreCondition.Rule("B")), + 4 + ), + new PreCondition.Rule("B") + ), 4).prune(); + + assertThat(result).isEqualTo(new PreCondition.Rule("B")); + } + @SafeVarargs private final Set setOf(T... rules) { return new HashSet<>(Arrays.asList(rules)); From 6eccf0cfb3b9aa20fbabb9be86b2611050ff0d53 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 08:48:58 +0100 Subject: [PATCH 17/35] toString improvement --- .../java/template/processor/PreCondition.java | 8 ++--- .../template/processor/PreConditionTest.java | 34 +++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/openrewrite/java/template/processor/PreCondition.java b/src/main/java/org/openrewrite/java/template/processor/PreCondition.java index 90bc6429..3b4f13ed 100644 --- a/src/main/java/org/openrewrite/java/template/processor/PreCondition.java +++ b/src/main/java/org/openrewrite/java/template/processor/PreCondition.java @@ -110,10 +110,10 @@ private static String joinPreconditions(Collection rules, String o } else if (rules.size() == 1) { return rules.iterator().next().toString(); } - char[] indentChars = new char[indent]; - Arrays.fill(indentChars, ' '); - String indentStr = new String(indentChars); + String whitespace = String.format("%" + indent + "s", " "); Set preconditions = rules.stream().map(Object::toString).collect(toSet()); - return "Preconditions." + op + "(\n" + indentStr + String.join(",\n" + indentStr, preconditions) + "\n" + indentStr.substring(0, indent - 4) + ')'; + return "Preconditions." + op + "(\n" + + whitespace + String.join(",\n" + whitespace, preconditions) + "\n" + + whitespace.substring(0, indent - 4) + ')'; } } diff --git a/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java b/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java index 53b271cd..47f90fc6 100644 --- a/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java +++ b/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java @@ -9,6 +9,40 @@ import static com.google.common.truth.Truth.assertThat; class PreConditionTest { + @Test + void toStringWithInden() { + String result = new PreCondition.Or( + setOf( + new PreCondition.And( + setOf( + new PreCondition.Rule("A"), + new PreCondition.Rule("B"), + new PreCondition.Rule("C")), + 4 + ), + new PreCondition.And( + setOf( + new PreCondition.Rule("X"), + new PreCondition.Rule("Y"), + new PreCondition.Rule("Z")), + 4 + ) + ), 4).toString(); + + assertThat(result).isEqualTo("Preconditions.or(\n" + + " Preconditions.and(\n" + + " X,\n" + + " Y,\n" + + " Z\n" + + "),\n" + + " Preconditions.and(\n" + + " A,\n" + + " B,\n" + + " C\n" + + ")\n" + + ")"); + } + @Test void sameRulesArePrunedAutomatically() { Set result = setOf(new PreCondition.Rule("A"), new PreCondition.Rule("A")); From b0a33a188153c445b34e7684372f57528a9e6287 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 09:14:56 +0100 Subject: [PATCH 18/35] toString improvement --- .../java/template/processor/PreCondition.java | 11 ++++---- .../template/processor/PreConditionTest.java | 10 ++++---- .../resources/refaster/EscapesRecipes.java | 4 +-- .../refaster/MethodThrowsRecipe.java | 2 +- .../refaster/NestedPreconditionsRecipe.java | 21 ++++++++-------- .../PreConditionsVerifierRecipes.java | 11 +++++--- .../refaster/RefasterAnyOfRecipes.java | 25 +++++++++---------- .../refaster/TwoVisitMethodsRecipe.java | 4 +-- 8 files changed, 45 insertions(+), 43 deletions(-) diff --git a/src/main/java/org/openrewrite/java/template/processor/PreCondition.java b/src/main/java/org/openrewrite/java/template/processor/PreCondition.java index 3b4f13ed..c127a4fb 100644 --- a/src/main/java/org/openrewrite/java/template/processor/PreCondition.java +++ b/src/main/java/org/openrewrite/java/template/processor/PreCondition.java @@ -5,14 +5,15 @@ import lombok.Value; import sun.reflect.generics.reflectiveObjects.NotImplementedException; -import java.util.Arrays; -import java.util.Collection; -import java.util.Set; +import java.util.*; -import static java.util.stream.Collectors.toSet; +import static java.util.stream.Collectors.toCollection; @RequiredArgsConstructor public abstract class PreCondition { + private static final Comparator BY_USES_TYPE_FIRST = Comparator + .comparing((String s) -> !s.startsWith("new UsesType")) + .thenComparing(Comparator.naturalOrder()); abstract boolean fitsInto(PreCondition p); @@ -111,7 +112,7 @@ private static String joinPreconditions(Collection rules, String o return rules.iterator().next().toString(); } String whitespace = String.format("%" + indent + "s", " "); - Set preconditions = rules.stream().map(Object::toString).collect(toSet()); + Set preconditions = rules.stream().map(Object::toString).sorted(BY_USES_TYPE_FIRST).collect(toCollection(LinkedHashSet::new)); return "Preconditions." + op + "(\n" + whitespace + String.join(",\n" + whitespace, preconditions) + "\n" + whitespace.substring(0, indent - 4) + ')'; diff --git a/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java b/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java index 47f90fc6..8801cc6b 100644 --- a/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java +++ b/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java @@ -30,15 +30,15 @@ void toStringWithInden() { ), 4).toString(); assertThat(result).isEqualTo("Preconditions.or(\n" + - " Preconditions.and(\n" + - " X,\n" + - " Y,\n" + - " Z\n" + - "),\n" + " Preconditions.and(\n" + " A,\n" + " B,\n" + " C\n" + + "),\n" + + " Preconditions.and(\n" + + " X,\n" + + " Y,\n" + + " Z\n" + ")\n" + ")"); } diff --git a/src/test/resources/refaster/EscapesRecipes.java b/src/test/resources/refaster/EscapesRecipes.java index 3d56a747..f8d7e3e2 100644 --- a/src/test/resources/refaster/EscapesRecipes.java +++ b/src/test/resources/refaster/EscapesRecipes.java @@ -117,8 +117,8 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { return Preconditions.check( Preconditions.and( new UsesType<>("com.sun.tools.javac.util.Convert", true), - new UsesMethod<>("java.lang.String format(..)", true), - new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true) + new UsesMethod<>("com.sun.tools.javac.util.Convert quote(..)", true), + new UsesMethod<>("java.lang.String format(..)", true) ), javaVisitor ); diff --git a/src/test/resources/refaster/MethodThrowsRecipe.java b/src/test/resources/refaster/MethodThrowsRecipe.java index 4820dee8..34529869 100644 --- a/src/test/resources/refaster/MethodThrowsRecipe.java +++ b/src/test/resources/refaster/MethodThrowsRecipe.java @@ -85,8 +85,8 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { }; return Preconditions.check( Preconditions.and( - new UsesType<>("java.nio.file.Files", true), new UsesType<>("java.nio.charset.StandardCharsets", true), + new UsesType<>("java.nio.file.Files", true), new UsesType<>("java.nio.file.Path", true), new UsesMethod<>("java.nio.file.Files readAllLines(..)", true) ), diff --git a/src/test/resources/refaster/NestedPreconditionsRecipe.java b/src/test/resources/refaster/NestedPreconditionsRecipe.java index 40f46988..d0aa6638 100644 --- a/src/test/resources/refaster/NestedPreconditionsRecipe.java +++ b/src/test/resources/refaster/NestedPreconditionsRecipe.java @@ -96,17 +96,16 @@ public J visitNewClass(J.NewClass elem, ExecutionContext ctx) { }; return Preconditions.check( - Preconditions.and( - new UsesType<>("java.util.Map", true), - Preconditions.or( - Preconditions.and( - new UsesType<>("java.util.HashMap", true), - new UsesMethod<>("java.util.HashMap (..)", true) - ), - Preconditions.and( - new UsesType<>("java.util.LinkedHashMap", true), - new UsesMethod<>("java.util.LinkedHashMap (..)", true) - ) + Preconditions.or( + Preconditions.and( + new UsesType<>("java.util.HashMap", true), + new UsesType<>("java.util.Map", true), + new UsesMethod<>("java.util.HashMap (..)", true) + ), + Preconditions.and( + new UsesType<>("java.util.LinkedHashMap", true), + new UsesType<>("java.util.Map", true), + new UsesMethod<>("java.util.LinkedHashMap (..)", true) ) ), javaVisitor diff --git a/src/test/resources/refaster/PreConditionsVerifierRecipes.java b/src/test/resources/refaster/PreConditionsVerifierRecipes.java index e9159e04..859c64fd 100644 --- a/src/test/resources/refaster/PreConditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreConditionsVerifierRecipes.java @@ -545,11 +545,14 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { }; return Preconditions.check( - Preconditions.and( - new UsesMethod<>("java.io.PrintStream println(..)", true), - Preconditions.or( + Preconditions.or( + Preconditions.and( new UsesType<>("java.util.List", true), - new UsesType<>("java.util.Map", true) + new UsesMethod<>("java.io.PrintStream println(..)", true) + ), + Preconditions.and( + new UsesType<>("java.util.Map", true), + new UsesMethod<>("java.io.PrintStream println(..)", true) ) ), javaVisitor diff --git a/src/test/resources/refaster/RefasterAnyOfRecipes.java b/src/test/resources/refaster/RefasterAnyOfRecipes.java index 9d32c614..a531afde 100644 --- a/src/test/resources/refaster/RefasterAnyOfRecipes.java +++ b/src/test/resources/refaster/RefasterAnyOfRecipes.java @@ -192,17 +192,16 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { }; return Preconditions.check( - Preconditions.and( - new UsesType<>("java.util.List", true), - Preconditions.or( - Preconditions.and( - new UsesType<>("java.util.LinkedList", true), - new UsesMethod<>("java.util.LinkedList (..)", true) - ), - Preconditions.and( - new UsesType<>("java.util.Collections", true), - new UsesMethod<>("java.util.Collections emptyList(..)", true) - ) + Preconditions.or( + Preconditions.and( + new UsesType<>("java.util.Collections", true), + new UsesType<>("java.util.List", true), + new UsesMethod<>("java.util.Collections emptyList(..)", true) + ), + Preconditions.and( + new UsesType<>("java.util.LinkedList", true), + new UsesType<>("java.util.List", true), + new UsesMethod<>("java.util.LinkedList (..)", true) ) ), javaVisitor @@ -271,8 +270,8 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { }; return Preconditions.check( Preconditions.or( - new UsesMethod<>("java.lang.String valueOf(..)", true), - new UsesMethod<>("java.lang.String copyValueOf(..)", true) + new UsesMethod<>("java.lang.String copyValueOf(..)", true), + new UsesMethod<>("java.lang.String valueOf(..)", true) ), javaVisitor ); diff --git a/src/test/resources/refaster/TwoVisitMethodsRecipe.java b/src/test/resources/refaster/TwoVisitMethodsRecipe.java index e5032f9d..d578ab3e 100644 --- a/src/test/resources/refaster/TwoVisitMethodsRecipe.java +++ b/src/test/resources/refaster/TwoVisitMethodsRecipe.java @@ -101,8 +101,8 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { }; return Preconditions.check( Preconditions.or( - new UsesMethod<>("java.lang.String length(..)", true), - new UsesMethod<>("java.lang.String equals(..)", true) + new UsesMethod<>("java.lang.String equals(..)", true), + new UsesMethod<>("java.lang.String length(..)", true) ), javaVisitor ); From 356e03d1ccbb2bec60029a3a759c60f544b499e7 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 10:30:39 +0100 Subject: [PATCH 19/35] Rename `PreCondition` to `Precondition` + add copyright header --- .../{PreCondition.java => Precondition.java} | 45 ++++--- .../processor/RefasterTemplateProcessor.java | 18 ++- .../template/processor/PreConditionTest.java | 104 --------------- .../template/processor/PreconditionTest.java | 119 ++++++++++++++++++ 4 files changed, 157 insertions(+), 129 deletions(-) rename src/main/java/org/openrewrite/java/template/processor/{PreCondition.java => Precondition.java} (69%) delete mode 100644 src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java create mode 100644 src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java diff --git a/src/main/java/org/openrewrite/java/template/processor/PreCondition.java b/src/main/java/org/openrewrite/java/template/processor/Precondition.java similarity index 69% rename from src/main/java/org/openrewrite/java/template/processor/PreCondition.java rename to src/main/java/org/openrewrite/java/template/processor/Precondition.java index c127a4fb..4661f636 100644 --- a/src/main/java/org/openrewrite/java/template/processor/PreCondition.java +++ b/src/main/java/org/openrewrite/java/template/processor/Precondition.java @@ -1,3 +1,18 @@ +/* + * 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 org.openrewrite.java.template.processor; import lombok.EqualsAndHashCode; @@ -10,25 +25,25 @@ import static java.util.stream.Collectors.toCollection; @RequiredArgsConstructor -public abstract class PreCondition { +public abstract class Precondition { private static final Comparator BY_USES_TYPE_FIRST = Comparator .comparing((String s) -> !s.startsWith("new UsesType")) .thenComparing(Comparator.naturalOrder()); - abstract boolean fitsInto(PreCondition p); + abstract boolean fitsInto(Precondition p); - public PreCondition prune() { + public Precondition prune() { return this; } @Value @EqualsAndHashCode(callSuper = false) @RequiredArgsConstructor - public static class Rule extends PreCondition { + public static class Rule extends Precondition { String rule; @Override - boolean fitsInto(PreCondition p) { + boolean fitsInto(Precondition p) { if (p instanceof Rule) { return ((Rule) p).rule.equals(rule); } else { @@ -45,20 +60,20 @@ public String toString() { @Value @EqualsAndHashCode(callSuper = false) @RequiredArgsConstructor - public static class Or extends PreCondition { - Set preConditions; + public static class Or extends Precondition { + Set preConditions; int indent; @Override - boolean fitsInto(PreCondition p) { + boolean fitsInto(Precondition p) { throw new NotImplementedException(); } @Override - public PreCondition prune() { - for (PreCondition p : preConditions) { + public Precondition prune() { + for (Precondition p : preConditions) { int matches = 0; - for (PreCondition p2 : preConditions) { + for (Precondition p2 : preConditions) { if (p == p2 || p.fitsInto(p2)) { matches++; } @@ -80,12 +95,12 @@ public String toString() { @Value @EqualsAndHashCode(callSuper = false) @RequiredArgsConstructor - public static class And extends PreCondition { - Set preConditions; + public static class And extends Precondition { + Set preConditions; int indent; @Override - boolean fitsInto(PreCondition p) { + boolean fitsInto(Precondition p) { if (p instanceof Rule) { return preConditions.contains(p); } else if (p instanceof Or) { @@ -105,7 +120,7 @@ public String toString() { } } - private static String joinPreconditions(Collection rules, String op, int indent) { + private static String joinPreconditions(Collection rules, String op, int indent) { if (rules.isEmpty()) { return ""; } else if (rules.size() == 1) { 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 8b208a21..7d05ddef 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -48,10 +48,8 @@ import java.util.function.Predicate; import java.util.stream.Stream; -import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static java.util.Objects.requireNonNull; -import static java.util.function.Function.identity; import static java.util.stream.Collectors.*; import static org.openrewrite.java.template.processor.RefasterTemplateProcessor.AFTER_TEMPLATE; import static org.openrewrite.java.template.processor.RefasterTemplateProcessor.BEFORE_TEMPLATE; @@ -228,7 +226,7 @@ public void visitClassDef(JCTree.JCClassDecl classDecl) { String javaVisitor = newAbstractRefasterJavaVisitor(beforeTemplates, after, descriptor); - PreCondition preconditions = generatePreconditions(descriptor.beforeTemplates, 16); + Precondition preconditions = generatePreconditions(descriptor.beforeTemplates, 16); if (preconditions == null) { recipe.append(String.format(" return %s;\n", javaVisitor)); } else { @@ -589,17 +587,17 @@ private Set getImportsAsStrings(Map> imp } /* Generate the minimal precondition that would allow to match each before template individually. */ - private @Nullable PreCondition generatePreconditions(List beforeTemplates, int indent) { - Map> preconditions = new LinkedHashMap<>(); + private @Nullable Precondition generatePreconditions(List beforeTemplates, int indent) { + Map> preconditions = new LinkedHashMap<>(); for (TemplateDescriptor beforeTemplate : beforeTemplates) { int arity = beforeTemplate.getArity(); for (int i = 0; i < arity; i++) { - Set usesVisitors = new LinkedHashSet<>(); + Set usesVisitors = new LinkedHashSet<>(); for (Symbol.ClassSymbol usedType : beforeTemplate.usedTypes(i)) { String name = usedType.getQualifiedName().toString().replace('$', '.'); if (!name.startsWith("java.lang.") && !name.startsWith("com.google.errorprone.refaster.")) { - usesVisitors.add(new PreCondition.Rule("new UsesType<>(\"" + name + "\", true)")); + usesVisitors.add(new Precondition.Rule("new UsesType<>(\"" + name + "\", true)")); } } for (Symbol.MethodSymbol method : beforeTemplate.usedMethods(i)) { @@ -608,7 +606,7 @@ private Set getImportsAsStrings(Map> imp } String methodName = method.name.toString(); methodName = methodName.equals("") ? "" : methodName; - usesVisitors.add(new PreCondition.Rule("new UsesMethod<>(\"" + method.owner.getQualifiedName().toString() + ' ' + methodName + "(..)\", true)")); + usesVisitors.add(new Precondition.Rule("new UsesMethod<>(\"" + method.owner.getQualifiedName().toString() + ' ' + methodName + "(..)\", true)")); } if (!usesVisitors.isEmpty()) { @@ -621,9 +619,9 @@ private Set getImportsAsStrings(Map> imp return null; } - return new PreCondition.Or( + return new Precondition.Or( preconditions.values().stream() - .map(it -> new PreCondition.And(it, indent + 4)) + .map(it -> new Precondition.And(it, indent + 4)) .collect(toSet()) , indent + 4) .prune(); diff --git a/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java b/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java deleted file mode 100644 index 8801cc6b..00000000 --- a/src/test/java/org/openrewrite/java/template/processor/PreConditionTest.java +++ /dev/null @@ -1,104 +0,0 @@ -package org.openrewrite.java.template.processor; - -import org.junit.jupiter.api.Test; - -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; - -import static com.google.common.truth.Truth.assertThat; - -class PreConditionTest { - @Test - void toStringWithInden() { - String result = new PreCondition.Or( - setOf( - new PreCondition.And( - setOf( - new PreCondition.Rule("A"), - new PreCondition.Rule("B"), - new PreCondition.Rule("C")), - 4 - ), - new PreCondition.And( - setOf( - new PreCondition.Rule("X"), - new PreCondition.Rule("Y"), - new PreCondition.Rule("Z")), - 4 - ) - ), 4).toString(); - - assertThat(result).isEqualTo("Preconditions.or(\n" + - " Preconditions.and(\n" + - " A,\n" + - " B,\n" + - " C\n" + - "),\n" + - " Preconditions.and(\n" + - " X,\n" + - " Y,\n" + - " Z\n" + - ")\n" + - ")"); - } - - @Test - void sameRulesArePrunedAutomatically() { - Set result = setOf(new PreCondition.Rule("A"), new PreCondition.Rule("A")); - - assertThat(result).isEqualTo(setOf(new PreCondition.Rule("A"))); - } - - @Test - void sameRulesArePrunedAutomaticallyInAnOr() { - PreCondition x = new PreCondition.Or( - setOf(new PreCondition.Rule("A"), new PreCondition.Rule("A")), - 4 - ); - - assertThat(x).isEqualTo(new PreCondition.Or( - setOf(new PreCondition.Rule("A")), - 4 - )); - } - - @Test - void pruneOrWithAnds() { - PreCondition result = new PreCondition.Or( - setOf( - new PreCondition.And( - setOf(new PreCondition.Rule("A"), new PreCondition.Rule("B")), - 4 - ), - new PreCondition.And( - setOf(new PreCondition.Rule("A"), new PreCondition.Rule("B"), new PreCondition.Rule("C")), - 4 - ) - ), 4).prune(); - - assertThat(result).isEqualTo(new PreCondition.And( - setOf(new PreCondition.Rule("A"), new PreCondition.Rule("B")), - 4 - )); - } - - @Test - void pruneOrWithAndAndRule() { - PreCondition result = new PreCondition.Or( - setOf( - new PreCondition.And( - setOf(new PreCondition.Rule("A"), new PreCondition.Rule("B")), - 4 - ), - new PreCondition.Rule("B") - ), 4).prune(); - - assertThat(result).isEqualTo(new PreCondition.Rule("B")); - } - - @SafeVarargs - private final Set setOf(T... rules) { - return new HashSet<>(Arrays.asList(rules)); - } -} diff --git a/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java b/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java new file mode 100644 index 00000000..46cc0472 --- /dev/null +++ b/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java @@ -0,0 +1,119 @@ +/* + * 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 org.openrewrite.java.template.processor; + +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import static com.google.common.truth.Truth.assertThat; + +class PreconditionTest { + @Test + void toStringWithInden() { + String result = new Precondition.Or( + setOf( + new Precondition.And( + setOf( + new Precondition.Rule("A"), + new Precondition.Rule("B"), + new Precondition.Rule("C")), + 4 + ), + new Precondition.And( + setOf( + new Precondition.Rule("X"), + new Precondition.Rule("Y"), + new Precondition.Rule("Z")), + 4 + ) + ), 4).toString(); + + assertThat(result).isEqualTo("Preconditions.or(\n" + + " Preconditions.and(\n" + + " A,\n" + + " B,\n" + + " C\n" + + "),\n" + + " Preconditions.and(\n" + + " X,\n" + + " Y,\n" + + " Z\n" + + ")\n" + + ")"); + } + + @Test + void sameRulesArePrunedAutomatically() { + Set result = setOf(new Precondition.Rule("A"), new Precondition.Rule("A")); + + assertThat(result).isEqualTo(setOf(new Precondition.Rule("A"))); + } + + @Test + void sameRulesArePrunedAutomaticallyInAnOr() { + Precondition x = new Precondition.Or( + setOf(new Precondition.Rule("A"), new Precondition.Rule("A")), + 4 + ); + + assertThat(x).isEqualTo(new Precondition.Or( + setOf(new Precondition.Rule("A")), + 4 + )); + } + + @Test + void pruneOrWithAnds() { + Precondition result = new Precondition.Or( + setOf( + new Precondition.And( + setOf(new Precondition.Rule("A"), new Precondition.Rule("B")), + 4 + ), + new Precondition.And( + setOf(new Precondition.Rule("A"), new Precondition.Rule("B"), new Precondition.Rule("C")), + 4 + ) + ), 4).prune(); + + assertThat(result).isEqualTo(new Precondition.And( + setOf(new Precondition.Rule("A"), new Precondition.Rule("B")), + 4 + )); + } + + @Test + void pruneOrWithAndAndRule() { + Precondition result = new Precondition.Or( + setOf( + new Precondition.And( + setOf(new Precondition.Rule("A"), new Precondition.Rule("B")), + 4 + ), + new Precondition.Rule("B") + ), 4).prune(); + + assertThat(result).isEqualTo(new Precondition.Rule("B")); + } + + @SafeVarargs + private final Set setOf(T... rules) { + return new HashSet<>(Arrays.asList(rules)); + } +} From 8a392fb9b4952ecc0acc64f537c69b3ccbd4bbab Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 11:56:49 +0100 Subject: [PATCH 20/35] Improve `fitsInto` --- .../java/template/processor/Precondition.java | 41 +++++++------ .../template/processor/PreconditionTest.java | 61 ++++++++++++++++++- 2 files changed, 80 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/openrewrite/java/template/processor/Precondition.java b/src/main/java/org/openrewrite/java/template/processor/Precondition.java index 4661f636..3f7f02c7 100644 --- a/src/main/java/org/openrewrite/java/template/processor/Precondition.java +++ b/src/main/java/org/openrewrite/java/template/processor/Precondition.java @@ -18,7 +18,6 @@ import lombok.EqualsAndHashCode; import lombok.RequiredArgsConstructor; import lombok.Value; -import sun.reflect.generics.reflectiveObjects.NotImplementedException; import java.util.*; @@ -45,10 +44,13 @@ public static class Rule extends Precondition { @Override boolean fitsInto(Precondition p) { if (p instanceof Rule) { - return ((Rule) p).rule.equals(rule); - } else { - return p.fitsInto(this); + return this.equals(p); + } else if (p instanceof Or) { + return ((Or) p).preconditions.stream().anyMatch(this::fitsInto); + } else if (p instanceof And) { + return ((And) p).preconditions.stream().anyMatch(this::fitsInto); } + return false; // unreachable code } @Override @@ -61,23 +63,26 @@ public String toString() { @EqualsAndHashCode(callSuper = false) @RequiredArgsConstructor public static class Or extends Precondition { - Set preConditions; + Set preconditions; int indent; @Override boolean fitsInto(Precondition p) { - throw new NotImplementedException(); + if (p instanceof Or) { + return this.equals(p); + } + return false; } @Override public Precondition prune() { - for (Precondition p : preConditions) { + for (Precondition p : preconditions) { int matches = 0; - for (Precondition p2 : preConditions) { + for (Precondition p2 : preconditions) { if (p == p2 || p.fitsInto(p2)) { matches++; } - if (matches == preConditions.size()) { + if (matches == preconditions.size()) { return p; } } @@ -88,7 +93,7 @@ public Precondition prune() { @Override public String toString() { - return joinPreconditions(preConditions, "or", indent); + return joinPreconditions(preconditions, "or", indent); } } @@ -96,27 +101,23 @@ public String toString() { @EqualsAndHashCode(callSuper = false) @RequiredArgsConstructor public static class And extends Precondition { - Set preConditions; + Set preconditions; int indent; @Override boolean fitsInto(Precondition p) { - if (p instanceof Rule) { - return preConditions.contains(p); - } else if (p instanceof Or) { - throw new NotImplementedException(); - } else if (p instanceof And) { - if (preConditions.size() > ((And) p).preConditions.size()) { + if (p instanceof And) { + if (preconditions.size() > ((And) p).preconditions.size()) { return false; } - return preConditions.stream().allMatch(it -> it.fitsInto(p)); + return preconditions.stream().allMatch(it -> it.fitsInto(p)); } - throw new IllegalArgumentException("Type is not supported: " + p.getClass()); + return false; } @Override public String toString() { - return joinPreconditions(preConditions, "and", indent); + return joinPreconditions(preconditions, "and", indent); } } diff --git a/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java b/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java index 46cc0472..bfcac061 100644 --- a/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java +++ b/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java @@ -58,6 +58,63 @@ void toStringWithInden() { ")"); } + @Test + void ruleFitsInRule() { + assertThat(new Precondition.Rule("A").fitsInto(new Precondition.Rule("A"))).isTrue(); + } + + @Test + void orFitsInOr() { + boolean result = new Precondition.Or( + setOf(new Precondition.Rule("A"), new Precondition.Rule("B")), + 4 + ).fitsInto(new Precondition.Or( + setOf(new Precondition.Rule("B"), new Precondition.Rule("A")), + 4 + )); + + assertThat(result).isTrue(); + } + + @Test + void ardFitsNotInAndWithDifferentRules() { + boolean result = new Precondition.Or( + setOf(new Precondition.Rule("A"), new Precondition.Rule("C")), + 4 + ).fitsInto(new Precondition.Or( + setOf(new Precondition.Rule("B"), new Precondition.Rule("A")), + 4 + )); + + assertThat(result).isFalse(); + } + + @Test + void andFitsInAnd() { + boolean result = new Precondition.And( + setOf(new Precondition.Rule("A")), + 4 + ).fitsInto(new Precondition.And( + setOf(new Precondition.Rule("B"), new Precondition.Rule("A")), + 4 + )); + + assertThat(result).isTrue(); + } + + @Test + void andFitsNotInAndWithDifferentRules() { + boolean result = new Precondition.And( + setOf(new Precondition.Rule("A"), new Precondition.Rule("C")), + 4 + ).fitsInto(new Precondition.And( + setOf(new Precondition.Rule("B"), new Precondition.Rule("A")), + 4 + )); + + assertThat(result).isFalse(); + } + @Test void sameRulesArePrunedAutomatically() { Set result = setOf(new Precondition.Rule("A"), new Precondition.Rule("A")); @@ -67,12 +124,12 @@ void sameRulesArePrunedAutomatically() { @Test void sameRulesArePrunedAutomaticallyInAnOr() { - Precondition x = new Precondition.Or( + Precondition result = new Precondition.Or( setOf(new Precondition.Rule("A"), new Precondition.Rule("A")), 4 ); - assertThat(x).isEqualTo(new Precondition.Or( + assertThat(result).isEqualTo(new Precondition.Or( setOf(new Precondition.Rule("A")), 4 )); From 680c7e41890d6749e52c56bd4430ff66fd3c3b7c Mon Sep 17 00:00:00 2001 From: Jacob van Lingen Date: Mon, 16 Dec 2024 11:59:54 +0100 Subject: [PATCH 21/35] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../openrewrite/java/template/processor/Precondition.java | 6 +++--- src/test/resources/refaster/PreConditionsVerifier.java | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/openrewrite/java/template/processor/Precondition.java b/src/main/java/org/openrewrite/java/template/processor/Precondition.java index 3f7f02c7..29742d4a 100644 --- a/src/main/java/org/openrewrite/java/template/processor/Precondition.java +++ b/src/main/java/org/openrewrite/java/template/processor/Precondition.java @@ -129,8 +129,8 @@ private static String joinPreconditions(Collection rules, String o } String whitespace = String.format("%" + indent + "s", " "); Set preconditions = rules.stream().map(Object::toString).sorted(BY_USES_TYPE_FIRST).collect(toCollection(LinkedHashSet::new)); - return "Preconditions." + op + "(\n" - + whitespace + String.join(",\n" + whitespace, preconditions) + "\n" - + whitespace.substring(0, indent - 4) + ')'; + return "Preconditions." + op + "(\n" + + whitespace + String.join(",\n" + whitespace, preconditions) + "\n" + + whitespace.substring(0, indent - 4) + ')'; } } diff --git a/src/test/resources/refaster/PreConditionsVerifier.java b/src/test/resources/refaster/PreConditionsVerifier.java index 61a7f832..ceb8bb30 100644 --- a/src/test/resources/refaster/PreConditionsVerifier.java +++ b/src/test/resources/refaster/PreConditionsVerifier.java @@ -18,7 +18,6 @@ import java.util.List; import java.util.Map; -import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate; import com.sun.tools.javac.util.Constants; import com.sun.tools.javac.util.Convert; From 87bd944f3c2add1ccb254beea0aa3c1c17dfc7f3 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 12:11:24 +0100 Subject: [PATCH 22/35] Revert formatting --- .../processor/RefasterTemplateProcessor.java | 88 +++++++++---------- 1 file changed, 44 insertions(+), 44 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 7d05ddef..f50482ea 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -308,11 +308,11 @@ private void writeRecipeClass(JCTree.JCClassDecl classDecl, boolean outerClassRe .collect(joining(",\n")); out.write( " @Override\n" + - " public List getRecipeList() {\n" + - " return Arrays.asList(\n" + - recipesAsList + '\n' + - " );\n" + - " }\n\n"); + " public List getRecipeList() {\n" + + " return Arrays.asList(\n" + + recipesAsList + '\n' + + " );\n" + + " }\n\n"); for (String r : recipes.values()) { out.write(r.replaceAll("(?m)^(.+)$", " $1")); @@ -414,7 +414,7 @@ private String generateVisitMethod(Map beforeTemplat List embedOptions = new ArrayList<>(); JCTree.JCExpression afterReturn = getReturnExpression(descriptor.afterTemplate.method); if (afterReturn instanceof JCTree.JCParens || - afterReturn instanceof JCTree.JCUnary && ((JCTree.JCUnary) afterReturn).getExpression() instanceof JCTree.JCParens) { + afterReturn instanceof JCTree.JCUnary && ((JCTree.JCUnary) afterReturn).getExpression() instanceof JCTree.JCParens) { embedOptions.add("REMOVE_PARENS"); } // TODO check if after template contains type or member references @@ -537,28 +537,28 @@ private String recipeDescriptor(JCTree.JCClassDecl classDecl, String defaultDisp } String recipeDescriptor = " @Override\n" + - " public String getDisplayName() {\n" + - " return \"" + displayName + "\";\n" + - " }\n" + - "\n" + - " @Override\n" + - " public String getDescription() {\n" + - " return \"" + description + "\";\n" + - " }\n" + - "\n"; + " public String getDisplayName() {\n" + + " return \"" + displayName + "\";\n" + + " }\n" + + "\n" + + " @Override\n" + + " public String getDescription() {\n" + + " return \"" + description + "\";\n" + + " }\n" + + "\n"; if (tags.size() == 1) { recipeDescriptor += " @Override\n" + - " public Set getTags() {\n" + - " return Collections.singleton(\"" + String.join("\", \"", tags) + "\");\n" + - " }\n" + - "\n"; + " public Set getTags() {\n" + + " return Collections.singleton(\"" + String.join("\", \"", tags) + "\");\n" + + " }\n" + + "\n"; } else if (tags.size() > 1) { recipeDescriptor += " @Override\n" + - " public Set getTags() {\n" + - " return new HashSet<>(Arrays.asList(\"" + String.join("\", \"", tags) + "\"));\n" + - " }\n" + - "\n"; + " public Set getTags() {\n" + + " return new HashSet<>(Arrays.asList(\"" + String.join("\", \"", tags) + "\"));\n" + + " }\n" + + "\n"; } return recipeDescriptor; @@ -644,9 +644,9 @@ public void scan(JCTree jcTree) { if (jcTree instanceof JCTree.JCIdent) { JCTree.JCIdent jcIdent = (JCTree.JCIdent) jcTree; if (jcIdent.sym instanceof Symbol.VarSymbol && - jcIdent.sym.owner instanceof Symbol.MethodSymbol && - ((Symbol.MethodSymbol) jcIdent.sym.owner).params.contains(jcIdent.sym) && - seenParams.add(jcIdent.sym)) { + jcIdent.sym.owner instanceof Symbol.MethodSymbol && + ((Symbol.MethodSymbol) jcIdent.sym.owner).params.contains(jcIdent.sym) && + seenParams.add(jcIdent.sym)) { beforeParamOrder.put(((Symbol.MethodSymbol) jcIdent.sym.owner).params.indexOf(jcIdent.sym), beforeParameterOccurrence.getAndIncrement()); } } @@ -661,9 +661,9 @@ public void scan(JCTree jcTree) { if (jcTree instanceof JCTree.JCIdent) { JCTree.JCIdent jcIdent = (JCTree.JCIdent) jcTree; if (jcIdent.sym instanceof Symbol.VarSymbol && - jcIdent.sym.owner instanceof Symbol.MethodSymbol && - ((Symbol.MethodSymbol) jcIdent.sym.owner).params.contains(jcIdent.sym) && - seenParams.add(jcIdent.sym)) { + jcIdent.sym.owner instanceof Symbol.MethodSymbol && + ((Symbol.MethodSymbol) jcIdent.sym.owner).params.contains(jcIdent.sym) && + seenParams.add(jcIdent.sym)) { afterParams.add(beforeParamOrder.get(((Symbol.MethodSymbol) jcIdent.sym.owner).params.indexOf(jcIdent.sym))); } } @@ -739,7 +739,7 @@ public RuleDescriptor(JCTree.JCClassDecl classDecl, JCCompilationUnit cu, Contex for (JCTree member : classDecl.getMembers()) { if (member instanceof JCTree.JCMethodDecl && beforeTemplates.stream().noneMatch(t -> t.method == member) && - (afterTemplate == null || member != afterTemplate.method)) { + (afterTemplate == null || member != afterTemplate.method)) { for (JCTree.JCAnnotation annotation : getTemplateAnnotations(((JCTree.JCMethodDecl) member), UNSUPPORTED_ANNOTATIONS::contains)) { printNoteOnce("@" + annotation.annotationType + " is currently not supported", classDecl.sym); return null; @@ -818,7 +818,7 @@ private boolean isAnyOfCall(JCTree.JCMethodInvocation call) { if (meth instanceof JCTree.JCFieldAccess) { JCTree.JCFieldAccess fieldAccess = (JCTree.JCFieldAccess) meth; return fieldAccess.name.toString().equals("anyOf") && - ((JCTree.JCIdent) fieldAccess.selected).name.toString().equals("Refaster"); + ((JCTree.JCIdent) fieldAccess.selected).name.toString().equals("Refaster"); } return false; } @@ -922,7 +922,7 @@ boolean validate(JCTree tree) { @Override public void visitSelect(JCTree.JCFieldAccess jcFieldAccess) { if (jcFieldAccess.selected.type.tsym.toString().equals("com.google.errorprone.refaster.Refaster") && - jcFieldAccess.name.toString().equals("anyOf")) { + jcFieldAccess.name.toString().equals("anyOf")) { // exception for `Refaster.anyOf()` if (++anyOfCount > 1) { printNoteOnce("Refaster.anyOf() can only be used once per template", classDecl.sym); @@ -936,8 +936,8 @@ public void visitSelect(JCTree.JCFieldAccess jcFieldAccess) { @Override public void visitIdent(JCTree.JCIdent jcIdent) { if (valid && - jcIdent.sym != null && - jcIdent.sym.packge().getQualifiedName().contentEquals("com.google.errorprone.refaster")) { + jcIdent.sym != null && + jcIdent.sym.packge().getQualifiedName().contentEquals("com.google.errorprone.refaster")) { printNoteOnce(jcIdent.type.tsym.getQualifiedName() + " is currently not supported", classDecl.sym); valid = false; } @@ -1056,16 +1056,16 @@ private static List getTemplateAnnotations(MethodTree metho for (AnnotationTree annotation : method.getModifiers().getAnnotations()) { Tree type = annotation.getAnnotationType(); if (type.getKind() == Tree.Kind.IDENTIFIER && ((JCTree.JCIdent) type).sym != null && - typePredicate.test(((JCTree.JCIdent) type).sym.getQualifiedName().toString())) { + typePredicate.test(((JCTree.JCIdent) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } else if (type.getKind() == Tree.Kind.IDENTIFIER && ((JCTree.JCAnnotation) annotation).attribute != null && - ((JCTree.JCAnnotation) annotation).attribute.type instanceof Type.ClassType && - ((JCTree.JCAnnotation) annotation).attribute.type.tsym != null && - typePredicate.test(((JCTree.JCAnnotation) annotation).attribute.type.tsym.getQualifiedName().toString())) { + ((JCTree.JCAnnotation) annotation).attribute.type instanceof Type.ClassType && + ((JCTree.JCAnnotation) annotation).attribute.type.tsym != null && + typePredicate.test(((JCTree.JCAnnotation) annotation).attribute.type.tsym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } else if (type.getKind() == Tree.Kind.MEMBER_SELECT && type instanceof JCTree.JCFieldAccess && - ((JCTree.JCFieldAccess) type).sym != null && - typePredicate.test(((JCTree.JCFieldAccess) type).sym.getQualifiedName().toString())) { + ((JCTree.JCFieldAccess) type).sym != null && + typePredicate.test(((JCTree.JCFieldAccess) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } } @@ -1077,12 +1077,12 @@ private static List getTemplateAnnotations(VariableTree par for (AnnotationTree annotation : parameter.getModifiers().getAnnotations()) { Tree type = annotation.getAnnotationType(); if (type.getKind() == Tree.Kind.IDENTIFIER && - ((JCTree.JCIdent) type).sym != null && - typePredicate.test(((JCTree.JCIdent) type).sym.getQualifiedName().toString())) { + ((JCTree.JCIdent) type).sym != null && + typePredicate.test(((JCTree.JCIdent) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } else if (type.getKind() == Tree.Kind.MEMBER_SELECT && type instanceof JCTree.JCFieldAccess && - ((JCTree.JCFieldAccess) type).sym != null && - typePredicate.test(((JCTree.JCFieldAccess) type).sym.getQualifiedName().toString())) { + ((JCTree.JCFieldAccess) type).sym != null && + typePredicate.test(((JCTree.JCFieldAccess) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } } From d7094c6e96e711cd3e4b1f9f3bf64d3265169945 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 12:12:23 +0100 Subject: [PATCH 23/35] Revert formatting --- .../processor/RefasterTemplateProcessor.java | 12 ++++++------ 1 file changed, 6 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 f50482ea..a5e3ae21 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -307,12 +307,12 @@ private void writeRecipeClass(JCTree.JCClassDecl classDecl, boolean outerClassRe .map(r -> " new " + r.substring(r.lastIndexOf('.') + 1) + "()") .collect(joining(",\n")); out.write( - " @Override\n" + - " public List getRecipeList() {\n" + - " return Arrays.asList(\n" + - recipesAsList + '\n' + - " );\n" + - " }\n\n"); + " @Override\n" + + " public List getRecipeList() {\n" + + " return Arrays.asList(\n" + + recipesAsList + '\n' + + " );\n" + + " }\n\n"); for (String r : recipes.values()) { out.write(r.replaceAll("(?m)^(.+)$", " $1")); From b65f7c0a0123b93056940ccc5939ea3ed1c8be69 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 12:13:13 +0100 Subject: [PATCH 24/35] Revert formatting --- .../processor/RefasterTemplateProcessor.java | 12 ++++++------ 1 file changed, 6 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 a5e3ae21..b9049c40 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -307,12 +307,12 @@ private void writeRecipeClass(JCTree.JCClassDecl classDecl, boolean outerClassRe .map(r -> " new " + r.substring(r.lastIndexOf('.') + 1) + "()") .collect(joining(",\n")); out.write( - " @Override\n" + - " public List getRecipeList() {\n" + - " return Arrays.asList(\n" + - recipesAsList + '\n' + - " );\n" + - " }\n\n"); + " @Override\n" + + " public List getRecipeList() {\n" + + " return Arrays.asList(\n" + + recipesAsList + '\n' + + " );\n" + + " }\n\n"); for (String r : recipes.values()) { out.write(r.replaceAll("(?m)^(.+)$", " $1")); From afe33c01bd733a4fbeb76a33b4a4899849334f9b Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 12:14:31 +0100 Subject: [PATCH 25/35] Revert formatting --- .../processor/RefasterTemplateProcessor.java | 12 ++++++------ 1 file changed, 6 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 b9049c40..ee5465c8 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -1059,12 +1059,12 @@ private static List getTemplateAnnotations(MethodTree metho typePredicate.test(((JCTree.JCIdent) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } else if (type.getKind() == Tree.Kind.IDENTIFIER && ((JCTree.JCAnnotation) annotation).attribute != null && - ((JCTree.JCAnnotation) annotation).attribute.type instanceof Type.ClassType && - ((JCTree.JCAnnotation) annotation).attribute.type.tsym != null && - typePredicate.test(((JCTree.JCAnnotation) annotation).attribute.type.tsym.getQualifiedName().toString())) { + ((JCTree.JCAnnotation) annotation).attribute.type instanceof Type.ClassType && + ((JCTree.JCAnnotation) annotation).attribute.type.tsym != null && + typePredicate.test(((JCTree.JCAnnotation) annotation).attribute.type.tsym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } else if (type.getKind() == Tree.Kind.MEMBER_SELECT && type instanceof JCTree.JCFieldAccess && - ((JCTree.JCFieldAccess) type).sym != null && + ((JCTree.JCFieldAccess) type).sym != null && typePredicate.test(((JCTree.JCFieldAccess) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } @@ -1081,8 +1081,8 @@ private static List getTemplateAnnotations(VariableTree par typePredicate.test(((JCTree.JCIdent) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } else if (type.getKind() == Tree.Kind.MEMBER_SELECT && type instanceof JCTree.JCFieldAccess && - ((JCTree.JCFieldAccess) type).sym != null && - typePredicate.test(((JCTree.JCFieldAccess) type).sym.getQualifiedName().toString())) { + ((JCTree.JCFieldAccess) type).sym != null && + typePredicate.test(((JCTree.JCFieldAccess) type).sym.getQualifiedName().toString())) { result.add((JCTree.JCAnnotation) annotation); } } From b51244368351fb23d53c4530276e2879b3a29cc9 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 16 Dec 2024 13:13:29 +0100 Subject: [PATCH 26/35] Reorder imports in PreConditionsVerifier.java --- src/test/resources/refaster/PreConditionsVerifier.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/resources/refaster/PreConditionsVerifier.java b/src/test/resources/refaster/PreConditionsVerifier.java index ceb8bb30..1f749ccf 100644 --- a/src/test/resources/refaster/PreConditionsVerifier.java +++ b/src/test/resources/refaster/PreConditionsVerifier.java @@ -15,13 +15,13 @@ */ package foo; -import java.util.List; -import java.util.Map; - import com.google.errorprone.refaster.annotation.BeforeTemplate; import com.sun.tools.javac.util.Constants; import com.sun.tools.javac.util.Convert; +import java.util.List; +import java.util.Map; + /** * A refaster template to test when a `UsesType`and Preconditions.or should or should not be applied to the Preconditions check. */ From 0257c95f8461e4963c46cb038372d8b50528c75e Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 13:27:24 +0100 Subject: [PATCH 27/35] Change name `PreConditionsVerifier` => `PreconditionsVerifier` --- .../RefasterTemplateProcessorTest.java | 2 +- ...rifier.java => PreconditionsVerifier.java} | 10 +++--- ...java => PreconditionsVerifierRecipes.java} | 36 +++++++++---------- 3 files changed, 24 insertions(+), 24 deletions(-) rename src/test/resources/refaster/{PreConditionsVerifier.java => PreconditionsVerifier.java} (97%) rename src/test/resources/refaster/{PreConditionsVerifierRecipes.java => PreconditionsVerifierRecipes.java} (96%) diff --git a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java index 3fe8f5b4..95f3a2c2 100644 --- a/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java +++ b/src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java @@ -96,7 +96,7 @@ void skipRecipeGeneration(String recipeName) { "SimplifyTernary", "RefasterAnyOf", "Parameters", - "PreConditionsVerifier", + "PreconditionsVerifier", }) void nestedRecipes(String recipeName) { Compilation compilation = compileResource("refaster/" + recipeName + ".java"); diff --git a/src/test/resources/refaster/PreConditionsVerifier.java b/src/test/resources/refaster/PreconditionsVerifier.java similarity index 97% rename from src/test/resources/refaster/PreConditionsVerifier.java rename to src/test/resources/refaster/PreconditionsVerifier.java index 1f749ccf..415fb600 100644 --- a/src/test/resources/refaster/PreConditionsVerifier.java +++ b/src/test/resources/refaster/PreconditionsVerifier.java @@ -15,17 +15,17 @@ */ package foo; -import com.google.errorprone.refaster.annotation.BeforeTemplate; -import com.sun.tools.javac.util.Constants; -import com.sun.tools.javac.util.Convert; - import java.util.List; import java.util.Map; +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; +import com.sun.tools.javac.util.Convert; + /** * A refaster template to test when a `UsesType`and Preconditions.or should or should not be applied to the Preconditions check. */ -public class PreConditionsVerifier { +public class PreconditionsVerifier { public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString { @BeforeTemplate void doubleAndInt(double actual, int ignore) { diff --git a/src/test/resources/refaster/PreConditionsVerifierRecipes.java b/src/test/resources/refaster/PreconditionsVerifierRecipes.java similarity index 96% rename from src/test/resources/refaster/PreConditionsVerifierRecipes.java rename to src/test/resources/refaster/PreconditionsVerifierRecipes.java index 859c64fd..dc8d3f0a 100644 --- a/src/test/resources/refaster/PreConditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreconditionsVerifierRecipes.java @@ -35,15 +35,15 @@ import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.*; /** - * OpenRewrite recipes created for Refaster template {@code foo.PreConditionsVerifier}. + * OpenRewrite recipes created for Refaster template {@code foo.PreconditionsVerifier}. */ @SuppressWarnings("all") @Generated("org.openrewrite.java.template.processor.RefasterTemplateProcessor") -public class PreConditionsVerifierRecipes extends Recipe { +public class PreconditionsVerifierRecipes extends Recipe { /** * Instantiates a new instance. */ - public PreConditionsVerifierRecipes() {} + public PreconditionsVerifierRecipes() {} @Override public String getDisplayName() { @@ -52,7 +52,7 @@ public String getDisplayName() { @Override public String getDescription() { - return "Refaster template recipes for `foo.PreConditionsVerifier`."; + return "Refaster template recipes for `foo.PreconditionsVerifier`."; } @Override @@ -69,7 +69,7 @@ public List getRecipeList() { } /** - * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString}. + * OpenRewrite recipe created for Refaster template {@code PreconditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString}. */ @SuppressWarnings("all") @NullMarked @@ -83,7 +83,7 @@ public NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrStringRecipe() {} @Override public String getDisplayName() { - return "Refaster template `PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString`"; + return "Refaster template `PreconditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString`"; } @Override @@ -135,7 +135,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { } /** - * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody}. + * OpenRewrite recipe created for Refaster template {@code PreconditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody}. */ @SuppressWarnings("all") @NullMarked @@ -149,7 +149,7 @@ public UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBod @Override public String getDisplayName() { - return "Refaster template `PreConditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody`"; + return "Refaster template `PreconditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInSomeBeforeBody`"; } @Override @@ -209,7 +209,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { } /** - * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody}. + * OpenRewrite recipe created for Refaster template {@code PreconditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody}. */ @SuppressWarnings("all") @NullMarked @@ -223,7 +223,7 @@ public UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody @Override public String getDisplayName() { - return "Refaster template `PreConditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody`"; + return "Refaster template `PreconditionsVerifier.UsesTypeWhenBeforeTemplateContainsPrimitiveOrStringAndTypeInAllBeforeBody`"; } @Override @@ -281,7 +281,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { } /** - * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType}. + * OpenRewrite recipe created for Refaster template {@code PreconditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType}. */ @SuppressWarnings("all") @NullMarked @@ -295,7 +295,7 @@ public NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherTypeRecipe() {} @Override public String getDisplayName() { - return "Refaster template `PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType`"; + return "Refaster template `PreconditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType`"; } @Override @@ -348,7 +348,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { } /** - * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType}. + * OpenRewrite recipe created for Refaster template {@code PreconditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType}. */ @SuppressWarnings("all") @NullMarked @@ -362,7 +362,7 @@ public NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherTypeRecipe() {} @Override public String getDisplayName() { - return "Refaster template `PreConditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType`"; + return "Refaster template `PreconditionsVerifier.NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType`"; } @Override @@ -415,7 +415,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { } /** - * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.UsesTypeMapWhenAllBeforeTemplatesContainsMap}. + * OpenRewrite recipe created for Refaster template {@code PreconditionsVerifier.UsesTypeMapWhenAllBeforeTemplatesContainsMap}. */ @SuppressWarnings("all") @NullMarked @@ -429,7 +429,7 @@ public UsesTypeMapWhenAllBeforeTemplatesContainsMapRecipe() {} @Override public String getDisplayName() { - return "Refaster template `PreConditionsVerifier.UsesTypeMapWhenAllBeforeTemplatesContainsMap`"; + return "Refaster template `PreconditionsVerifier.UsesTypeMapWhenAllBeforeTemplatesContainsMap`"; } @Override @@ -484,7 +484,7 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { } /** - * OpenRewrite recipe created for Refaster template {@code PreConditionsVerifier.UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList}. + * OpenRewrite recipe created for Refaster template {@code PreconditionsVerifier.UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList}. */ @SuppressWarnings("all") @NullMarked @@ -498,7 +498,7 @@ public UsesTypeMapOrListWhenBeforeTemplateContainsMapAndListRecipe() {} @Override public String getDisplayName() { - return "Refaster template `PreConditionsVerifier.UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList`"; + return "Refaster template `PreconditionsVerifier.UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList`"; } @Override From 23f9475a07559b0aa194258b140e5241d2fec54b Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 13:29:04 +0100 Subject: [PATCH 28/35] Change name `PreConditionsVerifier` => `PreconditionsVerifier` --- src/test/resources/refaster/PreconditionsVerifier.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/resources/refaster/PreconditionsVerifier.java b/src/test/resources/refaster/PreconditionsVerifier.java index 415fb600..3197b5af 100644 --- a/src/test/resources/refaster/PreconditionsVerifier.java +++ b/src/test/resources/refaster/PreconditionsVerifier.java @@ -15,13 +15,13 @@ */ package foo; -import java.util.List; -import java.util.Map; - import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate; import com.sun.tools.javac.util.Convert; +import java.util.List; +import java.util.Map; + /** * A refaster template to test when a `UsesType`and Preconditions.or should or should not be applied to the Preconditions check. */ From d458031a255d19ad22cba28a0ea249cd0594e1d6 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 15:27:29 +0100 Subject: [PATCH 29/35] Improvement: extractCommonElements --- .../java/template/processor/Precondition.java | 62 +++++++++ .../template/processor/PreconditionTest.java | 119 +++++++++++------- .../refaster/NestedPreconditionsRecipe.java | 21 ++-- .../PreconditionsVerifierRecipes.java | 11 +- .../refaster/RefasterAnyOfRecipes.java | 21 ++-- 5 files changed, 162 insertions(+), 72 deletions(-) diff --git a/src/main/java/org/openrewrite/java/template/processor/Precondition.java b/src/main/java/org/openrewrite/java/template/processor/Precondition.java index 29742d4a..ec652288 100644 --- a/src/main/java/org/openrewrite/java/template/processor/Precondition.java +++ b/src/main/java/org/openrewrite/java/template/processor/Precondition.java @@ -76,6 +76,25 @@ boolean fitsInto(Precondition p) { @Override public Precondition prune() { + Precondition pruned = takeElementIfItFitsInAllOtherElements(); + return pruned == null ? extractCommonElements() : pruned; + } + + /** + * If element fits in all others, take element as precondition. Eg: + * 

+         * or(
+         *    and(new UsesType<>("Map"), new UsesMethod<>("PrintStream println(..)")),
+         *    new UsesMethod<>("PrintStream println(..)")
+         * )
+         *
+ *

+ * will return: + * 

+         * new UsesMethod<>("PrintStream println()")
+         *
+ */ + private Precondition takeElementIfItFitsInAllOtherElements() { for (Precondition p : preconditions) { int matches = 0; for (Precondition p2 : preconditions) { @@ -87,6 +106,49 @@ public Precondition prune() { } } } + return null; + } + + /** + * If child element of an element exist as child element in all others, move child element up. Eg: + * 
+         * or(
+         *    and(new UsesType<>("Map"), new UsesType<>("HashMap"), new UsesMethod<>("PrintStream println(..)")),
+         *    and(new UsesType<>("List"), new UsesType<>("ArrayList"), new UsesMethod<>("PrintStream println(..)"))
+         * )
+         *
+ *

+ * will return: + * 

+         * and(
+         *    new UsesMethod<>("PrintStream println()"),
+         *    or(
+         *      and(new UsesType<>("Map"), new UsesType<>("HashMap")),
+         *      and(new UsesType<>("List"), new UsesType<>("ArrayList")),
+         *    )
+         * )
+         *
+ */ + private Precondition extractCommonElements() { + boolean first = true; + Set commons = new HashSet<>(); + for (Precondition p : preconditions) { + if (!(p instanceof And)) { + return this; + } + if (first) { + commons.addAll(((And) p).preconditions); + first = false; + } else { + commons.retainAll(((And) p).preconditions); + } + } + + if (!commons.isEmpty()) { + preconditions.forEach(it -> ((And) it).preconditions.removeAll(commons)); + commons.add(new Or(preconditions, indent)); + return new And(commons, indent); + } return this; } diff --git a/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java b/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java index bfcac061..127ef7fe 100644 --- a/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java +++ b/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java @@ -16,6 +16,9 @@ package org.openrewrite.java.template.processor; import org.junit.jupiter.api.Test; +import org.openrewrite.java.template.processor.Precondition.And; +import org.openrewrite.java.template.processor.Precondition.Or; +import org.openrewrite.java.template.processor.Precondition.Rule; import java.util.Arrays; import java.util.HashSet; @@ -26,20 +29,20 @@ class PreconditionTest { @Test void toStringWithInden() { - String result = new Precondition.Or( + String result = new Or( setOf( - new Precondition.And( + new And( setOf( - new Precondition.Rule("A"), - new Precondition.Rule("B"), - new Precondition.Rule("C")), + new Rule("A"), + new Rule("B"), + new Rule("C")), 4 ), - new Precondition.And( + new And( setOf( - new Precondition.Rule("X"), - new Precondition.Rule("Y"), - new Precondition.Rule("Z")), + new Rule("X"), + new Rule("Y"), + new Rule("Z")), 4 ) ), 4).toString(); @@ -60,16 +63,16 @@ void toStringWithInden() { @Test void ruleFitsInRule() { - assertThat(new Precondition.Rule("A").fitsInto(new Precondition.Rule("A"))).isTrue(); + assertThat(new Rule("A").fitsInto(new Rule("A"))).isTrue(); } @Test void orFitsInOr() { - boolean result = new Precondition.Or( - setOf(new Precondition.Rule("A"), new Precondition.Rule("B")), + boolean result = new Or( + setOf(new Rule("A"), new Rule("B")), 4 - ).fitsInto(new Precondition.Or( - setOf(new Precondition.Rule("B"), new Precondition.Rule("A")), + ).fitsInto(new Or( + setOf(new Rule("B"), new Rule("A")), 4 )); @@ -78,11 +81,11 @@ void orFitsInOr() { @Test void ardFitsNotInAndWithDifferentRules() { - boolean result = new Precondition.Or( - setOf(new Precondition.Rule("A"), new Precondition.Rule("C")), + boolean result = new Or( + setOf(new Rule("A"), new Rule("C")), 4 - ).fitsInto(new Precondition.Or( - setOf(new Precondition.Rule("B"), new Precondition.Rule("A")), + ).fitsInto(new Or( + setOf(new Rule("B"), new Rule("A")), 4 )); @@ -91,11 +94,11 @@ void ardFitsNotInAndWithDifferentRules() { @Test void andFitsInAnd() { - boolean result = new Precondition.And( - setOf(new Precondition.Rule("A")), + boolean result = new And( + setOf(new Rule("A")), 4 - ).fitsInto(new Precondition.And( - setOf(new Precondition.Rule("B"), new Precondition.Rule("A")), + ).fitsInto(new And( + setOf(new Rule("B"), new Rule("A")), 4 )); @@ -104,11 +107,11 @@ void andFitsInAnd() { @Test void andFitsNotInAndWithDifferentRules() { - boolean result = new Precondition.And( - setOf(new Precondition.Rule("A"), new Precondition.Rule("C")), + boolean result = new And( + setOf(new Rule("A"), new Rule("C")), 4 - ).fitsInto(new Precondition.And( - setOf(new Precondition.Rule("B"), new Precondition.Rule("A")), + ).fitsInto(new And( + setOf(new Rule("B"), new Rule("A")), 4 )); @@ -117,60 +120,86 @@ void andFitsNotInAndWithDifferentRules() { @Test void sameRulesArePrunedAutomatically() { - Set result = setOf(new Precondition.Rule("A"), new Precondition.Rule("A")); + Set result = setOf(new Rule("A"), new Rule("A")); - assertThat(result).isEqualTo(setOf(new Precondition.Rule("A"))); + assertThat(result).isEqualTo(setOf(new Rule("A"))); } @Test void sameRulesArePrunedAutomaticallyInAnOr() { - Precondition result = new Precondition.Or( - setOf(new Precondition.Rule("A"), new Precondition.Rule("A")), + Precondition result = new Or( + setOf(new Rule("A"), new Rule("A")), 4 ); - assertThat(result).isEqualTo(new Precondition.Or( - setOf(new Precondition.Rule("A")), + assertThat(result).isEqualTo(new Or( + setOf(new Rule("A")), 4 )); } @Test void pruneOrWithAnds() { - Precondition result = new Precondition.Or( + Precondition result = new Or( setOf( - new Precondition.And( - setOf(new Precondition.Rule("A"), new Precondition.Rule("B")), + new And( + setOf(new Rule("A"), new Rule("B")), 4 ), - new Precondition.And( - setOf(new Precondition.Rule("A"), new Precondition.Rule("B"), new Precondition.Rule("C")), + new And( + setOf(new Rule("A"), new Rule("B"), new Rule("C")), 4 ) ), 4).prune(); - assertThat(result).isEqualTo(new Precondition.And( - setOf(new Precondition.Rule("A"), new Precondition.Rule("B")), + assertThat(result).isEqualTo(new And( + setOf(new Rule("A"), new Rule("B")), 4 )); } @Test void pruneOrWithAndAndRule() { - Precondition result = new Precondition.Or( + Precondition result = new Or( setOf( - new Precondition.And( - setOf(new Precondition.Rule("A"), new Precondition.Rule("B")), + new And( + setOf(new Rule("A"), new Rule("B")), 4 ), - new Precondition.Rule("B") + new Rule("B") ), 4).prune(); - assertThat(result).isEqualTo(new Precondition.Rule("B")); + assertThat(result).isEqualTo(new Rule("B")); + } + + @Test + void pruneOrWithTypeChange() { + Precondition result = new Or( + setOf( + new And( + setOf(new Rule("A"), new Rule("B"), new Rule("C")), + 4 + ), + new And( + setOf(new Rule("D"), new Rule("B"), new Rule("E")), + 4 + ) + ), 4).prune(); + + assertThat(result).isEqualTo(new And( + setOf( + new Rule("B"), + new Or( + setOf( + new And(setOf(new Rule("A"), new Rule("C")), 4), + new And(setOf(new Rule("D"), new Rule("E")), 4) + ), 4 + ) + ), 4)); } @SafeVarargs - private final Set setOf(T... rules) { + private static Set setOf(T... rules) { return new HashSet<>(Arrays.asList(rules)); } } diff --git a/src/test/resources/refaster/NestedPreconditionsRecipe.java b/src/test/resources/refaster/NestedPreconditionsRecipe.java index d0aa6638..40f46988 100644 --- a/src/test/resources/refaster/NestedPreconditionsRecipe.java +++ b/src/test/resources/refaster/NestedPreconditionsRecipe.java @@ -96,16 +96,17 @@ public J visitNewClass(J.NewClass elem, ExecutionContext ctx) { }; return Preconditions.check( - Preconditions.or( - Preconditions.and( - new UsesType<>("java.util.HashMap", true), - new UsesType<>("java.util.Map", true), - new UsesMethod<>("java.util.HashMap (..)", true) - ), - Preconditions.and( - new UsesType<>("java.util.LinkedHashMap", true), - new UsesType<>("java.util.Map", true), - new UsesMethod<>("java.util.LinkedHashMap (..)", true) + Preconditions.and( + new UsesType<>("java.util.Map", true), + Preconditions.or( + Preconditions.and( + new UsesType<>("java.util.HashMap", true), + new UsesMethod<>("java.util.HashMap (..)", true) + ), + Preconditions.and( + new UsesType<>("java.util.LinkedHashMap", true), + new UsesMethod<>("java.util.LinkedHashMap (..)", true) + ) ) ), javaVisitor diff --git a/src/test/resources/refaster/PreconditionsVerifierRecipes.java b/src/test/resources/refaster/PreconditionsVerifierRecipes.java index dc8d3f0a..eae10ce8 100644 --- a/src/test/resources/refaster/PreconditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreconditionsVerifierRecipes.java @@ -545,15 +545,12 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { }; return Preconditions.check( - Preconditions.or( - Preconditions.and( + Preconditions.and( + Preconditions.or( new UsesType<>("java.util.List", true), - new UsesMethod<>("java.io.PrintStream println(..)", true) + new UsesType<>("java.util.Map", true) ), - Preconditions.and( - new UsesType<>("java.util.Map", true), - new UsesMethod<>("java.io.PrintStream println(..)", true) - ) + new UsesMethod<>("java.io.PrintStream println(..)", true) ), javaVisitor ); diff --git a/src/test/resources/refaster/RefasterAnyOfRecipes.java b/src/test/resources/refaster/RefasterAnyOfRecipes.java index a531afde..ef2cd7c1 100644 --- a/src/test/resources/refaster/RefasterAnyOfRecipes.java +++ b/src/test/resources/refaster/RefasterAnyOfRecipes.java @@ -192,16 +192,17 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { }; return Preconditions.check( - Preconditions.or( - Preconditions.and( - new UsesType<>("java.util.Collections", true), - new UsesType<>("java.util.List", true), - new UsesMethod<>("java.util.Collections emptyList(..)", true) - ), - Preconditions.and( - new UsesType<>("java.util.LinkedList", true), - new UsesType<>("java.util.List", true), - new UsesMethod<>("java.util.LinkedList (..)", true) + Preconditions.and( + new UsesType<>("java.util.List", true), + Preconditions.or( + Preconditions.and( + new UsesType<>("java.util.Collections", true), + new UsesMethod<>("java.util.Collections emptyList(..)", true) + ), + Preconditions.and( + new UsesType<>("java.util.LinkedList", true), + new UsesMethod<>("java.util.LinkedList (..)", true) + ) ) ), javaVisitor From 44044407b9dd416762be836f431ca6209db1e653 Mon Sep 17 00:00:00 2001 From: Jacob van Lingen Date: Mon, 16 Dec 2024 15:47:51 +0100 Subject: [PATCH 30/35] Update src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java Co-authored-by: Tim te Beek --- .../java/template/processor/RefasterTemplateProcessor.java | 2 ++ 1 file changed, 2 insertions(+) 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 ee5465c8..01ada198 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -611,6 +611,8 @@ private Set getImportsAsStrings(Map> imp if (!usesVisitors.isEmpty()) { preconditions.put(beforeTemplate.method.name.toString() + (arity == 1 ? "" : "$" + i), usesVisitors); + } else { + return null; // At least one of the before templates has no preconditions, so we can not use any preconditions } } } From 52d6dbb222637979e422182fa243326a869cf066 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 15:57:07 +0100 Subject: [PATCH 31/35] Change test accordingly --- .../refaster/PreconditionsVerifier.java | 20 ++-- .../PreconditionsVerifierRecipes.java | 102 +++++++++--------- 2 files changed, 62 insertions(+), 60 deletions(-) diff --git a/src/test/resources/refaster/PreconditionsVerifier.java b/src/test/resources/refaster/PreconditionsVerifier.java index 3197b5af..e4e7ceab 100644 --- a/src/test/resources/refaster/PreconditionsVerifier.java +++ b/src/test/resources/refaster/PreconditionsVerifier.java @@ -29,12 +29,12 @@ public class PreconditionsVerifier { public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString { @BeforeTemplate void doubleAndInt(double actual, int ignore) { - System.out.println(actual); + double s = actual; } @BeforeTemplate void stringAndString(String actual, String ignore) { - System.out.println(actual); + String s = actual; } @AfterTemplate @@ -80,12 +80,12 @@ Object after(Object actual) { public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType { @BeforeTemplate void _int(int actual) { - System.out.println(actual); + int s = actual; } @BeforeTemplate void map(Map actual) { - System.out.println(actual); + Map m = actual; } @AfterTemplate @@ -97,12 +97,12 @@ void after(Object actual) { public static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType { @BeforeTemplate void string(String actual) { - System.out.println(actual); + String s = actual; } @BeforeTemplate void map(Map actual) { - System.out.println(actual); + Map m = (Map) actual; } @AfterTemplate @@ -114,12 +114,12 @@ void after(Object actual) { public static class UsesTypeMapWhenAllBeforeTemplatesContainsMap { @BeforeTemplate void mapWithGeneric(Map actual) { - System.out.println(actual); + Map m = actual; } @BeforeTemplate void mapWithGenericTwo(Map actual) { - System.out.println(actual); + Map m = actual; } @AfterTemplate @@ -131,12 +131,12 @@ void mapWithoutGeneric(Map actual) { public static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList { @BeforeTemplate void list(List actual) { - System.out.println(actual); + List l = actual; } @BeforeTemplate void map(Map actual) { - System.out.println(actual); + Map m = actual; } @AfterTemplate diff --git a/src/test/resources/refaster/PreconditionsVerifierRecipes.java b/src/test/resources/refaster/PreconditionsVerifierRecipes.java index eae10ce8..e206c2e8 100644 --- a/src/test/resources/refaster/PreconditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreconditionsVerifierRecipes.java @@ -88,24 +88,28 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, int ignore) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String ignore) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, int ignore) {\n double s = actual;\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String ignore) {\n String s = actual;\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { - JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + return new AbstractRefasterJavaVisitor() { final JavaTemplate doubleAndInt = JavaTemplate - .builder("System.out.println(#{actual:any(double)});") + .builder("double s = #{actual:any(double)};") .build(); final JavaTemplate stringAndString = JavaTemplate - .builder("System.out.println(#{actual:any(java.lang.String)});") + .builder("String s = #{actual:any(java.lang.String)};") .build(); final JavaTemplate after = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") .build(); @Override - public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { + public J visitStatement(Statement elem, ExecutionContext ctx) { + if (elem instanceof J.Block) { + // FIXME workaround + return elem; + } JavaTemplate.Matcher matcher; if ((matcher = doubleAndInt.matcher(getCursor())).find()) { return embed( @@ -123,14 +127,10 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - return super.visitMethodInvocation(elem, ctx); + return super.visitStatement(elem, ctx); } }; - return Preconditions.check( - new UsesMethod<>("java.io.PrintStream println(..)", true), - javaVisitor - ); } } @@ -300,24 +300,28 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType {\n \n @BeforeTemplate()\n void _int(int actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType {\n \n @BeforeTemplate()\n void _int(int actual) {\n int s = actual;\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n Map m = actual;\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { - JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + return new AbstractRefasterJavaVisitor() { final JavaTemplate _int = JavaTemplate - .builder("System.out.println(#{actual:any(int)});") + .builder("int s = #{actual:any(int)};") .build(); final JavaTemplate map = JavaTemplate - .builder("System.out.println(#{actual:any(java.util.Map)});") + .builder("java.util.Map m = #{actual:any(java.util.Map)};") .build(); final JavaTemplate after = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") .build(); @Override - public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { + public J visitStatement(Statement elem, ExecutionContext ctx) { + if (elem instanceof J.Block) { + // FIXME workaround + return elem; + } JavaTemplate.Matcher matcher; if ((matcher = _int.matcher(getCursor())).find()) { return embed( @@ -336,14 +340,10 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - return super.visitMethodInvocation(elem, ctx); + return super.visitStatement(elem, ctx); } }; - return Preconditions.check( - new UsesMethod<>("java.io.PrintStream println(..)", true), - javaVisitor - ); } } @@ -367,24 +367,28 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType {\n \n @BeforeTemplate()\n void string(String actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType {\n \n @BeforeTemplate()\n void string(String actual) {\n String s = actual;\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n Map m = (Map)actual;\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { - JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + return new AbstractRefasterJavaVisitor() { final JavaTemplate string = JavaTemplate - .builder("System.out.println(#{actual:any(java.lang.String)});") + .builder("String s = #{actual:any(java.lang.String)};") .build(); final JavaTemplate map = JavaTemplate - .builder("System.out.println(#{actual:any(java.util.Map)});") + .builder("java.util.Map m = (java.util.Map)#{actual:any(java.util.Map)};") .build(); final JavaTemplate after = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") .build(); @Override - public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { + public J visitStatement(Statement elem, ExecutionContext ctx) { + if (elem instanceof J.Block) { + // FIXME workaround + return elem; + } JavaTemplate.Matcher matcher; if ((matcher = string.matcher(getCursor())).find()) { return embed( @@ -403,14 +407,10 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - return super.visitMethodInvocation(elem, ctx); + return super.visitStatement(elem, ctx); } }; - return Preconditions.check( - new UsesMethod<>("java.io.PrintStream println(..)", true), - javaVisitor - ); } } @@ -434,24 +434,28 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapWhenAllBeforeTemplatesContainsMap {\n \n @BeforeTemplate()\n void mapWithGeneric(Map actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void mapWithGenericTwo(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void mapWithoutGeneric(Map actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapWhenAllBeforeTemplatesContainsMap {\n \n @BeforeTemplate()\n void mapWithGeneric(Map actual) {\n Map m = actual;\n }\n \n @BeforeTemplate()\n void mapWithGenericTwo(Map actual) {\n Map m = actual;\n }\n \n @AfterTemplate()\n void mapWithoutGeneric(Map actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate mapWithGeneric = JavaTemplate - .builder("System.out.println(#{actual:any(java.util.Map)});") + .builder("java.util.Map m = #{actual:any(java.util.Map)};") .build(); final JavaTemplate mapWithGenericTwo = JavaTemplate - .builder("System.out.println(#{actual:any(java.util.Map)});") + .builder("java.util.Map m = #{actual:any(java.util.Map)};") .build(); final JavaTemplate mapWithoutGeneric = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.util.Map)});") .build(); @Override - public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { + public J visitStatement(Statement elem, ExecutionContext ctx) { + if (elem instanceof J.Block) { + // FIXME workaround + return elem; + } JavaTemplate.Matcher matcher; if ((matcher = mapWithGeneric.matcher(getCursor())).find()) { return embed( @@ -469,15 +473,12 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - return super.visitMethodInvocation(elem, ctx); + return super.visitStatement(elem, ctx); } }; return Preconditions.check( - Preconditions.and( - new UsesType<>("java.util.Map", true), - new UsesMethod<>("java.io.PrintStream println(..)", true) - ), + new UsesType<>("java.util.Map", true), javaVisitor ); } @@ -503,24 +504,28 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList {\n \n @BeforeTemplate()\n void list(List actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList {\n \n @BeforeTemplate()\n void list(List actual) {\n List l = actual;\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n Map m = actual;\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate list = JavaTemplate - .builder("System.out.println(#{actual:any(java.util.List)});") + .builder("java.util.List l = #{actual:any(java.util.List)};") .build(); final JavaTemplate map = JavaTemplate - .builder("System.out.println(#{actual:any(java.util.Map)});") + .builder("java.util.Map m = #{actual:any(java.util.Map)};") .build(); final JavaTemplate after = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") .build(); @Override - public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { + public J visitStatement(Statement elem, ExecutionContext ctx) { + if (elem instanceof J.Block) { + // FIXME workaround + return elem; + } JavaTemplate.Matcher matcher; if ((matcher = list.matcher(getCursor())).find()) { maybeRemoveImport("java.util.List"); @@ -540,17 +545,14 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - return super.visitMethodInvocation(elem, ctx); + return super.visitStatement(elem, ctx); } }; return Preconditions.check( - Preconditions.and( - Preconditions.or( - new UsesType<>("java.util.List", true), - new UsesType<>("java.util.Map", true) - ), - new UsesMethod<>("java.io.PrintStream println(..)", true) + Preconditions.or( + new UsesType<>("java.util.List", true), + new UsesType<>("java.util.Map", true) ), javaVisitor ); From 605b1d782faaabb337d46e612bd58fbe64e548f2 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 16 Dec 2024 16:31:05 +0100 Subject: [PATCH 32/35] Revert test --- .../refaster/PreconditionsVerifier.java | 20 ++-- .../PreconditionsVerifierRecipes.java | 102 +++++++++--------- 2 files changed, 60 insertions(+), 62 deletions(-) diff --git a/src/test/resources/refaster/PreconditionsVerifier.java b/src/test/resources/refaster/PreconditionsVerifier.java index e4e7ceab..3197b5af 100644 --- a/src/test/resources/refaster/PreconditionsVerifier.java +++ b/src/test/resources/refaster/PreconditionsVerifier.java @@ -29,12 +29,12 @@ public class PreconditionsVerifier { public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString { @BeforeTemplate void doubleAndInt(double actual, int ignore) { - double s = actual; + System.out.println(actual); } @BeforeTemplate void stringAndString(String actual, String ignore) { - String s = actual; + System.out.println(actual); } @AfterTemplate @@ -80,12 +80,12 @@ Object after(Object actual) { public static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType { @BeforeTemplate void _int(int actual) { - int s = actual; + System.out.println(actual); } @BeforeTemplate void map(Map actual) { - Map m = actual; + System.out.println(actual); } @AfterTemplate @@ -97,12 +97,12 @@ void after(Object actual) { public static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType { @BeforeTemplate void string(String actual) { - String s = actual; + System.out.println(actual); } @BeforeTemplate void map(Map actual) { - Map m = (Map) actual; + System.out.println(actual); } @AfterTemplate @@ -114,12 +114,12 @@ void after(Object actual) { public static class UsesTypeMapWhenAllBeforeTemplatesContainsMap { @BeforeTemplate void mapWithGeneric(Map actual) { - Map m = actual; + System.out.println(actual); } @BeforeTemplate void mapWithGenericTwo(Map actual) { - Map m = actual; + System.out.println(actual); } @AfterTemplate @@ -131,12 +131,12 @@ void mapWithoutGeneric(Map actual) { public static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList { @BeforeTemplate void list(List actual) { - List l = actual; + System.out.println(actual); } @BeforeTemplate void map(Map actual) { - Map m = actual; + System.out.println(actual); } @AfterTemplate diff --git a/src/test/resources/refaster/PreconditionsVerifierRecipes.java b/src/test/resources/refaster/PreconditionsVerifierRecipes.java index e206c2e8..eae10ce8 100644 --- a/src/test/resources/refaster/PreconditionsVerifierRecipes.java +++ b/src/test/resources/refaster/PreconditionsVerifierRecipes.java @@ -88,28 +88,24 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, int ignore) {\n double s = actual;\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String ignore) {\n String s = actual;\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveOrString {\n \n @BeforeTemplate()\n void doubleAndInt(double actual, int ignore) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void stringAndString(String actual, String ignore) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { - return new AbstractRefasterJavaVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate doubleAndInt = JavaTemplate - .builder("double s = #{actual:any(double)};") + .builder("System.out.println(#{actual:any(double)});") .build(); final JavaTemplate stringAndString = JavaTemplate - .builder("String s = #{actual:any(java.lang.String)};") + .builder("System.out.println(#{actual:any(java.lang.String)});") .build(); final JavaTemplate after = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") .build(); @Override - public J visitStatement(Statement elem, ExecutionContext ctx) { - if (elem instanceof J.Block) { - // FIXME workaround - return elem; - } + public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; if ((matcher = doubleAndInt.matcher(getCursor())).find()) { return embed( @@ -127,10 +123,14 @@ public J visitStatement(Statement elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - return super.visitStatement(elem, ctx); + return super.visitMethodInvocation(elem, ctx); } }; + return Preconditions.check( + new UsesMethod<>("java.io.PrintStream println(..)", true), + javaVisitor + ); } } @@ -300,28 +300,24 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType {\n \n @BeforeTemplate()\n void _int(int actual) {\n int s = actual;\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n Map m = actual;\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsPrimitiveAndAnotherType {\n \n @BeforeTemplate()\n void _int(int actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { - return new AbstractRefasterJavaVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate _int = JavaTemplate - .builder("int s = #{actual:any(int)};") + .builder("System.out.println(#{actual:any(int)});") .build(); final JavaTemplate map = JavaTemplate - .builder("java.util.Map m = #{actual:any(java.util.Map)};") + .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); final JavaTemplate after = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") .build(); @Override - public J visitStatement(Statement elem, ExecutionContext ctx) { - if (elem instanceof J.Block) { - // FIXME workaround - return elem; - } + public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; if ((matcher = _int.matcher(getCursor())).find()) { return embed( @@ -340,10 +336,14 @@ public J visitStatement(Statement elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - return super.visitStatement(elem, ctx); + return super.visitMethodInvocation(elem, ctx); } }; + return Preconditions.check( + new UsesMethod<>("java.io.PrintStream println(..)", true), + javaVisitor + ); } } @@ -367,28 +367,24 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType {\n \n @BeforeTemplate()\n void string(String actual) {\n String s = actual;\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n Map m = (Map)actual;\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class NoUsesTypeWhenBeforeTemplateContainsStringAndAnotherType {\n \n @BeforeTemplate()\n void string(String actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { - return new AbstractRefasterJavaVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate string = JavaTemplate - .builder("String s = #{actual:any(java.lang.String)};") + .builder("System.out.println(#{actual:any(java.lang.String)});") .build(); final JavaTemplate map = JavaTemplate - .builder("java.util.Map m = (java.util.Map)#{actual:any(java.util.Map)};") + .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); final JavaTemplate after = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") .build(); @Override - public J visitStatement(Statement elem, ExecutionContext ctx) { - if (elem instanceof J.Block) { - // FIXME workaround - return elem; - } + public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; if ((matcher = string.matcher(getCursor())).find()) { return embed( @@ -407,10 +403,14 @@ public J visitStatement(Statement elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - return super.visitStatement(elem, ctx); + return super.visitMethodInvocation(elem, ctx); } }; + return Preconditions.check( + new UsesMethod<>("java.io.PrintStream println(..)", true), + javaVisitor + ); } } @@ -434,28 +434,24 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapWhenAllBeforeTemplatesContainsMap {\n \n @BeforeTemplate()\n void mapWithGeneric(Map actual) {\n Map m = actual;\n }\n \n @BeforeTemplate()\n void mapWithGenericTwo(Map actual) {\n Map m = actual;\n }\n \n @AfterTemplate()\n void mapWithoutGeneric(Map actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapWhenAllBeforeTemplatesContainsMap {\n \n @BeforeTemplate()\n void mapWithGeneric(Map actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void mapWithGenericTwo(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void mapWithoutGeneric(Map actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate mapWithGeneric = JavaTemplate - .builder("java.util.Map m = #{actual:any(java.util.Map)};") + .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); final JavaTemplate mapWithGenericTwo = JavaTemplate - .builder("java.util.Map m = #{actual:any(java.util.Map)};") + .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); final JavaTemplate mapWithoutGeneric = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.util.Map)});") .build(); @Override - public J visitStatement(Statement elem, ExecutionContext ctx) { - if (elem instanceof J.Block) { - // FIXME workaround - return elem; - } + public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; if ((matcher = mapWithGeneric.matcher(getCursor())).find()) { return embed( @@ -473,12 +469,15 @@ public J visitStatement(Statement elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - return super.visitStatement(elem, ctx); + return super.visitMethodInvocation(elem, ctx); } }; return Preconditions.check( - new UsesType<>("java.util.Map", true), + Preconditions.and( + new UsesType<>("java.util.Map", true), + new UsesMethod<>("java.io.PrintStream println(..)", true) + ), javaVisitor ); } @@ -504,28 +503,24 @@ public String getDisplayName() { @Override public String getDescription() { - return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList {\n \n @BeforeTemplate()\n void list(List actual) {\n List l = actual;\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n Map m = actual;\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; + return "Recipe created for the following Refaster template:\n```java\npublic static class UsesTypeMapOrListWhenBeforeTemplateContainsMapAndList {\n \n @BeforeTemplate()\n void list(List actual) {\n System.out.println(actual);\n }\n \n @BeforeTemplate()\n void map(Map actual) {\n System.out.println(actual);\n }\n \n @AfterTemplate()\n void after(Object actual) {\n System.out.println(\"Changed: \" + actual);\n }\n}\n```\n."; } @Override public TreeVisitor getVisitor() { JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { final JavaTemplate list = JavaTemplate - .builder("java.util.List l = #{actual:any(java.util.List)};") + .builder("System.out.println(#{actual:any(java.util.List)});") .build(); final JavaTemplate map = JavaTemplate - .builder("java.util.Map m = #{actual:any(java.util.Map)};") + .builder("System.out.println(#{actual:any(java.util.Map)});") .build(); final JavaTemplate after = JavaTemplate .builder("System.out.println(\"Changed: \" + #{actual:any(java.lang.Object)});") .build(); @Override - public J visitStatement(Statement elem, ExecutionContext ctx) { - if (elem instanceof J.Block) { - // FIXME workaround - return elem; - } + public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; if ((matcher = list.matcher(getCursor())).find()) { maybeRemoveImport("java.util.List"); @@ -545,14 +540,17 @@ public J visitStatement(Statement elem, ExecutionContext ctx) { SHORTEN_NAMES ); } - return super.visitStatement(elem, ctx); + return super.visitMethodInvocation(elem, ctx); } }; return Preconditions.check( - Preconditions.or( - new UsesType<>("java.util.List", true), - new UsesType<>("java.util.Map", true) + Preconditions.and( + Preconditions.or( + new UsesType<>("java.util.List", true), + new UsesType<>("java.util.Map", true) + ), + new UsesMethod<>("java.io.PrintStream println(..)", true) ), javaVisitor ); From 3856a7ce9181b6ca850f1746f3bc5a14c22c60ba Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 17 Dec 2024 09:20:20 +0100 Subject: [PATCH 33/35] Remove unneeded `RequiredArgsConstructor` --- .../openrewrite/java/template/processor/Precondition.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/main/java/org/openrewrite/java/template/processor/Precondition.java b/src/main/java/org/openrewrite/java/template/processor/Precondition.java index ec652288..fd3cb21d 100644 --- a/src/main/java/org/openrewrite/java/template/processor/Precondition.java +++ b/src/main/java/org/openrewrite/java/template/processor/Precondition.java @@ -16,14 +16,12 @@ package org.openrewrite.java.template.processor; import lombok.EqualsAndHashCode; -import lombok.RequiredArgsConstructor; import lombok.Value; import java.util.*; import static java.util.stream.Collectors.toCollection; -@RequiredArgsConstructor public abstract class Precondition { private static final Comparator BY_USES_TYPE_FIRST = Comparator .comparing((String s) -> !s.startsWith("new UsesType")) @@ -37,7 +35,6 @@ public Precondition prune() { @Value @EqualsAndHashCode(callSuper = false) - @RequiredArgsConstructor public static class Rule extends Precondition { String rule; @@ -61,7 +58,6 @@ public String toString() { @Value @EqualsAndHashCode(callSuper = false) - @RequiredArgsConstructor public static class Or extends Precondition { Set preconditions; int indent; @@ -161,7 +157,6 @@ public String toString() { @Value @EqualsAndHashCode(callSuper = false) - @RequiredArgsConstructor public static class And extends Precondition { Set preconditions; int indent; From 7822f2e288b791e1533d79a0b6575f0831d614f2 Mon Sep 17 00:00:00 2001 From: Jacob van Lingen Date: Tue, 17 Dec 2024 10:12:24 +0100 Subject: [PATCH 34/35] Apply suggestions from code review Co-authored-by: Tim te Beek --- .../org/openrewrite/java/template/processor/Precondition.java | 4 ++-- .../java/template/processor/RefasterTemplateProcessor.java | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/java/template/processor/Precondition.java b/src/main/java/org/openrewrite/java/template/processor/Precondition.java index fd3cb21d..923e1775 100644 --- a/src/main/java/org/openrewrite/java/template/processor/Precondition.java +++ b/src/main/java/org/openrewrite/java/template/processor/Precondition.java @@ -57,7 +57,7 @@ public String toString() { } @Value - @EqualsAndHashCode(callSuper = false) + @EqualsAndHashCode(callSuper = false, of = "preconditions") public static class Or extends Precondition { Set preconditions; int indent; @@ -157,7 +157,7 @@ public String toString() { @Value @EqualsAndHashCode(callSuper = false) - public static class And extends Precondition { + @EqualsAndHashCode(callSuper = false, of = "preconditions") Set preconditions; int indent; 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 01ada198..cc6f980e 100644 --- a/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java +++ b/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java @@ -606,7 +606,8 @@ private Set getImportsAsStrings(Map> imp } String methodName = method.name.toString(); methodName = methodName.equals("") ? "" : methodName; - usesVisitors.add(new Precondition.Rule("new UsesMethod<>(\"" + method.owner.getQualifiedName().toString() + ' ' + methodName + "(..)\", true)")); + usesVisitors.add(new Precondition.Rule(String.format("new UsesMethod<>(\"%s %s(..)\", true)", + method.owner.getQualifiedName().toString(), methodName))); } if (!usesVisitors.isEmpty()) { From ef1ed2dd330f3ad3edd597a483dbae0eec5a5657 Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 17 Dec 2024 10:14:36 +0100 Subject: [PATCH 35/35] Apply suggestions from code review --- .../org/openrewrite/java/template/processor/Precondition.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/java/template/processor/Precondition.java b/src/main/java/org/openrewrite/java/template/processor/Precondition.java index 923e1775..365c825e 100644 --- a/src/main/java/org/openrewrite/java/template/processor/Precondition.java +++ b/src/main/java/org/openrewrite/java/template/processor/Precondition.java @@ -156,8 +156,8 @@ public String toString() { } @Value - @EqualsAndHashCode(callSuper = false) @EqualsAndHashCode(callSuper = false, of = "preconditions") + public static class And extends Precondition { Set preconditions; int indent;