Avoid failed compilations caused by JDK classes without zero argument constructors

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

@@ -16,7 +16,9 @@
 import com.github.javaparser.ast.type.ClassOrInterfaceType;
 import com.github.javaparser.ast.type.Type;
 import com.github.javaparser.resolution.UnsolvedSymbolException;
+import com.github.javaparser.resolution.declarations.ResolvedConstructorDeclaration;
 import com.github.javaparser.resolution.declarations.ResolvedMethodLikeDeclaration;
+import com.github.javaparser.resolution.declarations.ResolvedReferenceTypeDeclaration;
 import com.github.javaparser.resolution.types.ResolvedReferenceType;
 import com.github.javaparser.resolution.types.ResolvedType;
 import com.google.common.base.Splitter;
@@ -345,6 +347,26 @@ public static String removeMethodReturnTypeSpacesAndAnnotations(NodeWithDeclarat
     return removeMethodReturnTypeAndAnnotationsImpl(declAsString).replaceAll("\\s", "");
   }
 
+  /**
+   * Checks if the enclosing class of the given constructor's superclass is a JDK class other than
+   * Object. Useful for e.g., keeping constructors that are necessary for compilation when extending
+   * a class in the JDK.
+   *
+   * @param resolved the resolved declaration of a constructor
+   * @return true iff the superclass of the class constructed by the given constructor is in the JDK
+   *     but is not java.lang.Object
+   */
+  public static boolean enclosingClassExtendsJDKClass(ResolvedConstructorDeclaration resolved) {
+    ResolvedReferenceTypeDeclaration enclosingClass = resolved.declaringType();
+    List<ResolvedReferenceType> ancestors = enclosingClass.getAllAncestors();
+    if (ancestors.isEmpty()) {
+      return false;
+    }
+    ResolvedReferenceType superClass = ancestors.get(0);
+    String superClassFQN = superClass.getQualifiedName();
+    return !"java.lang.Object".equals(superClassFQN) && JavaLangUtils.inJdkPackage(superClassFQN);
+  }
+
   /**
    * Implementation of {@link #removeMethodReturnTypeAndAnnotations(NodeWithDeclaration)}. Separated
    * for testing.

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

@@ -10,6 +10,7 @@
 import com.github.javaparser.ast.body.FieldDeclaration;
 import com.github.javaparser.ast.body.InitializerDeclaration;
 import com.github.javaparser.ast.body.MethodDeclaration;
+import com.github.javaparser.ast.body.Parameter;
 import com.github.javaparser.ast.body.VariableDeclarator;
 import com.github.javaparser.ast.expr.AnnotationExpr;
 import com.github.javaparser.ast.expr.BooleanLiteralExpr;
@@ -307,11 +308,14 @@ public Visitable visit(MethodDeclaration methodDecl, Void p) {
 
   @Override
   public Visitable visit(ConstructorDeclaration constructorDecl, Void p) {
+    ResolvedConstructorDeclaration resolved;
     String qualifiedSignature;
+
     try {
-      // resolved() will only check if the return type is solvable
+      // resolve() will only check if the return type is solvable
       // getQualifiedSignature() will also check if the parameters are solvable
-      qualifiedSignature = constructorDecl.resolve().getQualifiedSignature();
+      resolved = constructorDecl.resolve();
+      qualifiedSignature = resolved.getQualifiedSignature();
     } catch (RuntimeException e) {
       // The current class is employed by the target methods, although not all of its members are
       // utilized. It's not surprising for unused members to remain unresolved.
@@ -325,11 +329,18 @@ public Visitable visit(ConstructorDeclaration constructorDecl, Void p) {
       return super.visit(constructorDecl, p);
     }
 
+    // Need to avoid "no zero-argument constructor" compilation problems that are caused
+    // by removing constructors from classes which extend a class in the JDK.
+    boolean mustPreserveToAvoidZeroArgProblem =
+        JavaParserUtil.enclosingClassExtendsJDKClass(resolved);
+
     // TODO: we should be cleverer about whether to preserve the constructors of
     // enums, but right now we don't remove any enum constants in related classes, so
     // we need to preserve all constructors to retain compilability.
-    if (usedMembers.contains(qualifiedSignature) || JavaParserUtil.isInEnum(constructorDecl)) {
-      if (!needToPreserveSuperOrThisCall(constructorDecl.resolve())) {
+    if (mustPreserveToAvoidZeroArgProblem
+        || usedMembers.contains(qualifiedSignature)
+        || JavaParserUtil.isInEnum(constructorDecl)) {
+      if (!mustPreserveToAvoidZeroArgProblem && !needToPreserveSuperOrThisCall(resolved)) {
         constructorDecl.setBody(StaticJavaParser.parseBlock("{ throw new java.lang.Error(); }"));
         return constructorDecl;
       }
@@ -339,10 +350,29 @@ public Visitable visit(ConstructorDeclaration constructorDecl, Void p) {
         return constructorDecl;
       }
       Statement firstStatement = bodyStatement.get(0);
+
       if (firstStatement.isExplicitConstructorInvocationStmt()) {
+        if (mustPreserveToAvoidZeroArgProblem
+            && firstStatement.asExplicitConstructorInvocationStmt().isThis()) {
+          // In this case, we only care about constructors which call super().
+          constructorDecl.remove();
+          return constructorDecl;
+        }
         BlockStmt minimized = new BlockStmt();
+        firstStatement
+            .asExplicitConstructorInvocationStmt()
+            .getArguments()
+            .replaceAll(x -> nullOrPrimitive(x));
         minimized.addStatement(firstStatement);
         constructorDecl.setBody(minimized);
+        // Also need to remove annotations from parameters in this case, since
+        // they won't be preserved (and this isn't a target method).
+        if (mustPreserveToAvoidZeroArgProblem) {
+          for (Parameter param : constructorDecl.getParameters()) {
+            param.setAnnotations(NodeList.nodeList());
+          }
+        }
+
         return constructorDecl;
       }
 
@@ -355,6 +385,29 @@ public Visitable visit(ConstructorDeclaration constructorDecl, Void p) {
     return constructorDecl;
   }
 
+  /**
+   * Helper to get either a new null literal expression or the incoming expression itself, if that
+   * expression has a primitive type.
+   *
+   * @param expr an expression
+   * @return the expression itself, iff it is of a primitive type, or a new null literal expression
+   *     otherwise
+   */
+  private static Expression nullOrPrimitive(Expression expr) {
+    Expression result;
+    try {
+      result =
+          JavaLangUtils.isPrimitive(expr.calculateResolvedType().describe())
+              ? expr
+              : new NullLiteralExpr();
+    } catch (UnsolvedSymbolException e) {
+      // Can happen when the parameter is e.g., a field that has already
+      // been removed. Default to a null literal.
+      result = new NullLiteralExpr();
+    }
+    return result;
+  }
+
   @Override
   public Visitable visit(FieldDeclaration fieldDecl, Void p) {
     if (insideTargetMember) {

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

@@ -0,0 +1,19 @@
+package org.checkerframework.specimin;
+
+import java.io.IOException;
+import org.junit.Test;
+
+/**
+ * This test checks that when extending a non-JDK class with only constructors that take more than
+ * zero arguments (all of which are primitives rather than objects), at least one constructor gets
+ * preserved so that the result compiles.
+ */
+public class NoZeroArgCtorIntTest {
+  @Test
+  public void runTest() throws IOException {
+    SpeciminTestExecutor.runTestWithoutJarPaths(
+        "nozeroargctorint",
+        new String[] {"com/example/Simple.java"},
+        new String[] {"com.example.Simple#bar()"});
+  }
+}

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

@@ -0,0 +1,24 @@
+package org.checkerframework.specimin;
+
+import java.io.IOException;
+import org.junit.Test;
+
+/**
+ * This test checks that when extending a JDK class with only constructors that take more than zero
+ * arguments, at least one constructor gets preserved so that the result compiles.
+ *
+ * <p>I tried to write an equivalent of the {@link NoZeroArgCtorIntTest} using a JDK class, but I
+ * couldn't find a suitable JDK class to extend that only has constructors that take primitives. I
+ * considered the following unsuitable JDK classes: Number (has a zero arg ctor), Integer (final),
+ * Date (zero arg ctor again), BigInteger (has a ctor that takes a String). Then, I gave up and
+ * decided to only deal with that situation if we encounter it in practice.
+ */
+public class NoZeroArgCtorJDKTest {
+  @Test
+  public void runTest() throws IOException {
+    SpeciminTestExecutor.runTestWithoutJarPaths(
+        "nozeroargctorjdk",
+        new String[] {"com/example/Simple.java"},
+        new String[] {"com.example.Simple#bar()"});
+  }
+}

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

@@ -0,0 +1,18 @@
+package org.checkerframework.specimin;
+
+import java.io.IOException;
+import org.junit.Test;
+
+/**
+ * This test checks that when extending a non-JDK class with only constructors that take more than
+ * zero arguments, at least one constructor gets preserved so that the result compiles.
+ */
+public class NoZeroArgCtorTest {
+  @Test
+  public void runTest() throws IOException {
+    SpeciminTestExecutor.runTestWithoutJarPaths(
+        "nozeroargctor",
+        new String[] {"com/example/Simple.java"},
+        new String[] {"com.example.Simple#bar()"});
+  }
+}

src/test/resources/constructorrequirement/expected/com/example/Foo.java

@@ -3,7 +3,7 @@
 class BazChild extends Baz {
 
     BazChild(String args) {
-        super(args);
+        super(null);
     }
 }
 

src/test/resources/customexception/expected/com/example/CustomException.java

@@ -3,6 +3,5 @@
 public class CustomException extends Exception {
 
     public CustomException(String msg) {
-        throw new java.lang.Error();
     }
 }

src/test/resources/nestedcatchclause/expected/com/example/CustomException.java

@@ -3,6 +3,5 @@
 public class CustomException extends Exception {
 
     public CustomException(String msg) {
-        throw new java.lang.Error();
     }
 }

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

@@ -0,0 +1,7 @@
+package com.example;
+
+class Simple extends SomeOtherClass {
+    void bar() {
+
+    }
+}

src/test/resources/nozeroargctor/expected/com/example/SomeOtherClass.java

@@ -0,0 +1,4 @@
+package com.example;
+
+class SomeOtherClass {
+}

src/test/resources/nozeroargctor/input/com/example/Simple.java

@@ -0,0 +1,13 @@
+package com.example;
+
+class Simple extends SomeOtherClass {
+    // Target method.
+    void bar() {
+
+    }
+
+    // This needs to be preserved, because otherwise the class won't compile.
+    public Simple() {
+        super(null);
+    }
+}

src/test/resources/nozeroargctor/input/com/example/SomeOtherClass.java

@@ -0,0 +1,7 @@
+package com.example;
+
+class SomeOtherClass {
+    public SomeOtherClass(Object object) {
+
+    }
+}

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

@@ -0,0 +1,7 @@
+package com.example;
+
+class Simple extends SomeOtherClass {
+    void bar() {
+
+    }
+}

src/test/resources/nozeroargctorint/expected/com/example/SomeOtherClass.java

@@ -0,0 +1,4 @@
+package com.example;
+
+class SomeOtherClass {
+}

src/test/resources/nozeroargctorint/input/com/example/Simple.java

@@ -0,0 +1,13 @@
+package com.example;
+
+class Simple extends SomeOtherClass {
+    // Target method.
+    void bar() {
+
+    }
+
+    // This needs to be preserved, because otherwise the class won't compile.
+    public Simple() {
+        super(5);
+    }
+}

src/test/resources/nozeroargctorint/input/com/example/SomeOtherClass.java

@@ -0,0 +1,7 @@
+package com.example;
+
+class SomeOtherClass {
+    public SomeOtherClass(int object) {
+
+    }
+}

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

@@ -0,0 +1,13 @@
+package com.example;
+
+import java.io.LineNumberReader;
+
+class Simple extends LineNumberReader {
+    void bar() {
+
+    }
+
+    public Simple() {
+        super(null);
+    }
+}

src/test/resources/nozeroargctorjdk/input/com/example/Simple.java

@@ -0,0 +1,18 @@
+package com.example;
+
+import java.io.LineNumberReader;
+
+class Simple extends LineNumberReader {
+    // Target method.
+    void bar() {
+
+    }
+
+    // This needs to be preserved, because otherwise the class won't compile.
+    public Simple() {
+        // Note that this input won't compile (String <: Reader is obviously false), but
+        // that's okay. This is just here to test that Specimin will replace any argument
+        // with a null literal.
+        super("bananas");
+    }
+}