fix: Enhanced UseCollectionInterfaces to prevent conversions when non interface methods are used

src/main/java/org/openrewrite/staticanalysis/UseCollectionInterfaces.java

@@ -24,12 +24,15 @@
 import org.openrewrite.marker.Markers;
 
 import java.time.Duration;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singleton;
+import static java.util.Collections.unmodifiableSet;
 import static java.util.Objects.requireNonNull;
 import static org.openrewrite.Tree.randomId;
 
@@ -57,6 +60,7 @@ public Duration getEstimatedEffortPerOccurrence() {
     }
 
     public static final Map<String, String> rspecRulesReplaceTypeMap = new HashMap<>();
+    public static final Map<String, Set<String>> nonInterfaceMethods = new HashMap<>();
 
     static {
         rspecRulesReplaceTypeMap.put("java.util.ArrayDeque", "java.util.Deque");
@@ -66,6 +70,7 @@ public Duration getEstimatedEffortPerOccurrence() {
         rspecRulesReplaceTypeMap.put("java.util.AbstractSequentialList", "java.util.List");
         rspecRulesReplaceTypeMap.put("java.util.ArrayList", "java.util.List");
         rspecRulesReplaceTypeMap.put("java.util.concurrent.CopyOnWriteArrayList", "java.util.List");
+        rspecRulesReplaceTypeMap.put("java.util.Vector", "java.util.List");
         // Map
         rspecRulesReplaceTypeMap.put("java.util.AbstractMap", "java.util.Map");
         rspecRulesReplaceTypeMap.put("java.util.EnumMap", "java.util.Map");
@@ -85,11 +90,17 @@ public Duration getEstimatedEffortPerOccurrence() {
         rspecRulesReplaceTypeMap.put("java.util.HashSet", "java.util.Set");
         rspecRulesReplaceTypeMap.put("java.util.LinkedHashSet", "java.util.Set");
         rspecRulesReplaceTypeMap.put("java.util.concurrent.CopyOnWriteArraySet", "java.util.Set");
+
+        nonInterfaceMethods.put("java.util.Hashtable", unmodifiableSet(new HashSet<>(Arrays.asList("keys", "elements", "contains"))));
+        nonInterfaceMethods.put("java.util.Vector", unmodifiableSet(new HashSet<>(Arrays.asList("elements", "capacity", "ensureCapacity", "setSize",
+                "copyInto", "trimToSize", "elementAt", "setElementAt", "removeElementAt", "insertElementAt", "addElement", "removeElement", "removeAllElements"))));
     }
 
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
         return new JavaIsoVisitor<ExecutionContext>() {
+            private boolean usesNonInterfaceMethods = false;
+
             @Override
             public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
                 // TODO: proper Groovy support requires some extra work
@@ -100,6 +111,13 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
             public J visit(@Nullable Tree tree, ExecutionContext ctx) {
                 if (tree instanceof JavaSourceFile) {
                     JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree);
+
+                    InterfaceIncompatibleMethodDetector checker = new InterfaceIncompatibleMethodDetector();
+                    checker.visit(cu, ctx);
+                    usesNonInterfaceMethods = checker.foundNonInterfaceMethod;
+                    if (usesNonInterfaceMethods) {
+                        return cu;
+                    }
                     for (JavaType type : cu.getTypesInUse().getTypesInUse()) {
                         JavaType.FullyQualified fq = TypeUtils.asFullyQualified(type);
                         if (fq != null && rspecRulesReplaceTypeMap.containsKey(fq.getFullyQualifiedName())) {
@@ -112,6 +130,9 @@ public J visit(@Nullable Tree tree, ExecutionContext ctx) {
 
             @Override
             public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) {
+                if (usesNonInterfaceMethods) {
+                    return method;
+                }
                 J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx);
                 if ((m.hasModifier(J.Modifier.Type.Public) || m.hasModifier(J.Modifier.Type.Private) || m.getModifiers().isEmpty()) &&
                     m.getReturnTypeExpression() != null) {
@@ -152,6 +173,9 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex
 
             @Override
             public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
+                if (usesNonInterfaceMethods) {
+                    return method;
+                }
                 J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
                 if ((mi.getSelect() instanceof J.Identifier || mi.getSelect() instanceof J.FieldAccess) && mi.getSelect().getType() != null) {
                     JavaType originalType = mi.getSelect().getType();
@@ -174,6 +198,9 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
 
             @Override
             public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) {
+                if (usesNonInterfaceMethods) {
+                    return multiVariable;
+                }
                 J.VariableDeclarations mv = super.visitVariableDeclarations(multiVariable, ctx);
                 JavaType.FullyQualified originalType = TypeUtils.asFullyQualified(mv.getType());
                 if ((mv.hasModifier(J.Modifier.Type.Public) || mv.hasModifier(J.Modifier.Type.Private) || mv.getModifiers().isEmpty()) &&
@@ -255,4 +282,30 @@ private TypeTree removeFromParameterizedType(JavaType.FullyQualified newType,
             }
         };
     }
+
+    private static class InterfaceIncompatibleMethodDetector extends JavaIsoVisitor<ExecutionContext> {
+        boolean foundNonInterfaceMethod  = false;
+
+        @Override
+        public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
+            if (foundNonInterfaceMethod) {
+                return method;
+            }
+            if (method.getSelect() != null) {
+                JavaType selectType = method.getSelect().getType();
+                JavaType.FullyQualified fqType = TypeUtils.asFullyQualified(selectType);
+
+                if (fqType != null) {
+                    String className = fqType.getFullyQualifiedName();
+                    Set<String> concreteMethods = nonInterfaceMethods.get(className);
+
+                    if (concreteMethods != null && concreteMethods.contains(method.getSimpleName())) {
+                        foundNonInterfaceMethod = true;
+                        return method;
+                    }
+                }
+            }
+            return super.visitMethodInvocation(method, ctx);
+        }
+    }
 }

src/test/java/org/openrewrite/staticanalysis/UseCollectionInterfacesTest.java

@@ -1112,4 +1112,107 @@ public int method(Set<Integer> input) {
           )
         );
     }
+
+    @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/688")
+    @Test
+    void usesMethodNotOnInterface() {
+        rewriteRun(
+          java(
+            """
+              import java.util.Enumeration;
+              import java.util.Hashtable;
+
+              public class A {
+                  public Enumeration<Integer> usesMethodNotOnInterface() {
+                      Hashtable<Integer,Object> table = new Hashtable<>();
+
+                      return table.keys();
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void hashtableWithOnlyMapMethods() {
+        rewriteRun(
+          java(
+            """
+              import java.util.Hashtable;
+
+              public class A {
+                  public Hashtable<String, Integer> useOnlyMapMethods() {
+                      Hashtable<String, Integer> table = new Hashtable<>();
+                      table.put("key", 1);
+                      return table;
+                  }
+              }
+              """,
+            """
+              import java.util.Hashtable;
+              import java.util.Map;
+
+              public class A {
+                  public Map<String, Integer> useOnlyMapMethods() {
+                      Map<String, Integer> table = new Hashtable<>();
+                      table.put("key", 1);
+                      return table;
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void vectorWithOnlyListMethods() {
+        rewriteRun(
+          java(
+            """
+              import java.util.Vector;
+
+              public class A {
+                  public Vector getData() {
+                      Vector<String> vector = new Vector<>();
+                      vector.add("item");
+                      return vector;
+                  }
+              }
+              """,
+            """
+              import java.util.List;
+              import java.util.Vector;
+
+              public class A {
+                  public List getData() {
+                      List<String> vector = new Vector<>();
+                      vector.add("item");
+                      return vector;
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void usesVectorElementsMethod() {
+        rewriteRun(
+          java(
+            """
+              import java.util.Enumeration;
+              import java.util.Vector;
+
+              public class A {
+                  public Enumeration<String> usesVectorElements() {
+                      Vector<String> vector = new Vector<>();
+                      return vector.elements();
+                  }
+              }
+              """
+          )
+        );
+    }
+
 }