Make Futures.getChecked prefer constructors with a Throwable parameter.

It still prefers constructors with a `String` parameter over anything else, but to break ties among the constructors that meet that criterion, it now looks for a `Throwable` parameter, too.

This helps with exception classes that accept a `Throwable` and do something with it beyond setting it as the `cause`.

RELNOTES=`util.concurrent`: `Futures.getChecked`, which continues to prefer to call constructors with a `String` parameter, now breaks ties based on whether the constructor has a `Throwable` parameter. Beyond that, the choice of constructor remains undefined. (For this and other reasons, we discourage the use of `getChecked`.)
PiperOrigin-RevId: 563089337

Author: cpovirk

android/guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedInputs.java

@@ -18,6 +18,7 @@
 
 import com.google.common.annotations.GwtCompatible;
 import java.util.concurrent.Future;
+import javax.annotation.CheckForNull;
 
 /**
  * Classes and futures used in {@link FuturesGetCheckedTest} and {@link FuturesGetUncheckedTest}.
@@ -58,6 +59,47 @@ private ExceptionWithPrivateConstructor(String message, Throwable cause) {
     }
   }
 
+  public static final class ExceptionWithManyConstructorsButOnlyOneThrowable extends Exception {
+    @CheckForNull private Throwable antecedent;
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(String message, String a1) {
+      super(message);
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(String message, String a1, String a2) {
+      super(message);
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(
+        String message, String a1, String a2, String a3) {
+      super(message);
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(String message, Throwable antecedent) {
+      super(message);
+      this.antecedent = antecedent;
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(
+        String message, String a1, String a2, String a3, String a4) {
+      super(message);
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(
+        String message, String a1, String a2, String a3, String a4, String a5) {
+      super(message);
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(
+        String message, String a1, String a2, String a3, String a4, String a5, String a6) {
+      super(message);
+    }
+
+    public Throwable getAntecedent() {
+      return antecedent;
+    }
+  }
+
   @SuppressWarnings("unused") // we're testing that they're not used
   public static final class ExceptionWithSomePrivateConstructors extends Exception {
     private ExceptionWithSomePrivateConstructors(String a) {}

android/guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java

@@ -30,11 +30,13 @@
 import static com.google.common.util.concurrent.FuturesGetCheckedInputs.RUNTIME_EXCEPTION_FUTURE;
 import static com.google.common.util.concurrent.FuturesGetCheckedInputs.UNCHECKED_EXCEPTION;
 import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.Assert.assertThrows;
 
 import com.google.common.testing.GcFinalization;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithBadConstructor;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithGoodAndBadConstructor;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithManyConstructors;
+import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithManyConstructorsButOnlyOneThrowable;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithPrivateConstructor;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithSomePrivateConstructors;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithWrongTypesConstructor;
@@ -349,6 +351,18 @@ public void testGetCheckedUntimed_exceptionClassUsedInitCause() {
     }
   }
 
+  public void testPrefersConstructorWithThrowableParameter() {
+    ExceptionWithManyConstructorsButOnlyOneThrowable exception =
+        assertThrows(
+            ExceptionWithManyConstructorsButOnlyOneThrowable.class,
+            () ->
+                getChecked(
+                    FAILED_FUTURE_CHECKED_EXCEPTION,
+                    ExceptionWithManyConstructorsButOnlyOneThrowable.class));
+    assertThat(exception).hasMessageThat().contains("mymessage");
+    assertThat(exception.getAntecedent()).isEqualTo(CHECKED_EXCEPTION);
+  }
+
   // Class unloading test:
 
   public static final class WillBeUnloadedException extends Exception {}

android/guava/src/com/google/common/util/concurrent/Futures.java

@@ -1167,10 +1167,10 @@ public String toString() {
    *
    * <p>Instances of {@code exceptionClass} are created by choosing an arbitrary public constructor
    * that accepts zero or more arguments, all of type {@code String} or {@code Throwable}
-   * (preferring constructors with at least one {@code String}) and calling the constructor via
-   * reflection. If the exception did not already have a cause, one is set by calling {@link
-   * Throwable#initCause(Throwable)} on it. If no such constructor exists, an {@code
-   * IllegalArgumentException} is thrown.
+   * (preferring constructors with at least one {@code String}, then preferring constructors with at
+   * least one {@code Throwable}) and calling the constructor via reflection. If the exception did
+   * not already have a cause, one is set by calling {@link Throwable#initCause(Throwable)} on it.
+   * If no such constructor exists, an {@code IllegalArgumentException} is thrown.
    *
    * @throws X if {@code get} throws any checked exception except for an {@code ExecutionException}
    *     whose cause is not itself a checked exception
@@ -1219,10 +1219,10 @@ public String toString() {
    *
    * <p>Instances of {@code exceptionClass} are created by choosing an arbitrary public constructor
    * that accepts zero or more arguments, all of type {@code String} or {@code Throwable}
-   * (preferring constructors with at least one {@code String}) and calling the constructor via
-   * reflection. If the exception did not already have a cause, one is set by calling {@link
-   * Throwable#initCause(Throwable)} on it. If no such constructor exists, an {@code
-   * IllegalArgumentException} is thrown.
+   * (preferring constructors with at least one {@code String}, then preferring constructors with at
+   * least one {@code Throwable}) and calling the constructor via reflection. If the exception did
+   * not already have a cause, one is set by calling {@link Throwable#initCause(Throwable)} on it.
+   * If no such constructor exists, an {@code IllegalArgumentException} is thrown.
    *
    * @throws X if {@code get} throws any checked exception except for an {@code ExecutionException}
    *     whose cause is not itself a checked exception

android/guava/src/com/google/common/util/concurrent/FuturesGetChecked.java

@@ -21,7 +21,6 @@
 import com.google.common.annotations.GwtIncompatible;
 import com.google.common.annotations.J2ktIncompatible;
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Function;
 import com.google.common.collect.Ordering;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.lang.ref.WeakReference;
@@ -193,7 +192,7 @@ private static <X extends Exception> X newWithCause(Class<X> exceptionClass, Thr
     // getConstructors() guarantees this as long as we don't modify the array.
     @SuppressWarnings({"unchecked", "rawtypes"})
     List<Constructor<X>> constructors = (List) Arrays.asList(exceptionClass.getConstructors());
-    for (Constructor<X> constructor : preferringStrings(constructors)) {
+    for (Constructor<X> constructor : preferringStringsThenThrowables(constructors)) {
       X instance = newFromConstructor(constructor, cause);
       if (instance != null) {
         if (instance.getCause() == null) {
@@ -209,17 +208,22 @@ private static <X extends Exception> X newWithCause(Class<X> exceptionClass, Thr
         cause);
   }
 
-  private static <X extends Exception> List<Constructor<X>> preferringStrings(
+  private static <X extends Exception> List<Constructor<X>> preferringStringsThenThrowables(
       List<Constructor<X>> constructors) {
-    return WITH_STRING_PARAM_FIRST.sortedCopy(constructors);
+    return WITH_STRING_PARAM_THEN_WITH_THROWABLE_PARAM.sortedCopy(constructors);
   }
 
-  private static final Ordering<Constructor<?>> WITH_STRING_PARAM_FIRST =
+  // TODO: b/296487962 - Consider defining a total order over constructors.
+  private static final Ordering<List<Class<?>>> ORDERING_BY_CONSTRUCTOR_PARAMETER_LIST =
       Ordering.natural()
-          .onResultOf(
-              (Function<Constructor<?>, Boolean>)
-                  input -> asList(input.getParameterTypes()).contains(String.class))
+          .onResultOf((List<Class<?>> params) -> params.contains(String.class))
+          .compound(
+              Ordering.natural()
+                  .onResultOf((List<Class<?>> params) -> params.contains(Throwable.class)))
           .reverse();
+  private static final Ordering<Constructor<?>> WITH_STRING_PARAM_THEN_WITH_THROWABLE_PARAM =
+      ORDERING_BY_CONSTRUCTOR_PARAMETER_LIST.onResultOf(
+          constructor -> asList(constructor.getParameterTypes()));
 
   @CheckForNull
   private static <X> X newFromConstructor(Constructor<X> constructor, Throwable cause) {

guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedInputs.java

@@ -18,6 +18,7 @@
 
 import com.google.common.annotations.GwtCompatible;
 import java.util.concurrent.Future;
+import javax.annotation.CheckForNull;
 
 /**
  * Classes and futures used in {@link FuturesGetCheckedTest} and {@link FuturesGetUncheckedTest}.
@@ -58,6 +59,47 @@ private ExceptionWithPrivateConstructor(String message, Throwable cause) {
     }
   }
 
+  public static final class ExceptionWithManyConstructorsButOnlyOneThrowable extends Exception {
+    @CheckForNull private Throwable antecedent;
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(String message, String a1) {
+      super(message);
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(String message, String a1, String a2) {
+      super(message);
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(
+        String message, String a1, String a2, String a3) {
+      super(message);
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(String message, Throwable antecedent) {
+      super(message);
+      this.antecedent = antecedent;
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(
+        String message, String a1, String a2, String a3, String a4) {
+      super(message);
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(
+        String message, String a1, String a2, String a3, String a4, String a5) {
+      super(message);
+    }
+
+    public ExceptionWithManyConstructorsButOnlyOneThrowable(
+        String message, String a1, String a2, String a3, String a4, String a5, String a6) {
+      super(message);
+    }
+
+    public Throwable getAntecedent() {
+      return antecedent;
+    }
+  }
+
   @SuppressWarnings("unused") // we're testing that they're not used
   public static final class ExceptionWithSomePrivateConstructors extends Exception {
     private ExceptionWithSomePrivateConstructors(String a) {}

guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java

@@ -30,11 +30,13 @@
 import static com.google.common.util.concurrent.FuturesGetCheckedInputs.RUNTIME_EXCEPTION_FUTURE;
 import static com.google.common.util.concurrent.FuturesGetCheckedInputs.UNCHECKED_EXCEPTION;
 import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.Assert.assertThrows;
 
 import com.google.common.testing.GcFinalization;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithBadConstructor;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithGoodAndBadConstructor;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithManyConstructors;
+import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithManyConstructorsButOnlyOneThrowable;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithPrivateConstructor;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithSomePrivateConstructors;
 import com.google.common.util.concurrent.FuturesGetCheckedInputs.ExceptionWithWrongTypesConstructor;
@@ -349,6 +351,18 @@ public void testGetCheckedUntimed_exceptionClassUsedInitCause() {
     }
   }
 
+  public void testPrefersConstructorWithThrowableParameter() {
+    ExceptionWithManyConstructorsButOnlyOneThrowable exception =
+        assertThrows(
+            ExceptionWithManyConstructorsButOnlyOneThrowable.class,
+            () ->
+                getChecked(
+                    FAILED_FUTURE_CHECKED_EXCEPTION,
+                    ExceptionWithManyConstructorsButOnlyOneThrowable.class));
+    assertThat(exception).hasMessageThat().contains("mymessage");
+    assertThat(exception.getAntecedent()).isEqualTo(CHECKED_EXCEPTION);
+  }
+
   // Class unloading test:
 
   public static final class WillBeUnloadedException extends Exception {}

guava/src/com/google/common/util/concurrent/Futures.java

@@ -1202,10 +1202,10 @@ public String toString() {
    *
    * <p>Instances of {@code exceptionClass} are created by choosing an arbitrary public constructor
    * that accepts zero or more arguments, all of type {@code String} or {@code Throwable}
-   * (preferring constructors with at least one {@code String}) and calling the constructor via
-   * reflection. If the exception did not already have a cause, one is set by calling {@link
-   * Throwable#initCause(Throwable)} on it. If no such constructor exists, an {@code
-   * IllegalArgumentException} is thrown.
+   * (preferring constructors with at least one {@code String}, then preferring constructors with at
+   * least one {@code Throwable}) and calling the constructor via reflection. If the exception did
+   * not already have a cause, one is set by calling {@link Throwable#initCause(Throwable)} on it.
+   * If no such constructor exists, an {@code IllegalArgumentException} is thrown.
    *
    * @throws X if {@code get} throws any checked exception except for an {@code ExecutionException}
    *     whose cause is not itself a checked exception
@@ -1254,10 +1254,10 @@ public String toString() {
    *
    * <p>Instances of {@code exceptionClass} are created by choosing an arbitrary public constructor
    * that accepts zero or more arguments, all of type {@code String} or {@code Throwable}
-   * (preferring constructors with at least one {@code String}) and calling the constructor via
-   * reflection. If the exception did not already have a cause, one is set by calling {@link
-   * Throwable#initCause(Throwable)} on it. If no such constructor exists, an {@code
-   * IllegalArgumentException} is thrown.
+   * (preferring constructors with at least one {@code String}, then preferring constructors with at
+   * least one {@code Throwable}) and calling the constructor via reflection. If the exception did
+   * not already have a cause, one is set by calling {@link Throwable#initCause(Throwable)} on it.
+   * If no such constructor exists, an {@code IllegalArgumentException} is thrown.
    *
    * @throws X if {@code get} throws any checked exception except for an {@code ExecutionException}
    *     whose cause is not itself a checked exception

guava/src/com/google/common/util/concurrent/FuturesGetChecked.java

@@ -21,7 +21,6 @@
 import com.google.common.annotations.GwtIncompatible;
 import com.google.common.annotations.J2ktIncompatible;
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Function;
 import com.google.common.collect.Ordering;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.j2objc.annotations.J2ObjCIncompatible;
@@ -234,7 +233,7 @@ private static <X extends Exception> X newWithCause(Class<X> exceptionClass, Thr
     // getConstructors() guarantees this as long as we don't modify the array.
     @SuppressWarnings({"unchecked", "rawtypes"})
     List<Constructor<X>> constructors = (List) Arrays.asList(exceptionClass.getConstructors());
-    for (Constructor<X> constructor : preferringStrings(constructors)) {
+    for (Constructor<X> constructor : preferringStringsThenThrowables(constructors)) {
       X instance = newFromConstructor(constructor, cause);
       if (instance != null) {
         if (instance.getCause() == null) {
@@ -250,17 +249,22 @@ private static <X extends Exception> X newWithCause(Class<X> exceptionClass, Thr
         cause);
   }
 
-  private static <X extends Exception> List<Constructor<X>> preferringStrings(
+  private static <X extends Exception> List<Constructor<X>> preferringStringsThenThrowables(
       List<Constructor<X>> constructors) {
-    return WITH_STRING_PARAM_FIRST.sortedCopy(constructors);
+    return WITH_STRING_PARAM_THEN_WITH_THROWABLE_PARAM.sortedCopy(constructors);
   }
 
-  private static final Ordering<Constructor<?>> WITH_STRING_PARAM_FIRST =
+  // TODO: b/296487962 - Consider defining a total order over constructors.
+  private static final Ordering<List<Class<?>>> ORDERING_BY_CONSTRUCTOR_PARAMETER_LIST =
       Ordering.natural()
-          .onResultOf(
-              (Function<Constructor<?>, Boolean>)
-                  input -> asList(input.getParameterTypes()).contains(String.class))
+          .onResultOf((List<Class<?>> params) -> params.contains(String.class))
+          .compound(
+              Ordering.natural()
+                  .onResultOf((List<Class<?>> params) -> params.contains(Throwable.class)))
           .reverse();
+  private static final Ordering<Constructor<?>> WITH_STRING_PARAM_THEN_WITH_THROWABLE_PARAM =
+      ORDERING_BY_CONSTRUCTOR_PARAMETER_LIST.onResultOf(
+          constructor -> asList(constructor.getParameterTypes()));
 
   @CheckForNull
   private static <X> X newFromConstructor(Constructor<X> constructor, Throwable cause) {