Let UsesMethod add its own marker (#4035)

* `HasJavaVersion` combined with `FindMethods` depends on order `org.openrewrite.java.search.HasJavaVersion` combined with `org.openrewrite.java.search.FindMethods` fails to find results for FindMethods if that recipe is running after `HasJavaVersion`. * Add `UsesMethod.MethodMatch` marker * Override `Marker#print()` * Override `Marker#print()` * Also add `SearchResult` marker --------- Co-authored-by: Knut Wannheden <[email protected]>
openrewrite · Feb 23, 2024 · 0ee95e4 · 0ee95e4
1 parent 7acd005
commit 0ee95e4
Show file tree

Hide file tree

Showing 5 changed files with 124 additions and 9 deletions.
diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/search/HasJavaVersionTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/search/HasJavaVersionTest.java
@@ -96,4 +96,79 @@ void declarativePreconditionMatch() {
         );
     }
 
+    @Test
+    void combinedWithFindMethod() {
+        rewriteRun(
+          spec -> spec.recipeFromYaml("""
+            ---
+            type: specs.openrewrite.org/v1beta/recipe
+            name: org.openrewrite.CombinedWithFindMethod
+            recipeList:
+              - org.openrewrite.java.search.HasJavaVersion:
+                  version: 11
+              - org.openrewrite.java.search.FindMethods:
+                  methodPattern: java.util.List add(..)
+            """, "org.openrewrite.CombinedWithFindMethod"),
+          java(
+            """
+              import java.util.List;
+              import java.util.ArrayList;
+              class Test {
+                  void test() {
+                      List<String> list = new ArrayList<>();
+                      list.add("1");
+                  }
+              }
+              """,
+            """
+              /*~~>*/import java.util.List;
+              import java.util.ArrayList;
+              class Test {
+                  void test() {
+                      List<String> list = new ArrayList<>();
+                      /*~~>*/list.add("1");
+                  }
+              }
+              """, spec -> spec.markers(javaVersion(11)))
+        );
+    }
+
+    @Test
+    void combinedWithFindMethodFirst() {
+        rewriteRun(
+          spec -> spec.recipeFromYaml("""
+            ---
+            type: specs.openrewrite.org/v1beta/recipe
+            name: org.openrewrite.CombinedWithFindMethod
+            recipeList:
+              - org.openrewrite.java.search.FindMethods:
+                  methodPattern: java.util.List add(..)
+              - org.openrewrite.java.search.HasJavaVersion:
+                  version: 11
+
+            """, "org.openrewrite.CombinedWithFindMethod"),
+          java(
+            """
+              import java.util.List;
+              import java.util.ArrayList;
+              class Test {
+                  void test() {
+                      List<String> list = new ArrayList<>();
+                      list.add("1");
+                  }
+              }
+              """,
+            """
+              /*~~>*/import java.util.List;
+              import java.util.ArrayList;
+              class Test {
+                  void test() {
+                      List<String> list = new ArrayList<>();
+                      /*~~>*/list.add("1");
+                  }
+              }
+              """, spec -> spec.markers(javaVersion(11)))
+        );
+    }
+
 }
diff --git a/rewrite-java/src/main/java/org/openrewrite/java/MethodMatcher.java b/rewrite-java/src/main/java/org/openrewrite/java/MethodMatcher.java
@@ -379,6 +379,15 @@ public static String methodPattern(JavaType.Method method) {
         return typePattern(method.getDeclaringType()) + " " +
                method.getName() + "(" + parameters + ")";
     }
+
+    @Override
+    public String toString() {
+        //noinspection DataFlowIssue
+        return (targetType != null ? targetType : targetTypePattern.pattern()) +
+               ' ' +
+               (methodName != null ? methodName : methodNamePattern.pattern()) +
+               '(' + argumentPattern.pattern() + ')';
+    }
 }
 
 class TypeVisitor extends MethodSignatureParserBaseVisitor<String> {

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/UseStaticImport.java b/rewrite-java/src/main/java/org/openrewrite/java/UseStaticImport.java
@@ -65,9 +65,10 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
     }
 
     private class UseStaticImportVisitor extends JavaIsoVisitor<ExecutionContext> {
+        final MethodMatcher methodMatcher = new MethodMatcher(methodPattern);
+
         @Override
         public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
-            MethodMatcher methodMatcher = new MethodMatcher(methodPattern);
             J.MethodInvocation m = super.visitMethodInvocation(method, ctx);
             if (methodMatcher.matches(m)) {
                 if (m.getTypeParameters() != null && !m.getTypeParameters().isEmpty()) {

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/search/FindMethods.java b/rewrite-java/src/main/java/org/openrewrite/java/search/FindMethods.java
@@ -67,8 +67,9 @@ public String getDescription() {
     @Override
     @SuppressWarnings("ConstantConditions")
     public TreeVisitor<?, ExecutionContext> getVisitor() {
-        MethodMatcher methodMatcher = new MethodMatcher(methodPattern, matchOverrides);
         return Preconditions.check(new UsesMethod<>(methodPattern, matchOverrides), new JavaIsoVisitor<ExecutionContext>() {
+            final MethodMatcher methodMatcher = new MethodMatcher(methodPattern, matchOverrides);
+
             @Override
             public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
                 J.MethodInvocation m = super.visitMethodInvocation(method, ctx);

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/search/UsesMethod.java b/rewrite-java/src/main/java/org/openrewrite/java/search/UsesMethod.java
@@ -15,32 +15,46 @@
  */
 package org.openrewrite.java.search;
 
+import lombok.EqualsAndHashCode;
+import lombok.Value;
+import lombok.With;
 import org.openrewrite.Tree;
 import org.openrewrite.internal.lang.Nullable;
 import org.openrewrite.java.JavaIsoVisitor;
 import org.openrewrite.java.MethodMatcher;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.JavaSourceFile;
 import org.openrewrite.java.tree.JavaType;
+import org.openrewrite.marker.Marker;
 import org.openrewrite.marker.SearchResult;
 
+import java.util.UUID;
+
+import static org.openrewrite.Tree.randomId;
+
 public class UsesMethod<P> extends JavaIsoVisitor<P> {
+    private final String methodPattern;
     private final MethodMatcher methodMatcher;
 
     public UsesMethod(String methodPattern) {
-        this(new MethodMatcher(methodPattern));
+        this(new MethodMatcher(methodPattern), methodPattern);
     }
 
     public UsesMethod(String methodPattern, boolean matchOverrides) {
-        this(new MethodMatcher(methodPattern, matchOverrides));
+        this(new MethodMatcher(methodPattern, matchOverrides), methodPattern);
     }
 
     public UsesMethod(String methodPattern, @Nullable Boolean matchOverrides) {
-        this(new MethodMatcher(methodPattern, Boolean.TRUE.equals(matchOverrides)));
+        this(new MethodMatcher(methodPattern, Boolean.TRUE.equals(matchOverrides)), methodPattern);
     }
 
     public UsesMethod(MethodMatcher methodMatcher) {
+        this(methodMatcher, methodMatcher.toString());
+    }
+
+    private UsesMethod(MethodMatcher methodMatcher, String methodPattern) {
         this.methodMatcher = methodMatcher;
+        this.methodPattern = methodPattern;
     }
 
     @Override
@@ -49,35 +63,50 @@ public J visit(@Nullable Tree tree, P p) {
             JavaSourceFile cu = (JavaSourceFile) tree;
             for (JavaType.Method type : cu.getTypesInUse().getUsedMethods()) {
                 if (methodMatcher.matches(type)) {
-                    return SearchResult.found(cu);
+                    return found(cu);
                 }
             }
             return (J) tree;
         }
         return super.visit(tree, p);
     }
 
+    private <J2 extends J> J2 found(J2 j) {
+        // also adding a `SearchResult` marker to get a visible diff
+        return SearchResult.found(j.withMarkers(j.getMarkers()
+                .compute(new MethodMatch(randomId(), methodPattern), (s1, s2) -> s1 == null ? s2 : s1)));
+    }
+
     @Override
     public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, P p) {
         if (methodMatcher.matches(method)) {
-            return SearchResult.found(method);
+            return found(method);
         }
         return super.visitMethodInvocation(method, p);
     }
 
     @Override
     public J.MemberReference visitMemberReference(J.MemberReference memberRef, P p) {
         if (methodMatcher.matches(memberRef)) {
-            return SearchResult.found(memberRef);
+            return found(memberRef);
         }
         return super.visitMemberReference(memberRef, p);
     }
 
     @Override
     public J.NewClass visitNewClass(J.NewClass newClass, P p) {
         if (methodMatcher.matches(newClass)) {
-            return SearchResult.found(newClass);
+            return found(newClass);
         }
         return super.visitNewClass(newClass, p);
     }
+
+    @Value
+    @With
+    @EqualsAndHashCode(onlyExplicitlyIncluded = true)
+    public static class MethodMatch implements Marker {
+        UUID id;
+        @EqualsAndHashCode.Include
+        String methodMatcher;
+    }
 }