Address various issues related to the crash in #402

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

@@ -26,6 +26,7 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -290,9 +291,8 @@ private static void performMinimizationImpl(
     // solved). So, we track all previous iterations, and if we ever see the same
     // outputs we set "problematicIteration" to that one. If we see that output again,
     // we break the loop below early.
-    Set<UnsolvedSymbolVisitorProgress> previousIterations = new HashSet<>();
+    Set<UnsolvedSymbolVisitorProgress> previousIterations = new LinkedHashSet<>();
     UnsolvedSymbolVisitorProgress problematicIteration = null;
-
     while (addMissingClass.gettingException()) {
       addMissingClass.setExceptionToFalse();
       for (CompilationUnit cu : parsedTargetFiles.values()) {

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

@@ -233,8 +233,7 @@ protected void maintainDataStructuresPreSuper(TypeDeclaration<?> decl) {
     } else if (!JavaParserUtil.isLocalClassDecl(decl)) {
       // the purpose of keeping track of class name is to recognize the signatures of target
       // methods. Since we don't support methods inside local classes as target methods, we don't
-      // need
-      // to keep track of class name in this case.
+      // need to keep track of class name in this case.
       this.currentClassQualifiedName = decl.getFullyQualifiedName().orElseThrow();
     }
   }

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

@@ -368,6 +368,15 @@ public boolean updateMethodByReturnType(String currentReturnType, String desired
         successfullyUpdated = true;
       }
     }
+    if (innerClasses != null) {
+      for (UnsolvedClassOrInterface innerClass : innerClasses) {
+        boolean foundInInnerClass =
+            innerClass.updateMethodByReturnType(currentReturnType, desiredReturnType);
+        if (foundInInnerClass) {
+          successfullyUpdated = true;
+        }
+      }
+    }
     return successfullyUpdated;
   }
 
@@ -413,8 +422,17 @@ public boolean updateFieldByType(String currentType, String correctType) {
                 correctType, staticKeyword + finalKeyword + correctType + " " + fieldName));
       }
     }
-
     classFields.addAll(newFields);
+
+    if (innerClasses != null) {
+      for (UnsolvedClassOrInterface innerClass : innerClasses) {
+        boolean foundInInnerClass = innerClass.updateFieldByType(currentType, correctType);
+        if (foundInInnerClass) {
+          successfullyUpdated = true;
+        }
+      }
+    }
+
     return successfullyUpdated;
   }
 
@@ -428,6 +446,12 @@ public void addInnerClass(UnsolvedClassOrInterface innerClass) {
       // LinkedHashSet to make the iteration order deterministic.
       this.innerClasses = new LinkedHashSet<>(1);
     }
+    if (this.innerClasses.contains(innerClass)) {
+      // replace the old version with the new one (e.g., if a method has been added; the
+      // number of methods is not included in the equality check for the set...)
+      // TODO: I think this data structure isn't quite right for what this is doing
+      this.innerClasses.remove(innerClass);
+    }
     this.innerClasses.add(innerClass);
   }
 

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

@@ -305,7 +305,8 @@ private void setclassAndPackageMap() {
       if (!importParts.isEmpty()) {
         String className = importParts.get(importParts.size() - 1);
         String packageName = importStatement.replace("." + className, "");
-        if (!"*".equals(className)) {
+        // Avoids accidentally adding the last package name in a wildcard import.
+        if (JavaParserUtil.isCapital(className)) {
           this.classAndPackageMap.put(className, packageName);
         }
       }
@@ -896,9 +897,11 @@ public Visitable visit(VariableDeclarator decl, Void p) {
     } catch (UnsolvedSymbolException | UnsupportedOperationException e) {
       String typeAsString = declType.asString();
       List<String> elements = Splitter.onPattern("\\.").splitToList(typeAsString);
-      // There could be three cases here: a type variable, a fully-qualified class name, or a simple
-      // class name.
-      // This is the fully-qualified case.
+      // There could be four cases here: a type variable, a fully-qualified class name, or a simple
+      // class name (this last case has two sub-cases: one for inner classes and one for true simple
+      // names).
+      // This is the fully-qualified case; it's necessary to carefully distinguish it from the inner
+      // class case, which looks similar.
       if (elements.size() > 1) {
         int typeParamIndex = typeAsString.indexOf('<');
         int typeParamCount = -1;
@@ -912,7 +915,7 @@ public Visitable visit(VariableDeclarator decl, Void p) {
             "signature") // since this type is in a fully-qualified form, or we make it
         // fully-qualified
         @FullyQualifiedName String qualifiedTypeName =
-            typeAsString.contains(".")
+            typeAsString.contains(".") && !JavaParserUtil.isCapital(typeAsString)
                 ? typeAsString
                 : getPackageFromClassName(typeAsString) + "." + typeAsString;
         UnsolvedClassOrInterface unsolved =
@@ -1449,7 +1452,7 @@ Test foo() {
       @SuppressWarnings("signature") // Already guaranteed to be a FQN here
       @FullyQualifiedName String qualifiedTypeName = anno.getNameAsString();
       unsolvedAnnotation = getSimpleSyntheticClassFromFullyQualifiedName(qualifiedTypeName);
-      updateMissingClass(unsolvedAnnotation);
+      unsolvedAnnotation = updateMissingClass(unsolvedAnnotation);
     } else {
       unsolvedAnnotation = updateUnsolvedClassWithClassName(anno.getNameAsString(), false, false);
     }
@@ -1496,7 +1499,7 @@ Test foo() {
       @SuppressWarnings("signature") // Already guaranteed to be a FQN here
       @FullyQualifiedName String qualifiedTypeName = anno.getNameAsString();
       unsolvedAnnotation = getSimpleSyntheticClassFromFullyQualifiedName(qualifiedTypeName);
-      updateMissingClass(unsolvedAnnotation);
+      unsolvedAnnotation = updateMissingClass(unsolvedAnnotation);
     } else {
       unsolvedAnnotation = updateUnsolvedClassWithClassName(anno.getNameAsString(), false, false);
     }
@@ -1553,7 +1556,7 @@ Test foo() {
       @SuppressWarnings("signature") // Already guaranteed to be a FQN here
       @FullyQualifiedName String qualifiedTypeName = anno.getNameAsString();
       unsolvedAnnotation = getSimpleSyntheticClassFromFullyQualifiedName(qualifiedTypeName);
-      updateMissingClass(unsolvedAnnotation);
+      unsolvedAnnotation = updateMissingClass(unsolvedAnnotation);
     } else {
       unsolvedAnnotation = updateUnsolvedClassWithClassName(anno.getNameAsString(), false, false);
     }
@@ -1744,6 +1747,7 @@ private void solveSymbolsForClassOrInterfaceType(
       typeRawName = typeRawName.substring(0, typeRawName.indexOf("<"));
     }
 
+    // TODO: why doesn't this work: Pair<String, String> pkgAndClass = splitName(typeRawName);
     String packageName, className;
     if (JavaParserUtil.isAClassPath(typeRawName)) {
       // Two cases: this could be either an Outer.Inner pair or it could
@@ -1952,15 +1956,13 @@ private void updateSyntheticClassesForTypeVar(ClassOrInterfaceType type) {
    * @param className the class name to be converted
    * @return the simple form of that class name
    */
-  // We can have certainty that this method is true as the last element of a class name is the
-  // simple form of that name
   @SuppressWarnings("signature")
-  public static @ClassGetSimpleName String toSimpleName(@DotSeparatedIdentifiers String className) {
-    List<String> elements = Splitter.onPattern("[.]").splitToList(className);
-    if (elements.size() < 2) {
-      return className;
+  public @ClassGetSimpleName String toSimpleName(@DotSeparatedIdentifiers String className) {
+    if ("void".equals(className)) {
+      return "void";
     }
-    return elements.get(elements.size() - 1);
+    Pair<String, String> pkgAndClass = splitName(className);
+    return (@ClassGetSimpleName String) pkgAndClass.b;
   }
 
   /**
@@ -2053,7 +2055,7 @@ public void updateUnsolvedClassOrInterfaceWithMethod(
     if (desiredReturnType.equals("")) {
       UnsolvedClassOrInterface returnTypeForThisMethod =
           new UnsolvedClassOrInterface(returnType, missingClass.getPackageName());
-      this.updateMissingClass(returnTypeForThisMethod);
+      returnTypeForThisMethod = this.updateMissingClass(returnTypeForThisMethod);
       classAndPackageMap.put(
           returnTypeForThisMethod.getClassName(), returnTypeForThisMethod.getPackageName());
     }
@@ -2245,8 +2247,7 @@ public UnsolvedClassOrInterface updateUnsolvedClassWithClassName(
     for (UnsolvedMethod unsolvedMethod : unsolvedMethods) {
       result.addMethod(unsolvedMethod);
     }
-    updateMissingClass(result);
-    return result;
+    return updateMissingClass(result);
   }
 
   /**
@@ -3058,17 +3059,16 @@ public static boolean calledByAnIncompleteClass(MethodCallExpr method) {
   }
 
   /**
-   * This method converts a @FullyQualifiedName classname to a @ClassGetSimpleName classname. Note
-   * that there is warning suppression here. It is safe to claim that if we split
-   * a @FullyQualifiedName name by dot ([.]), then the last part is the @ClassGetSimpleName part.
+   * This method converts a @FullyQualifiedName name to a @ClassGetSimpleName name.
    *
-   * @param fullyQualifiedName a @FullyQualifiedName classname
+   * @param fullyQualifiedName a @FullyQualifiedName class name
    * @return the @ClassGetSimpleName version of that class
    */
-  public static @ClassGetSimpleName String fullyQualifiedToSimple(
+  public @ClassGetSimpleName String fullyQualifiedToSimple(
       @FullyQualifiedName String fullyQualifiedName) {
+    Pair<String, String> pkgAndClass = splitName(fullyQualifiedName);
     @SuppressWarnings("signature")
-    @ClassGetSimpleName String simpleName = fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf(".") + 1);
+    @ClassGetSimpleName String simpleName = pkgAndClass.b;
     return simpleName;
   }
 
@@ -3091,16 +3091,20 @@ public static boolean calledByAnIncompleteClass(MethodCallExpr method) {
    * method to an existing class.
    *
    * @param missedClass the class to be updated
+   * @return the class or interface declaration after the update. This may differ from the input,
+   *     since it might need to be combined with a synthetic class that already exist (or it might
+   *     be an inner class of some other synthetic class, or...). If any further operations are
+   *     going to be done with the input, use the result instead.
    */
-  public void updateMissingClass(UnsolvedClassOrInterface missedClass) {
+  public UnsolvedClassOrInterface updateMissingClass(UnsolvedClassOrInterface missedClass) {
     String qualifiedName = missedClass.getQualifiedClassName();
     // If an original class from the input codebase is used with unsolved type parameters, it may be
     // misunderstood as an unresolved class.
     if (classfileIsInOriginalCodebase(qualifiedName)) {
-      return;
+      return missedClass;
     }
     if (JavaLangUtils.inJdkPackage(qualifiedName)) {
-      return;
+      return missedClass;
     }
 
     // If the input contains something simple like Map.Entry,
@@ -3133,7 +3137,7 @@ public void updateMissingClass(UnsolvedClassOrInterface missedClass) {
               new UnsolvedClassOrInterface.UnsolvedInnerClass(innerClassName, e.getPackageName());
           updateMissingClassHelper(missedClass, innerClass);
           e.addInnerClass(innerClass);
-          return;
+          return e;
         }
       }
       // The outer class doesn't exist yet. Create it.
@@ -3146,16 +3150,17 @@ public void updateMissingClass(UnsolvedClassOrInterface missedClass) {
       updateMissingClassHelper(missedClass, innerClass);
       outerClass.addInnerClass(innerClass);
       missingClass.add(outerClass);
-      return;
+      return outerClass;
     }
 
     for (UnsolvedClassOrInterface e : missingClass) {
       if (e.equals(missedClass)) {
         updateMissingClassHelper(missedClass, e);
-        return;
+        return e;
       }
     }
     missingClass.add(missedClass);
+    return missedClass;
   }
 
   /**
@@ -3281,7 +3286,7 @@ public static String toCapital(String string) {
    * @param fullyName the fully-qualified name of the class
    * @return the corresponding instance of UnsolvedClass
    */
-  public static UnsolvedClassOrInterface getSimpleSyntheticClassFromFullyQualifiedName(
+  public UnsolvedClassOrInterface getSimpleSyntheticClassFromFullyQualifiedName(
       @FullyQualifiedName String fullyName) {
     if (!JavaParserUtil.isAClassPath(fullyName)) {
       throw new RuntimeException(
@@ -3588,11 +3593,11 @@ public void updateClassSetWithQualifiedFieldSignature(
     fieldDeclaration =
         setInitialValueForVariableDeclaration(fieldTypeClassName.toString(), fieldDeclaration);
     classThatContainField.addFields(fieldDeclaration);
+    this.updateMissingClass(typeClass);
+    classThatContainField = this.updateMissingClass(classThatContainField);
     classAndPackageMap.put(thisFieldType, packageName.toString());
     classAndPackageMap.put(className, packageName.toString());
     syntheticTypeAndClass.put(thisFieldType, classThatContainField);
-    this.updateMissingClass(typeClass);
-    this.updateMissingClass(classThatContainField);
   }
 
   /**
@@ -3714,11 +3719,13 @@ public boolean updateTypeForSyntheticClasses(
       boolean updateAField,
       String incorrectTypeName,
       String correctTypeName) {
+
     // Make sure that correctTypeName is fully qualified, so that we don't need to
     // add an import to the synthetic class.
     if (!correctTypeName.contains(JavaTypeCorrect.SYNTHETIC_UNCONSTRAINED_TYPE)) {
       correctTypeName = lookupFQNs(correctTypeName);
     }
+
     boolean updatedSuccessfully = false;
     UnsolvedClassOrInterface classToSearch = new UnsolvedClassOrInterface(className, packageName);
     Iterator<UnsolvedClassOrInterface> iterator = missingClass.iterator();
@@ -3743,7 +3750,11 @@ public boolean updateTypeForSyntheticClasses(
         return updatedSuccessfully;
       }
     }
-    throw new RuntimeException("Could not find the corresponding missing class!");
+    throw new RuntimeException(
+        "Could not find the corresponding missing class! Looking for: "
+            + packageName
+            + "."
+            + className);
   }
 
   /**
@@ -3963,6 +3974,8 @@ private void lookupTypeArgumentFQN(StringBuilder fullyQualifiedName, Type typeAr
       } else if (asWildcardType.getExtendedType().isPresent()) {
         fullyQualifiedName.append("? extends ");
         lookupTypeArgumentFQN(fullyQualifiedName, asWildcardType.getExtendedType().get());
+      } else {
+        fullyQualifiedName.append("?");
       }
     } else if (JavaParserUtil.isAClassPath(erased)) {
       // If it's already a fully qualified name, don't do anything

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

@@ -0,0 +1,18 @@
+package org.checkerframework.specimin;
+
+import java.io.IOException;
+import org.junit.Test;
+
+/**
+ * This test checks that a simple Java file with no dependencies and a single target method with one
+ * method that it depends on results in that depended-on method being replaced by an empty body.
+ */
+public class ReturnTypeFailTest {
+  @Test
+  public void runTest() throws IOException {
+    SpeciminTestExecutor.runTestWithoutJarPaths(
+        "returnTypeFail",
+        new String[] {"com/example/MapperConfigBase.java"},
+        new String[] {"com.example.MapperConfigBase#getDefaultPropertyInclusion(Class<?>)"});
+  }
+}

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

@@ -0,0 +1,19 @@
+package org.checkerframework.specimin;
+
+import java.io.IOException;
+import org.junit.Ignore;
+import org.junit.Test;
+
+@Ignore
+public class StringIndexOobTest {
+  @Test
+  public void runTest() throws IOException {
+    SpeciminTestExecutor.runTestWithoutJarPaths(
+        "stringindexoob",
+        new String[] {"com/example/BasicDeserializerFactory.java"},
+        new String[] {
+          "com.example.BasicDeserializerFactory#createArrayDeserializer(DeserializationContext,"
+              + " ArrayType, BeanDescription)"
+        });
+  }
+}

src/test/resources/returnTypeFail/expected/com/example/MapperConfigBase.java

@@ -0,0 +1,24 @@
+package com.example;
+
+import com.fasterxml.jackson.annotation.*;
+
+@SuppressWarnings("serial")
+public abstract class MapperConfigBase<CFG extends ConfigFeature, T> extends MapperConfig<T> {
+
+    public final ConfigOverride getConfigOverride(Class<?> type) {
+        throw new java.lang.Error();
+    }
+
+    public final JsonInclude.Value getDefaultPropertyInclusion() {
+        throw new java.lang.Error();
+    }
+
+    public final JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType) {
+        JsonInclude.Value v = getConfigOverride(baseType).getInclude();
+        JsonInclude.Value def = getDefaultPropertyInclusion();
+        if (def == null) {
+            return v;
+        }
+        return def.withOverrides(v);
+    }
+}

src/test/resources/returnTypeFail/expected/com/fasterxml/jackson/annotation/ConfigFeature.java

@@ -0,0 +1,4 @@
+package com.fasterxml.jackson.annotation;
+
+public class ConfigFeature {
+}

src/test/resources/returnTypeFail/expected/com/fasterxml/jackson/annotation/ConfigOverride.java

@@ -0,0 +1,8 @@
+package com.fasterxml.jackson.annotation;
+
+public class ConfigOverride {
+
+    public com.fasterxml.jackson.annotation.JsonInclude.Value getInclude() {
+        throw new java.lang.Error();
+    }
+}