From c9e7d2d4bd4913041b0fb25a12b322642c7fe887 Mon Sep 17 00:00:00 2001
From: Andrew Ash <andrew@andrewash.com>
Date: Tue, 29 Aug 2023 11:07:49 -0700
Subject: [PATCH] Add DoNotCall annotation to deprecated+buggy methods in
 MoreStreams (#213)

---
 changelog/@unreleased/pr-213.v2.yml           |  5 +++++
 streams/build.gradle                          |  2 ++
 .../palantir/common/streams/MoreStreams.java  | 19 +++++++++++++++----
 .../common/streams/MoreStreamsTests.java      |  2 ++
 4 files changed, 24 insertions(+), 4 deletions(-)
 create mode 100644 changelog/@unreleased/pr-213.v2.yml

diff --git a/changelog/@unreleased/pr-213.v2.yml b/changelog/@unreleased/pr-213.v2.yml
new file mode 100644
index 0000000..cb2add5
--- /dev/null
+++ b/changelog/@unreleased/pr-213.v2.yml
@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Add DoNotCall annotation to deprecated+buggy methods in MoreStreams
+  links:
+  - https://github.com/palantir/streams/pull/213
diff --git a/streams/build.gradle b/streams/build.gradle
index b5fa3b8..fa885fa 100644
--- a/streams/build.gradle
+++ b/streams/build.gradle
@@ -3,6 +3,8 @@ apply plugin: 'com.palantir.external-publish-jar'
 dependencies {
     api 'com.google.guava:guava'
 
+    implementation 'com.google.errorprone:error_prone_annotations'
+
     testImplementation 'org.assertj:assertj-core'
     testImplementation 'org.junit.jupiter:junit-jupiter'
     testImplementation 'org.junit.jupiter:junit-jupiter-api'
diff --git a/streams/src/main/java/com/palantir/common/streams/MoreStreams.java b/streams/src/main/java/com/palantir/common/streams/MoreStreams.java
index 9aaf1ae..a4f77b2 100644
--- a/streams/src/main/java/com/palantir/common/streams/MoreStreams.java
+++ b/streams/src/main/java/com/palantir/common/streams/MoreStreams.java
@@ -17,6 +17,7 @@
 
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
+import com.google.errorprone.annotations.DoNotCall;
 import com.palantir.common.streams.BufferingSpliterator.InCompletionOrder;
 import com.palantir.common.streams.BufferingSpliterator.InSourceOrder;
 import java.util.Iterator;
@@ -41,6 +42,7 @@ public final class MoreStreams {
      * @deprecated This function provides no guarantees, maxParallelism is
      * ignored in many cases (e.g. flatmap has been called).
      */
+    @DoNotCall("Use the other inCompletionOrder overload instead")
     @Deprecated
     public static <T, F extends ListenableFuture<T>> Stream<F> inCompletionOrder(
             Stream<F> futures, int maxParallelism) {
@@ -51,8 +53,13 @@ public static <T, F extends ListenableFuture<T>> Stream<F> inCompletionOrder(
     }
 
     /**
-     * A convenient variant of {@link #inCompletionOrder(Stream, int)} in which the user passes in a
-     * function and an executor to run it on.
+     * Given a stream of arguments and a Function mapper, this function will return a blocking stream of the completed
+     * futures in completion order, looking at most {@code maxParallelism} arguments ahead in the stream.
+     *
+     * The caller is required to pass in an executor to run the mapper function on.
+     *
+     * Note: the resulting stream may contain results in a different order than the input arguments. To receive results
+     * in the same order as input arguments, use {@link #blockingStreamWithParallelism(Stream, Function, Executor, int)}.
      */
     public static <U, V> Stream<V> inCompletionOrder(
             Stream<U> arguments, Function<U, V> mapper, Executor executor, int maxParallelism) {
@@ -73,6 +80,7 @@ public static <U, V> Stream<V> inCompletionOrder(
      * @deprecated This function provides no guarantees, maxParallelism is
      * ignored in many cases (e.g. flatmap has been called).
      */
+    @DoNotCall("Use the other blockingStreamWithParallelism overload instead")
     @Deprecated
     public static <T, F extends ListenableFuture<T>> Stream<F> blockingStreamWithParallelism(
             Stream<F> futures, int maxParallelism) {
@@ -84,8 +92,11 @@ public static <T, F extends ListenableFuture<T>> Stream<F> blockingStreamWithPar
     }
 
     /**
-     * A convenient variant of {@link #blockingStreamWithParallelism(Stream, int)} in which the user passes in a
-     * function and an executor to run it on.
+     * Given a stream of arguments and a Function mapper, this function will return a blocking stream that waits for
+     * each future to complete before returning it, but which looks ahead {@code maxParallelism} arguments to ensure a
+     * fixed parallelism rate.
+     *
+     * The caller is required to pass in an executor to run the mapper function on.
      */
     public static <U, V> Stream<V> blockingStreamWithParallelism(
             Stream<U> arguments, Function<U, V> mapper, Executor executor, int maxParallelism) {
diff --git a/streams/src/test/java/com/palantir/common/streams/MoreStreamsTests.java b/streams/src/test/java/com/palantir/common/streams/MoreStreamsTests.java
index bc8b992..b8ca5bc 100644
--- a/streams/src/test/java/com/palantir/common/streams/MoreStreamsTests.java
+++ b/streams/src/test/java/com/palantir/common/streams/MoreStreamsTests.java
@@ -78,12 +78,14 @@ public void before() {
         stream = StreamSupport.stream(spliterator, false);
     }
 
+    @SuppressWarnings("DoNotCall")
     @Test
     public void testInCompletionOrder_future() {
         Stream<SettableFuture<String>> completedFutureStream = MoreStreams.inCompletionOrder(stream, 3);
         assertThat(completedFutureStream).containsExactly(secondInSource, firstInSource);
     }
 
+    @SuppressWarnings("DoNotCall")
     @Test
     public void testBlockingStreamWithParallelism_future() {
         Stream<SettableFuture<String>> completedFutureStream = MoreStreams.blockingStreamWithParallelism(stream, 3);