Deprecate the constructors of our ExecutionException-like classes t…

…hat don't accept a cause. Callers commonly assume that instances of these types have a cause, and in practice, they nearly always do. See jspecify/jspecify#490. RELNOTES=`util.concurrent`: Deprecated the constructors of `UncheckedExecutionException` and `ExecutionError` that don't accept a cause. We won't remove these constructors, but we recommend migrating off them, as users of those classes often assume that instances will contain a cause. PiperOrigin-RevId: 613614892
google · Mar 7, 2024 · 1bb3c43 · 1bb3c43
1 parent b90ce5b
commit 1bb3c43
Show file tree

Hide file tree

Showing 4 changed files with 144 additions and 24 deletions.
diff --git a/android/guava/src/com/google/common/util/concurrent/ExecutionError.java b/android/guava/src/com/google/common/util/concurrent/ExecutionError.java
@@ -31,25 +31,55 @@
 @ElementTypesAreNonnullByDefault
 public class ExecutionError extends Error {
   /*
-   * Ideally, this class would have exposed only constructors that require a non-null cause. We
-   * might try to move in that direction, but there are complications. See
+   * Ideally, this class would have exposed only constructors that require a non-null cause. See
    * https://github.com/jspecify/jspecify-reference-checker/blob/61aafa4ae52594830cfc2d61c8b113009dbdb045/src/main/java/com/google/jspecify/nullness/NullSpecTransfer.java#L789
+   * and https://github.com/jspecify/jspecify/issues/490.
+   *
+   * (That would also have ensured that its cause was always an Error, rather than possibly another
+   * kind of Throwable that was later passed to initCause. Then we could have declared the override
+   * `public final Error getCause()`.)
    */
 
-  /** Creates a new instance with {@code null} as its detail message. */
+  /**
+   * Creates a new instance with {@code null} as its detail message and no cause.
+   *
+   * @deprecated Prefer {@linkplain ExecutionError(Error)} a constructor that accepts a cause: Users
+   *     of this class typically expect for instances to have a non-null cause. At the moment, you
+   *     can <i>usually</i> still preserve behavior by passing an explicit {@code null} cause. Note,
+   *     however, that passing an explicit {@code null} cause prevents anyone from calling {@link
+   *     #initCause} later, so it is not quite equivalent to using a constructor that omits the
+   *     cause.
+   */
+  @Deprecated
   protected ExecutionError() {}
 
-  /** Creates a new instance with the given detail message. */
+  /**
+   * Creates a new instance with the given detail message and no cause.
+   *
+   * @deprecated Prefer {@linkplain ExecutionError(String, Error)} a constructor that accepts a
+   *     cause: Users of this class typically expect for instances to have a non-null cause. At the
+   *     moment, you can <i>usually</i> still preserve behavior by passing an explicit {@code null}
+   *     cause. Note, however, that passing an explicit {@code null} cause prevents anyone from
+   *     calling {@link #initCause} later, so it is not quite equivalent to using a constructor that
+   *     omits the cause.
+   */
+  @Deprecated
   protected ExecutionError(@CheckForNull String message) {
     super(message);
   }
 
-  /** Creates a new instance with the given detail message and cause. */
+  /**
+   * Creates a new instance with the given detail message and cause. Prefer to provide a
+   * non-nullable {@code cause}, as many users expect to find one.
+   */
   public ExecutionError(@CheckForNull String message, @CheckForNull Error cause) {
     super(message, cause);
   }
 
-  /** Creates a new instance with the given cause. */
+  /**
+   * Creates a new instance with {@code null} as its detail message and the given cause. Prefer to
+   * provide a non-nullable {@code cause}, as many users expect to find one.
+   */
   public ExecutionError(@CheckForNull Error cause) {
     super(cause);
   }

diff --git a/android/guava/src/com/google/common/util/concurrent/UncheckedExecutionException.java b/android/guava/src/com/google/common/util/concurrent/UncheckedExecutionException.java
@@ -36,25 +36,55 @@
 @ElementTypesAreNonnullByDefault
 public class UncheckedExecutionException extends RuntimeException {
   /*
-   * Ideally, this class would have exposed only constructors that require a non-null cause. We
-   * might try to move in that direction, but there are complications. See
+   * Ideally, this class would have exposed only constructors that require a non-null cause. See
    * https://github.com/jspecify/jspecify-reference-checker/blob/61aafa4ae52594830cfc2d61c8b113009dbdb045/src/main/java/com/google/jspecify/nullness/NullSpecTransfer.java#L789
+   * and https://github.com/jspecify/jspecify/issues/490.
+   *
+   * (Perhaps it should also have required that its cause was a RuntimeException. However, that
+   * would have required that we throw a different kind of exception for wrapping *checked*
+   * exceptions in methods like Futures.getUnchecked and LoadingCache.get.)
    */
 
-  /** Creates a new instance with {@code null} as its detail message. */
+  /**
+   * Creates a new instance with {@code null} as its detail message and no cause.
+   *
+   * @deprecated Prefer {@linkplain UncheckedExecutionException(Throwable)} a constructor that
+   *     accepts a cause: Users of this class typically expect for instances to have a non-null
+   *     cause. At the moment, you can <i>usually</i> still preserve behavior by passing an explicit
+   *     {@code null} cause. Note, however, that passing an explicit {@code null} cause prevents
+   *     anyone from calling {@link #initCause} later, so it is not quite equivalent to using a
+   *     constructor that omits the cause.
+   */
+  @Deprecated
   protected UncheckedExecutionException() {}
 
-  /** Creates a new instance with the given detail message. */
+  /**
+   * Creates a new instance with the given detail message and no cause.
+   *
+   * @deprecated Prefer {@linkplain UncheckedExecutionException(String, Throwable)} a constructor
+   *     that accepts a cause: Users of this class typically expect for instances to have a non-null
+   *     cause. At the moment, you can <i>usually</i> still preserve behavior by passing an explicit
+   *     {@code null} cause. Note, however, that passing an explicit {@code null} cause prevents
+   *     anyone from calling {@link #initCause} later, so it is not quite equivalent to using a
+   *     constructor that omits the cause.
+   */
+  @Deprecated
   protected UncheckedExecutionException(@CheckForNull String message) {
     super(message);
   }
 
-  /** Creates a new instance with the given detail message and cause. */
+  /**
+   * Creates a new instance with the given detail message and cause. Prefer to provide a
+   * non-nullable {@code cause}, as many users expect to find one.
+   */
   public UncheckedExecutionException(@CheckForNull String message, @CheckForNull Throwable cause) {
     super(message, cause);
   }
 
-  /** Creates a new instance with the given cause. */
+  /**
+   * Creates a new instance with {@code null} as its detail message and the given cause. Prefer to
+   * provide a non-nullable {@code cause}, as many users expect to find one.
+   */
   public UncheckedExecutionException(@CheckForNull Throwable cause) {
     super(cause);
   }

diff --git a/guava/src/com/google/common/util/concurrent/ExecutionError.java b/guava/src/com/google/common/util/concurrent/ExecutionError.java
@@ -31,25 +31,55 @@
 @ElementTypesAreNonnullByDefault
 public class ExecutionError extends Error {
   /*
-   * Ideally, this class would have exposed only constructors that require a non-null cause. We
-   * might try to move in that direction, but there are complications. See
+   * Ideally, this class would have exposed only constructors that require a non-null cause. See
    * https://github.com/jspecify/jspecify-reference-checker/blob/61aafa4ae52594830cfc2d61c8b113009dbdb045/src/main/java/com/google/jspecify/nullness/NullSpecTransfer.java#L789
+   * and https://github.com/jspecify/jspecify/issues/490.
+   *
+   * (That would also have ensured that its cause was always an Error, rather than possibly another
+   * kind of Throwable that was later passed to initCause. Then we could have declared the override
+   * `public final Error getCause()`.)
    */
 
-  /** Creates a new instance with {@code null} as its detail message. */
+  /**
+   * Creates a new instance with {@code null} as its detail message and no cause.
+   *
+   * @deprecated Prefer {@linkplain ExecutionError(Error)} a constructor that accepts a cause: Users
+   *     of this class typically expect for instances to have a non-null cause. At the moment, you
+   *     can <i>usually</i> still preserve behavior by passing an explicit {@code null} cause. Note,
+   *     however, that passing an explicit {@code null} cause prevents anyone from calling {@link
+   *     #initCause} later, so it is not quite equivalent to using a constructor that omits the
+   *     cause.
+   */
+  @Deprecated
   protected ExecutionError() {}
 
-  /** Creates a new instance with the given detail message. */
+  /**
+   * Creates a new instance with the given detail message and no cause.
+   *
+   * @deprecated Prefer {@linkplain ExecutionError(String, Error)} a constructor that accepts a
+   *     cause: Users of this class typically expect for instances to have a non-null cause. At the
+   *     moment, you can <i>usually</i> still preserve behavior by passing an explicit {@code null}
+   *     cause. Note, however, that passing an explicit {@code null} cause prevents anyone from
+   *     calling {@link #initCause} later, so it is not quite equivalent to using a constructor that
+   *     omits the cause.
+   */
+  @Deprecated
   protected ExecutionError(@CheckForNull String message) {
     super(message);
   }
 
-  /** Creates a new instance with the given detail message and cause. */
+  /**
+   * Creates a new instance with the given detail message and cause. Prefer to provide a
+   * non-nullable {@code cause}, as many users expect to find one.
+   */
   public ExecutionError(@CheckForNull String message, @CheckForNull Error cause) {
     super(message, cause);
   }
 
-  /** Creates a new instance with the given cause. */
+  /**
+   * Creates a new instance with {@code null} as its detail message and the given cause. Prefer to
+   * provide a non-nullable {@code cause}, as many users expect to find one.
+   */
   public ExecutionError(@CheckForNull Error cause) {
     super(cause);
   }

diff --git a/guava/src/com/google/common/util/concurrent/UncheckedExecutionException.java b/guava/src/com/google/common/util/concurrent/UncheckedExecutionException.java
@@ -36,25 +36,55 @@
 @ElementTypesAreNonnullByDefault
 public class UncheckedExecutionException extends RuntimeException {
   /*
-   * Ideally, this class would have exposed only constructors that require a non-null cause. We
-   * might try to move in that direction, but there are complications. See
+   * Ideally, this class would have exposed only constructors that require a non-null cause. See
    * https://github.com/jspecify/jspecify-reference-checker/blob/61aafa4ae52594830cfc2d61c8b113009dbdb045/src/main/java/com/google/jspecify/nullness/NullSpecTransfer.java#L789
+   * and https://github.com/jspecify/jspecify/issues/490.
+   *
+   * (Perhaps it should also have required that its cause was a RuntimeException. However, that
+   * would have required that we throw a different kind of exception for wrapping *checked*
+   * exceptions in methods like Futures.getUnchecked and LoadingCache.get.)
    */
 
-  /** Creates a new instance with {@code null} as its detail message. */
+  /**
+   * Creates a new instance with {@code null} as its detail message and no cause.
+   *
+   * @deprecated Prefer {@linkplain UncheckedExecutionException(Throwable)} a constructor that
+   *     accepts a cause: Users of this class typically expect for instances to have a non-null
+   *     cause. At the moment, you can <i>usually</i> still preserve behavior by passing an explicit
+   *     {@code null} cause. Note, however, that passing an explicit {@code null} cause prevents
+   *     anyone from calling {@link #initCause} later, so it is not quite equivalent to using a
+   *     constructor that omits the cause.
+   */
+  @Deprecated
   protected UncheckedExecutionException() {}
 
-  /** Creates a new instance with the given detail message. */
+  /**
+   * Creates a new instance with the given detail message and no cause.
+   *
+   * @deprecated Prefer {@linkplain UncheckedExecutionException(String, Throwable)} a constructor
+   *     that accepts a cause: Users of this class typically expect for instances to have a non-null
+   *     cause. At the moment, you can <i>usually</i> still preserve behavior by passing an explicit
+   *     {@code null} cause. Note, however, that passing an explicit {@code null} cause prevents
+   *     anyone from calling {@link #initCause} later, so it is not quite equivalent to using a
+   *     constructor that omits the cause.
+   */
+  @Deprecated
   protected UncheckedExecutionException(@CheckForNull String message) {
     super(message);
   }
 
-  /** Creates a new instance with the given detail message and cause. */
+  /**
+   * Creates a new instance with the given detail message and cause. Prefer to provide a
+   * non-nullable {@code cause}, as many users expect to find one.
+   */
   public UncheckedExecutionException(@CheckForNull String message, @CheckForNull Throwable cause) {
     super(message, cause);
   }
 
-  /** Creates a new instance with the given cause. */
+  /**
+   * Creates a new instance with {@code null} as its detail message and the given cause. Prefer to
+   * provide a non-nullable {@code cause}, as many users expect to find one.
+   */
   public UncheckedExecutionException(@CheckForNull Throwable cause) {
     super(cause);
   }