Add an example Yaml recipe test (#40)

* Add an example Yaml recipe test * Document each of the tests * Apply formatter and add new test * Sprinkle in some more documentation * Add an example of using TreeVisitingPrinter * Document the use of best practices * Update README.md * Consistently use single line comments * No need to set active recipe in build.gradle.kts
moderneinc · Feb 7, 2024 · 3fc7c41 · 3fc7c41
1 parent 20b3d00
commit 3fc7c41
Show file tree

Hide file tree

Showing 8 changed files with 226 additions and 79 deletions.
diff --git a/README.md b/README.md
@@ -106,3 +106,12 @@ Run the release action to publish a release version of a recipe.
 To build a snapshot, run `./gradlew snapshot publish` to build a snapshot and publish it to Moderne's open artifact repository for inclusion at [app.moderne.io](https://app.moderne.io).
 
 To build a release, run `./gradlew final publish` to tag a release and publish it to Moderne's open artifact repository for inclusion at [app.moderne.io](https://app.moderne.io).
+
+
+## Applying OpenRewrite recipe development best practices
+
+We maintain a collection of [best practices for writing OpenRewrite recipes](https://github.com/openrewrite/rewrite-recommendations/).
+You can apply these recommendations to your recipes by running the following command:
+```bash
+./gradlew rewriteRun -Drewrite.activeRecipe=org.openrewrite.recipes.OpenRewriteBestPractices
+```
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -1,5 +1,8 @@
 plugins {
     id("org.openrewrite.build.recipe-library") version "latest.release"
+
+    // Only needed when you want to apply the OpenRewriteBestPractices recipe to your recipes
+    id("org.openrewrite.rewrite") version "latest.release"
 }
 
 // Set as appropriate for your organization
@@ -28,6 +31,9 @@ dependencies {
 
     // Our recipe converts Guava's `Lists` type
     testRuntimeOnly("com.google.guava:guava:latest.release")
+
+    // Contains the OpenRewriteBestPractices recipe, which you can apply to your recipes
+    rewrite("org.openrewrite.recipe:rewrite-recommendations:latest.release")
 }
 
 configure<PublishingExtension> {

diff --git a/src/main/java/com/yourorg/NoGuavaListsNewArrayList.java b/src/main/java/com/yourorg/NoGuavaListsNewArrayList.java
@@ -25,6 +25,7 @@
 @Value
 @EqualsAndHashCode(callSuper = false)
 public class NoGuavaListsNewArrayList extends Recipe {
+    // These matchers use a syntax described on https://docs.openrewrite.org/reference/method-patterns
     private static final MethodMatcher NEW_ARRAY_LIST = new MethodMatcher("com.google.common.collect.Lists newArrayList()");
     private static final MethodMatcher NEW_ARRAY_LIST_ITERABLE = new MethodMatcher("com.google.common.collect.Lists newArrayList(java.lang.Iterable)");
     private static final MethodMatcher NEW_ARRAY_LIST_CAPACITY = new MethodMatcher("com.google.common.collect.Lists newArrayListWithCapacity(int)");
@@ -67,6 +68,18 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
                     .imports("java.util.ArrayList")
                     .build();
 
+                // This method override is only here to show how to print the AST for debugging purposes.
+                // You can remove this method if you don't need it.
+                @Override
+                public J visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) {
+                    // This is a useful debugging tool if you're ever unsure what the visitor is visiting
+                    String printed = TreeVisitingPrinter.printTree(cu);
+                    System.out.println(printed);
+                    // You must always delegate to the super method to ensure the visitor continues to visit deeper
+                    return super.visitCompilationUnit(cu, ctx);
+                }
+
+                // Visit any method invocation, and replace matches with the new ArrayList instantiation.
                 @Override
                 public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
                     if (NEW_ARRAY_LIST.matches(method)) {

diff --git a/src/main/resources/META-INF/rewrite/rewrite.yml b/src/main/resources/META-INF/rewrite/rewrite.yml
@@ -16,6 +16,8 @@
 
 # Include any Declarative YAML format recipes here, as per:
 # https://docs.openrewrite.org/reference/yaml-format-reference
+# These are most easily composed through the Yaml recipe builder at:
+# https://app.moderne.io/recipes/builder
 ---
 type: specs.openrewrite.org/v1beta/recipe
 name: com.yourorg.RecipeA
@@ -26,3 +28,15 @@ tags:
   - tag2
 recipeList:
   - com.yourorg.NoGuavaListsNewArrayList
+---
+
+# Notice how we can have multiple recipes in the same file, separated by `---`
+# You can also have multiple files in `src/main/resources/META-INF/rewrite`, each containing one or more recipes.
+type: specs.openrewrite.org/v1beta/recipe
+name: com.yourorg.UseOpenRewriteNullable
+displayName: Prefer OpenRewrite Nullable
+description: Replaces JetBrains Nullable with OpenRewrite Nullable.
+recipeList:
+  - org.openrewrite.java.ChangeType:
+      oldFullyQualifiedTypeName: org.jetbrains.annotations.Nullable
+      newFullyQualifiedTypeName: org.openrewrite.internal.lang.Nullable
diff --git a/src/test/java/.editorconfig b/src/test/java/.editorconfig
@@ -1,5 +1,6 @@
 root = true
 
+# Limit continuation indent to 2 spaces for Java files, as we heavily use continuations around our text blocks.
 [*.java]
 indent_size = 4
 ij_continuation_indent_size = 2
diff --git a/src/test/java/com/yourorg/NoGuavaListsNewArrayListTest.java b/src/test/java/com/yourorg/NoGuavaListsNewArrayListTest.java
@@ -23,105 +23,136 @@
 
 import static org.openrewrite.java.Assertions.java;
 
+// This is a test for the NoGuavaListsNewArrayList recipe, as an example of how to write a test for an imperative recipe.
 class NoGuavaListsNewArrayListTest implements RewriteTest {
 
     // Note, you can define defaults for the RecipeSpec and these defaults will be used for all tests.
     // In this case, the recipe and the parser are common. See below, on how the defaults can be overridden
     // per test.
     @Override
     public void defaults(RecipeSpec spec) {
+        // Note how we directly instantiate the recipe class here
         spec.recipe(new NoGuavaListsNewArrayList())
-            .parser(JavaParser.fromJavaVersion()
-                .logCompilationWarningsAndErrors(true)
-                .classpath("guava"));
+          .parser(JavaParser.fromJavaVersion()
+            .logCompilationWarningsAndErrors(true)
+            .classpath("guava"));
     }
 
     @DocumentExample
     @Test
     void replaceWithNewArrayList() {
         rewriteRun(
-            // There is an overloaded version or rewriteRun that allows the RecipeSpec to be customized specifically
-            // for a given test. In this case, the parser for this test is configured to not log compilation warnings.
-            spec -> spec
-                .parser(JavaParser.fromJavaVersion()
-                    .logCompilationWarningsAndErrors(false)
-                    .classpath("guava")),
-            // language=java
-            java(
+          // There is an overloaded version or rewriteRun that allows the RecipeSpec to be customized specifically
+          // for a given test. In this case, the parser for this test is configured to not log compilation warnings.
+          spec -> spec
+            .parser(JavaParser.fromJavaVersion()
+              .logCompilationWarningsAndErrors(false)
+              .classpath("guava")),
+          // language=java
+          java(
+            """
+              import com.google.common.collect.*;
+              
+              import java.util.List;
+              
+              class Test {
+                  List<Integer> cardinalsWorldSeries = Lists.newArrayList();
+              }
+              """,
             """
-                    import com.google.common.collect.*;
-                    
-                    import java.util.List;
-                    
-                    class Test {
-                        List<Integer> cardinalsWorldSeries = Lists.newArrayList();
-                    }
-                    """,
-                """
-                    import java.util.ArrayList;
-                    import java.util.List;
-                    
-                    class Test {
-                        List<Integer> cardinalsWorldSeries = new ArrayList<>();
-                    }
-                    """
-            )
+              import java.util.ArrayList;
+              import java.util.List;
+              
+              class Test {
+                  List<Integer> cardinalsWorldSeries = new ArrayList<>();
+              }
+              """
+          )
         );
     }
 
     @Test
     void replaceWithNewArrayListIterable() {
         rewriteRun(
-            // language=java
-            java(
+          // language=java
+          java(
             """
-                    import com.google.common.collect.*;
-                    
-                    import java.util.Collections;
-                    import java.util.List;
-                    
-                    class Test {
-                        List<Integer> l = Collections.emptyList();
-                        List<Integer> cardinalsWorldSeries = Lists.newArrayList(l);
-                    }
-                    """,
-                """
-                    import java.util.ArrayList;
-                    import java.util.Collections;
-                    import java.util.List;
-                    
-                    class Test {
-                        List<Integer> l = Collections.emptyList();
-                        List<Integer> cardinalsWorldSeries = new ArrayList<>(l);
-                    }
-                    """
-            )
+              import com.google.common.collect.*;
+              
+              import java.util.Collections;
+              import java.util.List;
+              
+              class Test {
+                  List<Integer> l = Collections.emptyList();
+                  List<Integer> cardinalsWorldSeries = Lists.newArrayList(l);
+              }
+              """,
+            """
+              import java.util.ArrayList;
+              import java.util.Collections;
+              import java.util.List;
+              
+              class Test {
+                  List<Integer> l = Collections.emptyList();
+                  List<Integer> cardinalsWorldSeries = new ArrayList<>(l);
+              }
+              """
+          )
         );
     }
 
     @Test
     void replaceWithNewArrayListWithCapacity() {
         rewriteRun(
-            // language=java
-            java(
+          // language=java
+          java(
+            """
+              import com.google.common.collect.*;
+              
+              import java.util.ArrayList;
+              import java.util.List;
+              
+              class Test {
+                  List<Integer> cardinalsWorldSeries = Lists.newArrayListWithCapacity(2);
+              }
+              """,
+            """
+              import java.util.ArrayList;
+              import java.util.List;
+              
+              class Test {
+                  List<Integer> cardinalsWorldSeries = new ArrayList<>(2);
+              }
+              """)
+        );
+    }
+
+    // This test is to show that the `super.visitMethodInvocation` is needed to ensure that nested method invocations are visited.
+    @Test
+    void showNeedForSuperVisitMethodInvocation() {
+        rewriteRun(
+          //language=java
+          java(
             """
-                import com.google.common.collect.*;
-                
-                import java.util.ArrayList;
-                import java.util.List;
-                
-                class Test {
-                    List<Integer> cardinalsWorldSeries = Lists.newArrayListWithCapacity(2);
-                }
-                """,
+              import com.google.common.collect.*;
+              
+              import java.util.Collections;
+              import java.util.List;
+              
+              class Test {
+                  List<Integer> cardinalsWorldSeries = Collections.unmodifiableList(Lists.newArrayList());
+              }
+              """,
             """
-                import java.util.ArrayList;
-                import java.util.List;
-                
-                class Test {
-                    List<Integer> cardinalsWorldSeries = new ArrayList<>(2);
-                }
-                """)
+              import java.util.ArrayList;
+              import java.util.Collections;
+              import java.util.List;
+              
+              class Test {
+                  List<Integer> cardinalsWorldSeries = Collections.unmodifiableList(new ArrayList<>());
+              }
+              """
+          )
         );
     }
 }
diff --git a/src/test/java/com/yourorg/SimplifyTernaryTest.java b/src/test/java/com/yourorg/SimplifyTernaryTest.java
@@ -17,16 +17,24 @@
 
 import org.junit.jupiter.api.Test;
 import org.openrewrite.DocumentExample;
+import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
 
 import static org.openrewrite.java.Assertions.java;
 
+// This is a test for the SimplifyTernary recipe, as an example of how to write a test for a Refaster style recipe.
 class SimplifyTernaryTest implements RewriteTest {
-    @DocumentExample
+
+    @Override
+    public void defaults(RecipeSpec spec) {
+        // Note that we instantiate a generated class here, with `Recipes` appended to the Refaster class name
+        spec.recipe(new SimplifyTernaryRecipes());
+    }
+
     @Test
+    @DocumentExample
     void simplified() {
         rewriteRun(
-          spec -> spec.recipe(new SimplifyTernaryRecipes()),
           //language=java
           java(
             """
@@ -37,19 +45,19 @@ class Test {
                   boolean trueCondition4 = trueCondition1 && trueCondition2 ? true : false;
                   boolean trueCondition5 = !true ? false : true;
                   boolean trueCondition6 = !false ? true : false;
-                  
+              
                   boolean falseCondition1 = true ? false : true;
                   boolean falseCondition2 = !false ? false : true;
                   boolean falseCondition3 = booleanExpression() ? false : true;
                   boolean falseCondition4 = trueCondition1 && trueCondition2 ? false : true;
                   boolean falseCondition5 = !false ? false : true;
                   boolean falseCondition6 = !true ? true : false;
-                  
+              
                   boolean binary1 = booleanExpression() && booleanExpression() ? true : false;
                   boolean binary2 = booleanExpression() && booleanExpression() ? false : true;
                   boolean binary3 = booleanExpression() || booleanExpression() ? true : false;
                   boolean binary4 = booleanExpression() || booleanExpression() ? false : true;
-                  
+              
                   boolean booleanExpression() {
                     return true;
                   }
@@ -63,19 +71,19 @@ class Test {
                   boolean trueCondition4 = trueCondition1 && trueCondition2;
                   boolean trueCondition5 = true;
                   boolean trueCondition6 = true;
-                  
+              
                   boolean falseCondition1 = false;
                   boolean falseCondition2 = false;
                   boolean falseCondition3 = !booleanExpression();
                   boolean falseCondition4 = !(trueCondition1 && trueCondition2);
                   boolean falseCondition5 = false;
                   boolean falseCondition6 = false;
-                  
+              
                   boolean binary1 = booleanExpression() && booleanExpression();
                   boolean binary2 = !(booleanExpression() && booleanExpression());
                   boolean binary3 = booleanExpression() || booleanExpression();
                   boolean binary4 = !(booleanExpression() || booleanExpression());
-                  
+              
                   boolean booleanExpression() {
                     return true;
                   }
@@ -85,18 +93,18 @@ boolean booleanExpression() {
         );
     }
 
+    // It's good practice to also include a test that verifies that the recipe doesn't change anything when it shouldn't.
     @Test
     void unchanged() {
         rewriteRun(
-          spec -> spec.recipe(new SimplifyTernaryRecipes()),
           //language=java
           java(
             """
               class Test {
                   boolean unchanged1 = booleanExpression() ? booleanExpression() : !booleanExpression();
                   boolean unchanged2 = booleanExpression() ? true : !booleanExpression();
                   boolean unchanged3 = booleanExpression() ? booleanExpression() : false;
-                  
+              
                   boolean booleanExpression() {
                     return true;
                   }