Sort preconditions by uses-type and uses-method first (#122)

* Sort preconditions by uses-type and uses-method first * Expand test to proof within the categories, it is still ordered alphabetically * Expand test to proof single And/Or rules are not printed
openrewrite · Dec 20, 2024 · 98c4659 · 98c4659
1 parent d39ec4f
commit 98c4659
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 31 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
@@ -25,8 +25,12 @@
 import static org.openrewrite.java.template.internal.StringUtils.indent;
 
 public abstract class Precondition {
-    private static final Comparator<String> BY_USES_TYPE_FIRST = Comparator
-            .comparing((String s) -> !s.startsWith("new UsesType"))
+    private static final Comparator<String> BY_USES_TYPE_METHOD_FIRST = Comparator
+            .comparing((String s) -> {
+                if (s.startsWith("new UsesType")) return 0;
+                if (s.startsWith("new UsesMethod")) return 1;
+                return 2;
+            })
             .thenComparing(Comparator.naturalOrder());
 
     abstract boolean fitsInto(Precondition p);
@@ -194,7 +198,7 @@ private static String joinPreconditions(Collection<Precondition> rules, String o
         } else if (rules.size() == 1) {
             return rules.iterator().next().toString();
         }
-        String preconditions = rules.stream().map(Object::toString).sorted(BY_USES_TYPE_FIRST).collect(joining(",\n"));
+        String preconditions = rules.stream().map(Object::toString).sorted(BY_USES_TYPE_METHOD_FIRST).collect(joining(",\n"));
         return "Preconditions." + op + "(\n" +
                 indent(preconditions, 4) + "\n" +
                 ")";

diff --git a/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java b/src/test/java/org/openrewrite/java/template/processor/PreconditionTest.java
@@ -29,31 +29,29 @@
 class PreconditionTest {
     @Test
     void toStringWithInden() {
-        String result = new Or(
-          new And(
-            new Or(new Rule("A"), new Rule("B")),
-            new Or(new Rule("C"), new Rule("D"))
+        String result = new And(
+          new And(new Rule("new UsesMethod<>(\"java.lang.String valueOf(..)\", true)")),
+          new Or(
+            new And(new Rule("new UsesMethod<>(\"java.util.HashMap <constructor>(..)\", true)"), new Rule("new UsesType<>(\"java.util.HashMap\", true)")),
+            new Or(new Rule("new UsesType<>(\"java.util.LinkedHashMap\", true)"))
           ),
-          new And(new Rule("X"), new Rule("Y"), new Rule("Z"))
+          new Rule("new UsesType<>(\"java.util.Map\", true)"),
+          new Rule("new UsesType<>(\"java.util.List\", true)")
         ).toString();
 
-        assertThat(result).isEqualTo("Preconditions.or(\n" +
-          "    Preconditions.and(\n" +
-          "        Preconditions.or(\n" +
-          "            A,\n" +
-          "            B\n" +
-          "        ),\n" +
-          "        Preconditions.or(\n" +
-          "            C,\n" +
-          "            D\n" +
-          "        )\n" +
-          "    ),\n" +
-          "    Preconditions.and(\n" +
-          "        X,\n" +
-          "        Y,\n" +
-          "        Z\n" +
-          "    )\n" +
-          ")");
+        assertThat(result).isEqualTo(
+          "Preconditions.and(\n" +
+            "    new UsesType<>(\"java.util.List\", true),\n" +
+            "    new UsesType<>(\"java.util.Map\", true),\n" +
+            "    new UsesMethod<>(\"java.lang.String valueOf(..)\", true),\n" +
+            "    Preconditions.or(\n" +
+            "        new UsesType<>(\"java.util.LinkedHashMap\", true),\n" +
+            "        Preconditions.and(\n" +
+            "            new UsesType<>(\"java.util.HashMap\", true),\n" +
+            "            new UsesMethod<>(\"java.util.HashMap <constructor>(..)\", true)\n" +
+            "        )\n" +
+            "    )\n" +
+            ")");
     }
 
     @Test

diff --git a/src/test/resources/refaster/PreconditionsVerifierRecipes.java b/src/test/resources/refaster/PreconditionsVerifierRecipes.java
@@ -197,11 +197,11 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) {
             };
             return Preconditions.check(
                     Preconditions.or(
+                            new UsesMethod<>("java.lang.String valueOf(..)", true),
                             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
             );
@@ -546,11 +546,11 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) {
             };
             return Preconditions.check(
                     Preconditions.and(
+                            new UsesMethod<>("java.io.PrintStream println(..)", true),
                             Preconditions.or(
                                     new UsesType<>("java.util.List", true),
                                     new UsesType<>("java.util.Map", true)
-                            ),
-                            new UsesMethod<>("java.io.PrintStream println(..)", true)
+                            )
                     ),
                     javaVisitor
             );

diff --git a/src/test/resources/refaster/ShouldAddImportsRecipes.java b/src/test/resources/refaster/ShouldAddImportsRecipes.java
@@ -190,11 +190,11 @@ public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) {
             };
             return Preconditions.check(
                     Preconditions.or(
+                            new UsesMethod<>("java.lang.Integer compare(..)", true),
                             Preconditions.and(
                                     new UsesType<>("java.util.Objects", true),
                                     new UsesMethod<>("java.util.Objects equals(..)", true)
-                            ),
-                            new UsesMethod<>("java.lang.Integer compare(..)", true)
+                            )
                     ),
                     javaVisitor
             );