Fix a false positive of LambdaBlockToExpression recipe to not rewrite…

… when there are ambiguous method overloading
openrewrite · Aug 24, 2023 · 3c3fded · 3c3fded · kunli2 · Aug 24, 2023
1 parent e1cfd80
commit 3c3fded
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 32 deletions.
diff --git a/src/main/java/org/openrewrite/staticanalysis/LambdaBlockToExpression.java b/src/main/java/org/openrewrite/staticanalysis/LambdaBlockToExpression.java
@@ -20,12 +20,11 @@
 import org.openrewrite.Recipe;
 import org.openrewrite.TreeVisitor;
 import org.openrewrite.java.JavaIsoVisitor;
-import org.openrewrite.java.tree.J;
-import org.openrewrite.java.tree.Space;
-import org.openrewrite.java.tree.Statement;
-import org.openrewrite.marker.SearchResult;
+import org.openrewrite.java.tree.*;
+import org.openrewrite.staticanalysis.java.JavaFileChecker;
 
 import java.util.List;
+import java.util.Optional;
 
 public class LambdaBlockToExpression extends Recipe {
     @Override
@@ -40,13 +39,7 @@ public String getDescription() {
 
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
-        return Preconditions.check(
-                new JavaIsoVisitor<ExecutionContext>() {
-                    @Override
-                    public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext executionContext) {
-                        return SearchResult.found(cu);
-                    }
-                },
+        return Preconditions.check(new JavaFileChecker<>(),
                 new JavaIsoVisitor<ExecutionContext>() {
                     @Override
                     public J.Lambda visitLambda(J.Lambda lambda, ExecutionContext executionContext) {
@@ -64,7 +57,50 @@ public J.Lambda visitLambda(J.Lambda lambda, ExecutionContext executionContext)
                         }
                         return l;
                     }
+
+                    @Override
+                    public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
+                        if (hasLambdaArgument(method) && hasMethodOverloading(method)) {
+                            return method;
+                        }
+                        return super.visitMethodInvocation(method, executionContext);
+                    }
                 }
         );
     }
+
+    // Check whether a method has overloading methods in the declaring class
+    private static boolean hasMethodOverloading(J.MethodInvocation method) {
+        String methodName = method.getSimpleName();
+        return Optional.ofNullable(method.getMethodType())
+                .map(JavaType.Method::getDeclaringType)
+                .filter(JavaType.Class.class::isInstance)
+                .map(JavaType.Class.class::cast)
+                .map(JavaType.Class::getMethods)
+                .map(methods -> {
+                    int overloadingCount = 0;
+                    for (JavaType.Method dm : methods) {
+                        if (dm.getName().equals(methodName)) {
+                            overloadingCount++;
+                            if (overloadingCount > 1) {
+
+                                return true;
+                            }
+                        }
+                    }
+                    return false;
+                })
+                .orElse(false);
+    }
+
+    private static boolean hasLambdaArgument(J.MethodInvocation method) {
+        boolean hasLambdaArgument = false;
+        for (Expression arg : method.getArguments()) {
+            if (arg instanceof J.Lambda) {
+                hasLambdaArgument = true;
+                break;
+            }
+        }
+        return hasLambdaArgument;
+    }
 }
diff --git a/src/test/java/org/openrewrite/staticanalysis/LambdaBlockToExpressionTest.java b/src/test/java/org/openrewrite/staticanalysis/LambdaBlockToExpressionTest.java
@@ -84,22 +84,21 @@ class Test {
         );
     }
 
-    @Test
     @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/162")
-    void simplifyLambdaBlockWithAmbiguousMethod() {
+    @Test
+    void noChangeIfLambdaBlockWithAmbiguousMethod() {
         //language=java
         rewriteRun(
-          java("""
-            import java.util.function.Function;
-            import java.util.function.Consumer;
-            class A {
-                void aMethod(Consumer<Integer> consumer){
-                }
-                
-                void aMethod(Function<Integer,String> function){
-                }
-            }
-            """),
+          java(
+            """
+              import java.util.function.Function;
+              import java.util.function.Consumer;
+              class A {
+                  void aMethod(Consumer<Integer> consumer) {}
+                  void aMethod(Function<Integer,String> function) {}
+              }
+              """
+          ),
           java(
             """
               class Test {
@@ -110,14 +109,6 @@ void doTest() {
                       });
                   }
               }
-              """,
-            """
-              class Test {
-                  void doTest() {
-                      A a = new A();
-                      a.aMethod(value -> value.toString());
-                  }
-              }
               """
           )
         );