OpenRewrite recipe best practices (#1074)

timtebeek · TeamModerne · web-flow · commit c69eb618ae13 · 2026-05-01T15:12:16.000+02:00
* OpenRewrite recipe best practices Use this link to re-run the recipe: https://app.moderne.io/recipes/org.openrewrite.recipes.rewrite.OpenRewriteRecipeBestPractices?organizationId=QUxML01vZGVybmUvTW9kZXJuZSArIE9wZW5SZXdyaXRl Co-authored-by: Moderne <team@moderne.io> * Use Java 25 toolchain Allows the unnamed variable `_` produced by the OpenRewrite best-practices recipe to compile without preview flags. * Revert "Use Java 25 toolchain" This reverts commit 16e93cc. * Avoid unnamed lambda parameter in UpgradeFacesOpenSourceLibrariesTest The recipe rewrote `(a, b) -> b` to `(_, b) -> b`, but the build's Java 21 toolchain rejects unnamed variables (a Java 22 feature) without --enable-preview. Restore the named parameter so compileTestJava succeeds on Java 21. --------- Co-authored-by: Moderne <team@moderne.io>
diff --git a/src/test/java/org/openrewrite/java/migrate/AddSurefireFailsafeArgLineTest.java b/src/test/java/org/openrewrite/java/migrate/AddSurefireFailsafeArgLineTest.java
@@ -25,9 +25,10 @@
 
 class AddSurefireFailsafeArgLineTest implements RewriteTest {
 
-    private static final String ARG_LINE = "--add-opens java.base/java.lang=ALL-UNNAMED" +
-            " --add-opens java.base/java.util=ALL-UNNAMED" +
-            " --add-opens java.base/java.lang.reflect=ALL-UNNAMED";
+    private static final String ARG_LINE = """
+            --add-opens java.base/java.lang=ALL-UNNAMED\
+             --add-opens java.base/java.util=ALL-UNNAMED\
+             --add-opens java.base/java.lang.reflect=ALL-UNNAMED""";
 
     @Override
     public void defaults(RecipeSpec spec) {
diff --git a/src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test.java b/src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test.java
@@ -21,7 +21,6 @@
 import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
 
-import java.util.Optional;
 import java.util.regex.Pattern;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -150,7 +149,7 @@ public String test() {
                     }
                     """,
                   spec -> spec.afterRecipe(cu -> {
-                      Optional<JavaVersion> jv = cu.getMarkers().findFirst(JavaVersion.class);
+                      var jv = cu.getMarkers().findFirst(JavaVersion.class);
                       assertThat(jv)
                         .map(JavaVersion::getSourceCompatibility)
                         .hasValue("17");
diff --git a/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxAnnotationMigrationToJakartaAnnotationTest.java b/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxAnnotationMigrationToJakartaAnnotationTest.java
@@ -130,9 +130,10 @@ class A {
                 """,
               spec -> spec.after(pom -> assertThat(pom)
                     .containsPattern(
-                      "<groupId>jakarta\\.annotation</groupId>\\s*" +
-                      "<artifactId>jakarta\\.annotation-api</artifactId>\\s*" +
-                      "<version>2\\.0\\.\\d+</version>")
+                      """
+                      <groupId>jakarta\\.annotation</groupId>\\s*\
+                      <artifactId>jakarta\\.annotation-api</artifactId>\\s*\
+                      <version>2\\.0\\.\\d+</version>""")
                     .actual())
             )
           )
diff --git a/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxInjectToJakartaInjectTest.java b/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxInjectToJakartaInjectTest.java
@@ -112,9 +112,10 @@ public class TestApplication {
               spec -> spec.after(pom -> {
                   return assertThat( pom )
                           .containsPattern(
-                                  "<groupId>jakarta\\.inject</groupId>\\s*" +
-                                          "<artifactId>jakarta\\.inject-api</artifactId>\\s*" +
-                                          "<version>2\\.0\\.\\d+</version>" ).actual();
+                                  """
+                                  <groupId>jakarta\\.inject</groupId>\\s*\
+                                  <artifactId>jakarta\\.inject-api</artifactId>\\s*\
+                                  <version>2\\.0\\.\\d+</version>""" ).actual();
               })
             )
           )
diff --git a/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxJspToJakartaJspTest.java b/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxJspToJakartaJspTest.java
@@ -65,9 +65,10 @@ void switchesJavaxJspApiDependencyToJakartaJspApiDependency() {
                   return assertThat( pom )
                           .doesNotContain( "javax.servlet.jsp" )
                           .containsPattern(
-                                  "<groupId>jakarta\\.servlet\\.jsp</groupId>\\s*" +
-                                          "<artifactId>jakarta\\.servlet\\.jsp-api</artifactId>\\s*" +
-                                          "<version>3\\.0\\.\\d+</version>" ).actual();
+                                  """
+                                  <groupId>jakarta\\.servlet\\.jsp</groupId>\\s*\
+                                  <artifactId>jakarta\\.servlet\\.jsp-api</artifactId>\\s*\
+                                  <version>3\\.0\\.\\d+</version>""" ).actual();
               })
             )
           ),
@@ -129,9 +130,10 @@ public class TestApplication {
               spec -> spec.after(pom -> {
                   return assertThat( pom )
                           .containsPattern(
-                                  "<groupId>jakarta\\.servlet\\.jsp</groupId>\\s*" +
-                                          "<artifactId>jakarta\\.servlet\\.jsp-api</artifactId>\\s*" +
-                                          "<version>3\\.0\\.\\d+</version>" ).actual();
+                                  """
+                                  <groupId>jakarta\\.servlet\\.jsp</groupId>\\s*\
+                                  <artifactId>jakarta\\.servlet\\.jsp-api</artifactId>\\s*\
+                                  <version>3\\.0\\.\\d+</version>""" ).actual();
               })
             )
           )
@@ -163,9 +165,10 @@ void upgradesJakartaJspApiDependencyIfAlreadyExistingAtLowerVersion() {
               spec -> spec.after(pom -> {
                   return assertThat( pom )
                           .containsPattern(
-                                  "<groupId>jakarta\\.servlet\\.jsp</groupId>\\s*" +
-                                          "<artifactId>jakarta\\.servlet\\.jsp-api</artifactId>\\s*" +
-                                          "<version>3\\.0\\.\\d+</version>" ).actual();
+                                  """
+                                  <groupId>jakarta\\.servlet\\.jsp</groupId>\\s*\
+                                  <artifactId>jakarta\\.servlet\\.jsp-api</artifactId>\\s*\
+                                  <version>3\\.0\\.\\d+</version>""" ).actual();
               })
             )
           )
diff --git a/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxPersistenceToJakartaPersistenceTest.java b/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxPersistenceToJakartaPersistenceTest.java
@@ -63,9 +63,10 @@ void switchesJavaxPersistenceApiDependencyToJakartaPersistenceApiDependency() {
                   return assertThat( pom )
                           .doesNotContain( "javax.persistence" )
                           .containsPattern(
-                                  "<groupId>jakarta\\.persistence</groupId>\\s*" +
-                                          "<artifactId>jakarta\\.persistence-api</artifactId>\\s*" +
-                                          "<version>3\\.0\\.\\d+</version>" ).actual();
+                                  """
+                                  <groupId>jakarta\\.persistence</groupId>\\s*\
+                                  <artifactId>jakarta\\.persistence-api</artifactId>\\s*\
+                                  <version>3\\.0\\.\\d+</version>""" ).actual();
               })
             )
           ),
@@ -127,9 +128,10 @@ public class TestApplication {
               spec -> spec.after(pom -> {
                   return assertThat( pom )
                           .containsPattern(
-                                  "<groupId>jakarta\\.persistence</groupId>\\s*" +
-                                          "<artifactId>jakarta\\.persistence-api</artifactId>\\s*" +
-                                          "<version>3\\.0\\.\\d+</version>" ).actual();
+                                  """
+                                  <groupId>jakarta\\.persistence</groupId>\\s*\
+                                  <artifactId>jakarta\\.persistence-api</artifactId>\\s*\
+                                  <version>3\\.0\\.\\d+</version>""" ).actual();
               })
             )
           )
diff --git a/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxTransactionMigrationToJakartaTransactionTest.java b/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxTransactionMigrationToJakartaTransactionTest.java
@@ -150,9 +150,10 @@ public class TestApplication {
               spec -> spec.after(pom -> {
                   return assertThat( pom )
                           .containsPattern(
-                                  "<groupId>jakarta\\.transaction</groupId>\\s*" +
-                                          "<artifactId>jakarta\\.transaction-api</artifactId>\\s*" +
-                                          "<version>2\\.0\\.\\d+</version>" ).actual();
+                                  """
+                                  <groupId>jakarta\\.transaction</groupId>\\s*\
+                                  <artifactId>jakarta\\.transaction-api</artifactId>\\s*\
+                                  <version>2\\.0\\.\\d+</version>""" ).actual();
               })
             )
           )
diff --git a/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxValidationMigrationToJakartaValidationTest.java b/src/test/java/org/openrewrite/java/migrate/jakarta/JavaxValidationMigrationToJakartaValidationTest.java
@@ -201,9 +201,10 @@ public class TestApplication {
               spec -> spec.after(pom -> {
                   return assertThat( pom )
                           .containsPattern(
-                                  "<groupId>jakarta\\.validation</groupId>\\s*" +
-                                          "<artifactId>jakarta\\.validation-api</artifactId>\\s*" +
-                                          "<version>3\\.0\\.\\d+</version>" ).actual();
+                                  """
+                                  <groupId>jakarta\\.validation</groupId>\\s*\
+                                  <artifactId>jakarta\\.validation-api</artifactId>\\s*\
+                                  <version>3\\.0\\.\\d+</version>""" ).actual();
               })
             )
           )
diff --git a/src/test/java/org/openrewrite/java/migrate/lang/RenameUnderscoreIdentifierTest.java b/src/test/java/org/openrewrite/java/migrate/lang/RenameUnderscoreIdentifierTest.java
@@ -28,11 +28,9 @@
 
 class RenameUnderscoreIdentifierTest implements RewriteTest {
 
-    /**
-     * Setup recipe that renames {@code UNDERSCORE} to {@code _} in the LST,
-     * simulating Java 8 source code that uses {@code _} as an identifier.
-     * This is necessary because the Java 9+ parser cannot parse {@code _} as a regular identifier.
-     */
+    /// Setup recipe that renames `UNDERSCORE` to `_` in the LST,
+    /// simulating Java 8 source code that uses `_` as an identifier.
+    /// This is necessary because the Java 9+ parser cannot parse `_` as a regular identifier.
     private static Recipe renameToUnderscore() {
         return new Recipe() {
             @Override
diff --git a/src/test/java/org/openrewrite/java/migrate/lang/UseTextBlocksTest.java b/src/test/java/org/openrewrite/java/migrate/lang/UseTextBlocksTest.java
@@ -25,11 +25,11 @@
 import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
 
+import java.util.List;
+import java.util.Set;
 import java.util.function.Consumer;
 import java.util.function.UnaryOperator;
 
-import static java.util.Collections.emptySet;
-import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.openrewrite.Tree.randomId;
 import static org.openrewrite.java.Assertions.java;
@@ -489,10 +489,10 @@ class Test {
     }
 
     private static Consumer<RecipeSpec> tabsAndIndents(UnaryOperator<TabsAndIndentsStyle> with, int tabSize) {
-        return spec -> spec.parser(JavaParser.fromJavaVersion().styles(singletonList(
+        return spec -> spec.parser(JavaParser.fromJavaVersion().styles(List.of(
           new NamedStyles(
-            randomId(), "TabsOnlyFile", "TabsOnlyFile", "tabSize is x", emptySet(),
-            singletonList(with.apply(buildTabsAndIndents(tabSize)))
+            randomId(), "TabsOnlyFile", "TabsOnlyFile", "tabSize is x", Set.of(),
+            List.of(with.apply(buildTabsAndIndents(tabSize)))
           )
         )));
     }
@@ -648,11 +648,9 @@ class Test {
         );
     }
 
-    /**
-     * Single escaping a quote in a string literal provides: " -> \"
-     * <p>
-     * On converting this to a text block, we can let go of the escaping for the double quote: \" -> "
-     */
+    /// Single escaping a quote in a string literal provides: " -> \"
+    ///
+    /// On converting this to a text block, we can let go of the escaping for the double quote: \" -> "
     @Test
     void singleEscapedQuote() {
         rewriteRun(
@@ -688,12 +686,10 @@ class Test {
         );
     }
 
-    /**
-     * Double escaping a quote in a string literal provides: " -> \" -> \\\"
-     * <p>
-     * On converting this to a text block, the escaped backslash should remain, but we can let go of the
-     * escaping for the double quote: \\\" -> \\"
-     */
+    /// Double escaping a quote in a string literal provides: " -> \" -> \\\"
+    ///
+    /// On converting this to a text block, the escaped backslash should remain, but we can let go of the
+    /// escaping for the double quote: \\\" -> \\"
     @Test
     void doubleEscapedQuote() {
         rewriteRun(
@@ -729,11 +725,9 @@ class Test {
         );
     }
 
-    /**
-     * Triple quotes in a string literal are escaped as: """ -> \"\"\"
-     * <p>
-     * On converting this to a text block, only one of the quotes needs to be escaped: \"\"\" -> ""\"
-     */
+    /// Triple quotes in a string literal are escaped as: """ -> \"\"\"
+    ///
+    /// On converting this to a text block, only one of the quotes needs to be escaped: \"\"\" -> ""\"
     @Test
     void tripleQuotes() {
         rewriteRun(
@@ -795,21 +789,19 @@ class Test {
         );
     }
 
-    /**
-     * Quote in a string literal is escaped as: " -> \"
-     * <p>
-     * On converting this to a text block, it needs to be escaped if it is the last character:
-     * <pre>
-     *             """
-     * "test\"" -> test\""""
-     * </pre>
-     * However, this is not required in case we use the newline escape (\) to put the ending delimiter on the next line:
-     * <pre>
-     *             """
-     * "test\"" -> test"\
-     *             """
-     * </pre>
-     */
+    /// Quote in a string literal is escaped as: " -> \"
+    ///
+    /// On converting this to a text block, it needs to be escaped if it is the last character:
+    /// ```
+    ///             """
+    /// "test\"" -> test\""""
+    /// ```
+    /// However, this is not required in case we use the newline escape (\) to put the ending delimiter on the next line:
+    /// ```
+    ///             """
+    /// "test\"" -> test"\
+    ///             """
+    /// ```
     @Test
     void endingQuote() {
         rewriteRun(
diff --git a/src/test/java/org/openrewrite/java/migrate/lombok/AdoptLombokGetterMethodNamesTest.java b/src/test/java/org/openrewrite/java/migrate/lombok/AdoptLombokGetterMethodNamesTest.java
@@ -92,10 +92,8 @@ class A {
         );
     }
 
-    /**
-     * Verifies that the correct method name is chosen for a boolean field with an 'is' prefix.
-     * The corresponding method name in this case should not be `isIsFoo` but just `isFoo`.
-     */
+    /// Verifies that the correct method name is chosen for a boolean field with an 'is' prefix.
+    /// The corresponding method name in this case should not be `isIsFoo` but just `isFoo`.
     @Test
     void renamePrimitiveBooleanWithPrefixInSingleClass() {
         rewriteRun(// language=java
@@ -136,11 +134,9 @@ class A {
         );
     }
 
-    /**
-     * Verifies that the correct method name is chosen for a boolean field with an 'is' prefix.
-     * The corresponding method name in this case should not be `isFoo` as in the primitive case,
-     * but prefix a `get` as for any other field.
-     */
+    /// Verifies that the correct method name is chosen for a boolean field with an 'is' prefix.
+    /// The corresponding method name in this case should not be `isFoo` as in the primitive case,
+    /// but prefix a `get` as for any other field.
     @Test
     void renameClassBooleanWithPrefixInSingleClass() {
         rewriteRun(// language=java
@@ -275,9 +271,7 @@ public long getFoo() {
         );
     }
 
-    /**
-     * Methods in inner classes should be renamed as well.
-     */
+    /// Methods in inner classes should be renamed as well.
     @Test
     void shouldWorkOnInnerClasses() {
         rewriteRun(// language=java
@@ -346,9 +340,7 @@ public long getFoo() {
         );
     }
 
-    /**
-     * Methods on top level should be renamed just as well when there is an inner class.
-     */
+    /// Methods on top level should be renamed just as well when there is an inner class.
     @ExpectedToFail("We use `cu.getTypesInUse().getDeclaredMethods()` as a performance optimization to avoid visits")
     @Test
     void shouldWorkDespiteInnerClassesSameNameMethods() {
@@ -396,9 +388,7 @@ public long getFoo() {
         );
     }
 
-    /**
-     * Methods on top level should be renamed just as well when there is an inner class.
-     */
+    /// Methods on top level should be renamed just as well when there is an inner class.
     @Test
     void shouldWorkDespiteInnerClassesDifferentNameMethods() {
         rewriteRun(// language=java
@@ -445,10 +435,8 @@ public long getBa() {
         );
     }
 
-    /**
-     * If existing method names need to be rotated in a loop the recipe should still work.
-     * For now this is not planned.
-     */
+    /// If existing method names need to be rotated in a loop the recipe should still work.
+    /// For now this is not planned.
     @ExpectedToFail("Not implemented yet")
     @Test
     void shouldWorkOnCircleCasesButDoesntYet() {
@@ -490,13 +478,11 @@ public int getBar() {
         );
     }
 
-    /**
-     * If two methods are effectively the same getter then only one can be renamed.
-     * Renaming both would result in a duplicate method definition, so we cannot do this.
-     * Ideally the other effective getter would have their usages renamed but be themselves deleted...
-     * TODO: create a second cleanup recipe that identifies redundant getters (isEffectiveGetter + field already has the getter annotation)
-     *  and redirects their usage (ChangeMethodName with both flags true) and then deletes them.
-     */
+    /// If two methods are effectively the same getter then only one can be renamed.
+    /// Renaming both would result in a duplicate method definition, so we cannot do this.
+    /// Ideally the other effective getter would have their usages renamed but be themselves deleted...
+    /// TODO: create a second cleanup recipe that identifies redundant getters (isEffectiveGetter + field already has the getter annotation)
+    ///  and redirects their usage (ChangeMethodName with both flags true) and then deletes them.
     @Test
     void twoMethodsWithSameName() {
         rewriteRun(// language=java
diff --git a/src/test/java/org/openrewrite/java/migrate/lombok/AdoptLombokSetterMethodNamesTest.java b/src/test/java/org/openrewrite/java/migrate/lombok/AdoptLombokSetterMethodNamesTest.java

Original file line number	Diff line number	Diff line change
`@@ -130,9 +130,10 @@ class A {`
`130`	`130`	`""",`
`131`	`131`	`spec -> spec.after(pom -> assertThat(pom)`
`132`	`132`	`.containsPattern(`
`133`		`- "<groupId>jakarta\\.annotation</groupId>\\s*" +`
`134`		`- "<artifactId>jakarta\\.annotation-api</artifactId>\\s*" +`
`135`		`- "<version>2\\.0\\.\\d+</version>")`
	`133`	`+ """`
	`134`	`+ <groupId>jakarta\\.annotation</groupId>\\s*\`
	`135`	`+ <artifactId>jakarta\\.annotation-api</artifactId>\\s*\`
	`136`	`+ <version>2\\.0\\.\\d+</version>""")`
`136`	`137`	`.actual())`
`137`	`138`	`)`
`138`	`139`	`)`
Original file line number	Diff line number	Diff line change
`@@ -112,9 +112,10 @@ public class TestApplication {`
`112`	`112`	`spec -> spec.after(pom -> {`
`113`	`113`	`return assertThat( pom )`
`114`	`114`	`.containsPattern(`
`115`		`- "<groupId>jakarta\\.inject</groupId>\\s*" +`
`116`		`- "<artifactId>jakarta\\.inject-api</artifactId>\\s*" +`
`117`		`- "<version>2\\.0\\.\\d+</version>" ).actual();`
	`115`	`+ """`
	`116`	`+ <groupId>jakarta\\.inject</groupId>\\s*\`
	`117`	`+ <artifactId>jakarta\\.inject-api</artifactId>\\s*\`
	`118`	`+ <version>2\\.0\\.\\d+</version>""" ).actual();`
`118`	`119`	`})`
`119`	`120`	`)`
`120`	`121`	`)`
Original file line number	Diff line number	Diff line change
`@@ -150,9 +150,10 @@ public class TestApplication {`
`150`	`150`	`spec -> spec.after(pom -> {`
`151`	`151`	`return assertThat( pom )`
`152`	`152`	`.containsPattern(`
`153`		`- "<groupId>jakarta\\.transaction</groupId>\\s*" +`
`154`		`- "<artifactId>jakarta\\.transaction-api</artifactId>\\s*" +`
`155`		`- "<version>2\\.0\\.\\d+</version>" ).actual();`
	`153`	`+ """`
	`154`	`+ <groupId>jakarta\\.transaction</groupId>\\s*\`
	`155`	`+ <artifactId>jakarta\\.transaction-api</artifactId>\\s*\`
	`156`	`+ <version>2\\.0\\.\\d+</version>""" ).actual();`
`156`	`157`	`})`
`157`	`158`	`)`
`158`	`159`	`)`
Original file line number	Diff line number	Diff line change
`@@ -201,9 +201,10 @@ public class TestApplication {`
`201`	`201`	`spec -> spec.after(pom -> {`
`202`	`202`	`return assertThat( pom )`
`203`	`203`	`.containsPattern(`
`204`		`- "<groupId>jakarta\\.validation</groupId>\\s*" +`
`205`		`- "<artifactId>jakarta\\.validation-api</artifactId>\\s*" +`
`206`		`- "<version>3\\.0\\.\\d+</version>" ).actual();`
	`204`	`+ """`
	`205`	`+ <groupId>jakarta\\.validation</groupId>\\s*\`
	`206`	`+ <artifactId>jakarta\\.validation-api</artifactId>\\s*\`
	`207`	`+ <version>3\\.0\\.\\d+</version>""" ).actual();`
`207`	`208`	`})`
`208`	`209`	`)`
`209`	`210`	`)`