Preserve MustImplementMethodsVisitor method type ancestors

src/main/java/org/checkerframework/specimin/MustImplementMethodsVisitor.java

@@ -11,10 +11,12 @@
 import com.github.javaparser.ast.stmt.BlockStmt;
 import com.github.javaparser.ast.type.ClassOrInterfaceType;
 import com.github.javaparser.ast.visitor.Visitable;
+import com.github.javaparser.resolution.MethodUsage;
 import com.github.javaparser.resolution.UnsolvedSymbolException;
 import com.github.javaparser.resolution.declarations.ResolvedMethodDeclaration;
 import com.github.javaparser.resolution.declarations.ResolvedParameterDeclaration;
 import com.github.javaparser.resolution.types.ResolvedReferenceType;
+import com.github.javaparser.resolution.types.ResolvedType;
 import com.github.javaparser.resolution.types.parametrization.ResolvedTypeParametersMap;
 import java.util.Collection;
 import java.util.HashMap;
@@ -30,7 +32,6 @@
  * run after the list of used classes is finalized.
  */
 public class MustImplementMethodsVisitor extends SpeciminStateVisitor {
-
   /**
    * Constructs a new SolveMethodOverridingVisitor with the provided sets of target methods, used
    * members, and used classes.
@@ -63,15 +64,16 @@ public Visitable visit(MethodDeclaration method, Void p) {
     // implementing required methods from interfaces in the target code,
     // but unfortunately I think it's the best that we can do here. (@Override
     // is technically optional, but it is widely used.)
-    if (isPreservedAndAbstract(overridden)
+    if (ancestorMethodPreservedAndAbstract(method)
         || (overridden == null && overridesAnInterfaceMethod(method))) {
       ResolvedMethodDeclaration resolvedMethod = method.resolve();
-      Set<String> returnAndParamTypes = new HashSet<>();
+      Map<String, ResolvedType> returnAndParamTypes = new HashMap<>();
       try {
-        returnAndParamTypes.add(resolvedMethod.getReturnType().describe());
+        returnAndParamTypes.put(
+            resolvedMethod.getReturnType().describe(), resolvedMethod.getReturnType());
         for (int i = 0; i < resolvedMethod.getNumberOfParams(); ++i) {
           ResolvedParameterDeclaration param = resolvedMethod.getParam(i);
-          returnAndParamTypes.add(param.describeType());
+          returnAndParamTypes.put(param.describeType(), param.getType());
         }
       } catch (UnsolvedSymbolException | UnsupportedOperationException e) {
         // In this case, don't keep the method (it won't compile anyway,
@@ -80,7 +82,8 @@ public Visitable visit(MethodDeclaration method, Void p) {
         return super.visit(method, p);
       }
       usedMembers.add(resolvedMethod.getQualifiedSignature());
-      for (String type : returnAndParamTypes) {
+      for (String type : returnAndParamTypes.keySet()) {
+        String originalType = type;
         type = type.trim();
         if (type.contains("<")) {
           // remove generics, if present, since this type will be used in
@@ -91,12 +94,83 @@ public Visitable visit(MethodDeclaration method, Void p) {
         if (type.contains("[]")) {
           type = type.replace("[]", "");
         }
+
+        boolean previouslyIncluded = usedTypeElements.contains(type);
+
         usedTypeElements.add(type);
+
+        ResolvedType resolvedType = returnAndParamTypes.get(originalType);
+
+        if (!previouslyIncluded && resolvedType != null && resolvedType.isReferenceType()) {
+          addAllResolvableAncestors(resolvedType.asReferenceType());
+        }
       }
     }
     return super.visit(method, p);
   }
 
+  /**
+   * Returns true iff any parent method is abstract and preserved. Use this method if it is unclear
+   * whether the direct super method will be preserved or not.
+   *
+   * @param method The method declaration to check
+   * @return true iff any parent method is abstract and preserved
+   */
+  private boolean ancestorMethodPreservedAndAbstract(MethodDeclaration method) {
+    ResolvedMethodDeclaration overridden = getOverriddenMethodInSuperClass(method);
+
+    if (isPreservedAndAbstract(overridden)) {
+      return true;
+    }
+
+    if (overridden != null) {
+      return false;
+    }
+    // The parent method is abstract, we should continue upwards
+
+    ResolvedMethodDeclaration resolvedMethod;
+    String currentMethodSignature;
+    try {
+      resolvedMethod = method.resolve();
+      currentMethodSignature = resolvedMethod.getQualifiedSignature();
+    } catch (UnsolvedSymbolException ex) {
+      return false;
+    }
+
+    String currentMethodName =
+        currentMethodSignature.substring(
+            currentMethodSignature.lastIndexOf('.', currentMethodSignature.indexOf('(')) + 1);
+    for (ResolvedReferenceType implementation :
+        getAllImplementations(new HashSet<>(resolvedMethod.declaringType().getAncestors()))) {
+      try {
+        for (MethodUsage potentialSuperMethod : implementation.getDeclaredMethods()) {
+          String methodSignature = potentialSuperMethod.getQualifiedSignature();
+          String potentialSuperMethodName =
+              methodSignature.substring(
+                  methodSignature.lastIndexOf('.', methodSignature.indexOf('(')) + 1);
+          if (!currentMethodName.equals(potentialSuperMethodName)) {
+            continue;
+          }
+          if (potentialSuperMethod.getDeclaration().isAbstract()) {
+            // These classes are beyond our control. It's better to retain the implementations of
+            // all abstract methods to ensure the code remains compilable.
+            if (JavaLangUtils.inJdkPackage(methodSignature)) {
+              return true;
+            }
+            if (usedMembers.contains(methodSignature)) {
+              return true;
+            }
+          }
+        }
+      } catch (UnsolvedSymbolException ex) {
+        // At least one of the methods can't be solved so we will ignore this type.
+        continue;
+      }
+    }
+
+    return false;
+  }
+
   /**
    * Returns true if the given method is abstract.
    *
@@ -148,6 +222,23 @@ private boolean overridesAnInterfaceMethod(MethodDeclaration method) {
     }
   }
 
+  /**
+   * Adds all resolvable ancestors (interfaces, superclasses) to the usedTypeElements set. It is
+   * intended to be used when the type is not already present in usedTypeElements, but there is no
+   * harm in calling this elsewhere.
+   *
+   * @param resolvedType the reference type to add its ancestors
+   */
+  private void addAllResolvableAncestors(ResolvedReferenceType resolvedType) {
+    // If this method is called, this type is not used anywhere else except in this location
+    // Therefore, its inherited types (if solvable) should be preserved since it was
+    // not able to be preserved elsewhere.
+    for (ResolvedReferenceType implementation :
+        getAllImplementations(new HashSet<>(resolvedType.getAllAncestors()))) {
+      usedTypeElements.add(implementation.getQualifiedName());
+    }
+  }
+
   /**
    * Helper method for overridesAnInterfaceMethod, to allow the same code to be shared in the enum
    * and class cases.
@@ -211,12 +302,10 @@ && erase(methodInInterface.getSignature()).equals(targetSignature)) {
   }
 
   /**
-   * Gets all interface implementations of a List of ClassOrInterfaceTypes, including those of
-   * ancestors. This method is intended to be used for interface implementations only (i.e. pass in
-   * ClassOrInterfaceDeclaration.getImplementedTypes()).
+   * Helper method for getAllImplementations(Set<ResolvedReferenceType>)
    *
-   * @param types A List of interface types to find all parent interfaces
-   * @return A Collection of ResolvedReferenceTypes containing all parent interfaces
+   * @param types A List of interface/class types to find all ancestors
+   * @return A Collection of ResolvedReferenceTypes containing all ancestors
    */
   private static Collection<ResolvedReferenceType> getAllImplementations(
       List<ClassOrInterfaceType> types) {
@@ -234,6 +323,19 @@ private static Collection<ResolvedReferenceType> getAllImplementations(
       }
     }
 
+    return getAllImplementations(toTraverse);
+  }
+
+  /**
+   * Gets all interface implementations of a List of ClassOrInterfaceTypes, including those of
+   * ancestors. This method is intended to be used for interface / class implementations only (i.e.
+   * pass in ClassOrInterfaceDeclaration.getImplementedTypes() or getExtendedTypes()).
+   *
+   * @param toTraverse A List of resolved reference types to find all ancestors
+   * @return A Collection of ResolvedReferenceTypes containing all ancestors
+   */
+  private static Collection<ResolvedReferenceType> getAllImplementations(
+      Set<ResolvedReferenceType> toTraverse) {
     Map<String, ResolvedReferenceType> qualifiedNameToType = new HashMap<>();
     while (!toTraverse.isEmpty()) {
       Set<ResolvedReferenceType> newToTraverse = new HashSet<>();

src/test/java/org/checkerframework/specimin/Issue103Test.java

@@ -0,0 +1,20 @@
+package org.checkerframework.specimin;
+
+import java.io.IOException;
+import org.junit.Test;
+
+/**
+ * This test checks if Specimin will preserve ancestors of types added in
+ * MustImplementMethodsVisitor. It also checks to see if methods whose direct override is abstract,
+ * but original definition is JDK are preserved. Test code derived from a modified, minimized
+ * version of NullAway issue 103.
+ */
+public class Issue103Test {
+  @Test
+  public void runTest() throws IOException {
+    SpeciminTestExecutor.runTestWithoutJarPaths(
+        "issue103",
+        new String[] {"com/example/Simple.java"},
+        new String[] {"com.example.Simple#foo()"});
+  }
+}

src/test/resources/issue103/expected/com/example/AbstractLinkedDeque.java

@@ -0,0 +1,104 @@
+package com.example;
+
+import java.util.AbstractCollection;
+
+public abstract class AbstractLinkedDeque<E> extends AbstractCollection<E> implements LinkedDeque<E> {
+
+    public int size() {
+        throw new Error();
+    }
+
+    public abstract boolean contains(Object o);
+
+    public E peek() {
+        throw new Error();
+    }
+
+    public E peekFirst() {
+        throw new Error();
+    }
+
+    public E peekLast() {
+        throw new Error();
+    }
+
+    public E getFirst() {
+        throw new Error();
+    }
+
+    public E getLast() {
+        throw new Error();
+    }
+
+    public E element() {
+        throw new Error();
+    }
+
+    public boolean offer(E e) {
+        throw new Error();
+    }
+
+    public boolean offerFirst(E e) {
+        throw new Error();
+    }
+
+    public boolean offerLast(E e) {
+        throw new Error();
+    }
+
+    public void addFirst(E e) {
+        throw new Error();
+    }
+
+    public void addLast(E e) {
+        throw new Error();
+    }
+
+    public E poll() {
+        throw new Error();
+    }
+
+    public E pollFirst() {
+        throw new Error();
+    }
+
+    public E pollLast() {
+        throw new Error();
+    }
+
+    public E remove() {
+        throw new Error();
+    }
+
+    public E removeFirst() {
+        throw new Error();
+    }
+
+    public boolean removeFirstOccurrence(Object o) {
+        throw new Error();
+    }
+
+    public E removeLast() {
+        throw new Error();
+    }
+
+    public boolean removeLastOccurrence(Object o) {
+        throw new Error();
+    }
+
+    public void push(E e) {
+        throw new Error();
+    }
+
+    public E pop() {
+        throw new Error();
+    }
+
+    public PeekingIterator<E> iterator() {
+        throw new Error();
+    }
+
+    public PeekingIterator<E> descendingIterator() {
+        throw new Error();
+    }
+}

src/test/resources/issue103/expected/com/example/AccessOrderDeque.java

@@ -0,0 +1,10 @@
+package com.example;
+
+import java.util.Deque;
+
+public final class AccessOrderDeque<E> extends AbstractLinkedDeque<E> {
+
+    public boolean contains(Object o) {
+        throw new Error();
+    }
+}

src/test/resources/issue103/expected/com/example/LinkedDeque.java

@@ -0,0 +1,10 @@
+package com.example;
+
+import java.util.Deque;
+import java.util.Iterator;
+
+public interface LinkedDeque<E> extends Deque<E> {
+
+    interface PeekingIterator<E> extends Iterator<E> {
+    }
+}

src/test/resources/issue103/expected/com/example/Simple.java

@@ -0,0 +1,12 @@
+package com.example;
+
+public class Simple<K> {
+
+    AccessOrderDeque<K> bar() {
+        throw new Error();
+    }
+
+    void foo() {
+        AccessOrderDeque<K> x = bar();
+    }
+}