From 60ba1ab80bb09ecb1cff8618589710b4fea9cab7 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Thu, 20 Feb 2025 09:59:57 -0800 Subject: [PATCH] Reduce scope of suppressions in `Abstract*Future` classes, and make the classes look slightly more similar to one another. RELNOTES=n/a PiperOrigin-RevId: 729152015 --- .../concurrent/AbstractCatchingFuture.java | 62 +++++++++---------- .../concurrent/AbstractTransformFuture.java | 20 +++--- .../concurrent/AbstractCatchingFuture.java | 62 +++++++++---------- .../concurrent/AbstractTransformFuture.java | 20 +++--- 4 files changed, 78 insertions(+), 86 deletions(-) diff --git a/android/guava/src/com/google/common/util/concurrent/AbstractCatchingFuture.java b/android/guava/src/com/google/common/util/concurrent/AbstractCatchingFuture.java index c15f6cb658e2..9482e213eeb4 100644 --- a/android/guava/src/com/google/common/util/concurrent/AbstractCatchingFuture.java +++ b/android/guava/src/com/google/common/util/concurrent/AbstractCatchingFuture.java @@ -34,11 +34,8 @@ /** Implementations of {@code Futures.catching*}. */ @GwtCompatible -@SuppressWarnings({ - // Whenever both tests are cheap and functional, it's faster to use &, | instead of &&, || - "ShortCircuitBoolean", - "nullness", // TODO(b/147136275): Remove once our checker understands & and |. -}) +// Whenever both tests are cheap and functional, it's faster to use &, | instead of &&, || +@SuppressWarnings("ShortCircuitBoolean") abstract class AbstractCatchingFuture< V extends @Nullable Object, X extends Throwable, F, T extends @Nullable Object> extends FluentFuture.TrustedFuture implements Runnable { @@ -47,9 +44,9 @@ abstract class AbstractCatchingFuture< Class exceptionType, Function fallback, Executor executor) { - CatchingFuture future = new CatchingFuture<>(input, exceptionType, fallback); - input.addListener(future, rejectionPropagatingExecutor(executor, future)); - return future; + CatchingFuture output = new CatchingFuture<>(input, exceptionType, fallback); + input.addListener(output, rejectionPropagatingExecutor(executor, output)); + return output; } static ListenableFuture createAsync( @@ -57,9 +54,9 @@ abstract class AbstractCatchingFuture< Class exceptionType, AsyncFunction fallback, Executor executor) { - AsyncCatchingFuture future = new AsyncCatchingFuture<>(input, exceptionType, fallback); - input.addListener(future, rejectionPropagatingExecutor(executor, future)); - return future; + AsyncCatchingFuture output = new AsyncCatchingFuture<>(input, exceptionType, fallback); + input.addListener(output, rejectionPropagatingExecutor(executor, output)); + return output; } /* @@ -78,6 +75,7 @@ abstract class AbstractCatchingFuture< } @Override + @SuppressWarnings("nullness") // TODO(b/147136275): Remove once our checker understands & and |. public final void run() { @RetainedLocalRef ListenableFuture localInputFuture = inputFuture; @RetainedLocalRef Class localExceptionType = exceptionType; @@ -148,6 +146,24 @@ public final void run() { setResult(fallbackResult); } + /** Template method for subtypes to actually run the fallback. */ + @ForOverride + @ParametricNullness + abstract T doFallback(F fallback, X throwable) throws Exception; + + /** Template method for subtypes to actually set the result. */ + @ForOverride + abstract void setResult(@ParametricNullness T result); + + @Override + protected final void afterDone() { + @RetainedLocalRef ListenableFuture localInputFuture = inputFuture; + maybePropagateCancellationTo(localInputFuture); + this.inputFuture = null; + this.exceptionType = null; + this.fallback = null; + } + @Override protected @Nullable String pendingToString() { @RetainedLocalRef ListenableFuture localInputFuture = inputFuture; @@ -171,24 +187,6 @@ public final void run() { return null; } - /** Template method for subtypes to actually run the fallback. */ - @ForOverride - @ParametricNullness - abstract T doFallback(F fallback, X throwable) throws Exception; - - /** Template method for subtypes to actually set the result. */ - @ForOverride - abstract void setResult(@ParametricNullness T result); - - @Override - protected final void afterDone() { - @RetainedLocalRef ListenableFuture localInputFuture = inputFuture; - maybePropagateCancellationTo(localInputFuture); - this.inputFuture = null; - this.exceptionType = null; - this.fallback = null; - } - /** * An {@link AbstractCatchingFuture} that delegates to an {@link AsyncFunction} and {@link * #setFuture(ListenableFuture)}. @@ -206,13 +204,13 @@ private static final class AsyncCatchingFuture doFallback( AsyncFunction fallback, X cause) throws Exception { - ListenableFuture replacement = fallback.apply(cause); + ListenableFuture output = fallback.apply(cause); checkNotNull( - replacement, + output, "AsyncFunction.apply returned null instead of a Future. " + "Did you mean to return immediateFuture(null)? %s", fallback); - return replacement; + return output; } @Override diff --git a/android/guava/src/com/google/common/util/concurrent/AbstractTransformFuture.java b/android/guava/src/com/google/common/util/concurrent/AbstractTransformFuture.java index 5a078eb48901..a455f7d24140 100644 --- a/android/guava/src/com/google/common/util/concurrent/AbstractTransformFuture.java +++ b/android/guava/src/com/google/common/util/concurrent/AbstractTransformFuture.java @@ -31,11 +31,8 @@ /** Implementations of {@code Futures.transform*}. */ @GwtCompatible -@SuppressWarnings({ - // Whenever both tests are cheap and functional, it's faster to use &, | instead of &&, || - "ShortCircuitBoolean", - "nullness", // TODO(b/147136275): Remove once our checker understands & and |. -}) +// Whenever both tests are cheap and functional, it's faster to use &, | instead of &&, || +@SuppressWarnings("ShortCircuitBoolean") abstract class AbstractTransformFuture< I extends @Nullable Object, O extends @Nullable Object, F, T extends @Nullable Object> extends FluentFuture.TrustedFuture implements Runnable { @@ -43,7 +40,6 @@ abstract class AbstractTransformFuture< ListenableFuture input, AsyncFunction function, Executor executor) { - checkNotNull(executor); AsyncTransformFuture output = new AsyncTransformFuture<>(input, function); input.addListener(output, rejectionPropagatingExecutor(executor, output)); return output; @@ -51,7 +47,6 @@ abstract class AbstractTransformFuture< static ListenableFuture create( ListenableFuture input, Function function, Executor executor) { - checkNotNull(function); TransformFuture output = new TransformFuture<>(input, function); input.addListener(output, rejectionPropagatingExecutor(executor, output)); return output; @@ -70,7 +65,10 @@ abstract class AbstractTransformFuture< } @Override - @SuppressWarnings("CatchingUnchecked") // sneaky checked exception + @SuppressWarnings({ + "CatchingUnchecked", // sneaky checked exception + "nullness", // TODO(b/147136275): Remove once our checker understands & and |. + }) public final void run() { @RetainedLocalRef ListenableFuture localInputFuture = inputFuture; @RetainedLocalRef F localFunction = function; @@ -225,13 +223,13 @@ private static final class AsyncTransformFuture< ListenableFuture doTransform( AsyncFunction function, @ParametricNullness I input) throws Exception { - ListenableFuture outputFuture = function.apply(input); + ListenableFuture output = function.apply(input); checkNotNull( - outputFuture, + output, "AsyncFunction.apply returned null instead of a Future. " + "Did you mean to return immediateFuture(null)? %s", function); - return outputFuture; + return output; } @Override diff --git a/guava/src/com/google/common/util/concurrent/AbstractCatchingFuture.java b/guava/src/com/google/common/util/concurrent/AbstractCatchingFuture.java index c15f6cb658e2..9482e213eeb4 100644 --- a/guava/src/com/google/common/util/concurrent/AbstractCatchingFuture.java +++ b/guava/src/com/google/common/util/concurrent/AbstractCatchingFuture.java @@ -34,11 +34,8 @@ /** Implementations of {@code Futures.catching*}. */ @GwtCompatible -@SuppressWarnings({ - // Whenever both tests are cheap and functional, it's faster to use &, | instead of &&, || - "ShortCircuitBoolean", - "nullness", // TODO(b/147136275): Remove once our checker understands & and |. -}) +// Whenever both tests are cheap and functional, it's faster to use &, | instead of &&, || +@SuppressWarnings("ShortCircuitBoolean") abstract class AbstractCatchingFuture< V extends @Nullable Object, X extends Throwable, F, T extends @Nullable Object> extends FluentFuture.TrustedFuture implements Runnable { @@ -47,9 +44,9 @@ abstract class AbstractCatchingFuture< Class exceptionType, Function fallback, Executor executor) { - CatchingFuture future = new CatchingFuture<>(input, exceptionType, fallback); - input.addListener(future, rejectionPropagatingExecutor(executor, future)); - return future; + CatchingFuture output = new CatchingFuture<>(input, exceptionType, fallback); + input.addListener(output, rejectionPropagatingExecutor(executor, output)); + return output; } static ListenableFuture createAsync( @@ -57,9 +54,9 @@ abstract class AbstractCatchingFuture< Class exceptionType, AsyncFunction fallback, Executor executor) { - AsyncCatchingFuture future = new AsyncCatchingFuture<>(input, exceptionType, fallback); - input.addListener(future, rejectionPropagatingExecutor(executor, future)); - return future; + AsyncCatchingFuture output = new AsyncCatchingFuture<>(input, exceptionType, fallback); + input.addListener(output, rejectionPropagatingExecutor(executor, output)); + return output; } /* @@ -78,6 +75,7 @@ abstract class AbstractCatchingFuture< } @Override + @SuppressWarnings("nullness") // TODO(b/147136275): Remove once our checker understands & and |. public final void run() { @RetainedLocalRef ListenableFuture localInputFuture = inputFuture; @RetainedLocalRef Class localExceptionType = exceptionType; @@ -148,6 +146,24 @@ public final void run() { setResult(fallbackResult); } + /** Template method for subtypes to actually run the fallback. */ + @ForOverride + @ParametricNullness + abstract T doFallback(F fallback, X throwable) throws Exception; + + /** Template method for subtypes to actually set the result. */ + @ForOverride + abstract void setResult(@ParametricNullness T result); + + @Override + protected final void afterDone() { + @RetainedLocalRef ListenableFuture localInputFuture = inputFuture; + maybePropagateCancellationTo(localInputFuture); + this.inputFuture = null; + this.exceptionType = null; + this.fallback = null; + } + @Override protected @Nullable String pendingToString() { @RetainedLocalRef ListenableFuture localInputFuture = inputFuture; @@ -171,24 +187,6 @@ public final void run() { return null; } - /** Template method for subtypes to actually run the fallback. */ - @ForOverride - @ParametricNullness - abstract T doFallback(F fallback, X throwable) throws Exception; - - /** Template method for subtypes to actually set the result. */ - @ForOverride - abstract void setResult(@ParametricNullness T result); - - @Override - protected final void afterDone() { - @RetainedLocalRef ListenableFuture localInputFuture = inputFuture; - maybePropagateCancellationTo(localInputFuture); - this.inputFuture = null; - this.exceptionType = null; - this.fallback = null; - } - /** * An {@link AbstractCatchingFuture} that delegates to an {@link AsyncFunction} and {@link * #setFuture(ListenableFuture)}. @@ -206,13 +204,13 @@ private static final class AsyncCatchingFuture doFallback( AsyncFunction fallback, X cause) throws Exception { - ListenableFuture replacement = fallback.apply(cause); + ListenableFuture output = fallback.apply(cause); checkNotNull( - replacement, + output, "AsyncFunction.apply returned null instead of a Future. " + "Did you mean to return immediateFuture(null)? %s", fallback); - return replacement; + return output; } @Override diff --git a/guava/src/com/google/common/util/concurrent/AbstractTransformFuture.java b/guava/src/com/google/common/util/concurrent/AbstractTransformFuture.java index 5a078eb48901..a455f7d24140 100644 --- a/guava/src/com/google/common/util/concurrent/AbstractTransformFuture.java +++ b/guava/src/com/google/common/util/concurrent/AbstractTransformFuture.java @@ -31,11 +31,8 @@ /** Implementations of {@code Futures.transform*}. */ @GwtCompatible -@SuppressWarnings({ - // Whenever both tests are cheap and functional, it's faster to use &, | instead of &&, || - "ShortCircuitBoolean", - "nullness", // TODO(b/147136275): Remove once our checker understands & and |. -}) +// Whenever both tests are cheap and functional, it's faster to use &, | instead of &&, || +@SuppressWarnings("ShortCircuitBoolean") abstract class AbstractTransformFuture< I extends @Nullable Object, O extends @Nullable Object, F, T extends @Nullable Object> extends FluentFuture.TrustedFuture implements Runnable { @@ -43,7 +40,6 @@ abstract class AbstractTransformFuture< ListenableFuture input, AsyncFunction function, Executor executor) { - checkNotNull(executor); AsyncTransformFuture output = new AsyncTransformFuture<>(input, function); input.addListener(output, rejectionPropagatingExecutor(executor, output)); return output; @@ -51,7 +47,6 @@ abstract class AbstractTransformFuture< static ListenableFuture create( ListenableFuture input, Function function, Executor executor) { - checkNotNull(function); TransformFuture output = new TransformFuture<>(input, function); input.addListener(output, rejectionPropagatingExecutor(executor, output)); return output; @@ -70,7 +65,10 @@ abstract class AbstractTransformFuture< } @Override - @SuppressWarnings("CatchingUnchecked") // sneaky checked exception + @SuppressWarnings({ + "CatchingUnchecked", // sneaky checked exception + "nullness", // TODO(b/147136275): Remove once our checker understands & and |. + }) public final void run() { @RetainedLocalRef ListenableFuture localInputFuture = inputFuture; @RetainedLocalRef F localFunction = function; @@ -225,13 +223,13 @@ private static final class AsyncTransformFuture< ListenableFuture doTransform( AsyncFunction function, @ParametricNullness I input) throws Exception { - ListenableFuture outputFuture = function.apply(input); + ListenableFuture output = function.apply(input); checkNotNull( - outputFuture, + output, "AsyncFunction.apply returned null instead of a Future. " + "Did you mean to return immediateFuture(null)? %s", function); - return outputFuture; + return output; } @Override