Ensure that the CompletableFuture returned by getCompletionSuggestions is always completed.

Prior to this commit, if one CompletableFuture failed to complete, the returned future would never be completed because thenRun required all futures to complete sucessfully. Code that relies on such a completion would then be left waiting ad infinitum, with the potential for a deadlock occurring if join or get is used on the future. In Minecraft, this results in client completions failing to be displayed if more than one node asks the server for results, because all but one are cancelled - an exceptional condition!

The solution I've gone for here is to just return the results of futures that completed sucessfully.

* Test added to verify behaviour when a node fails to return a completion.

src/main/java/com/mojang/brigadier/CommandDispatcher.java

@@ -599,16 +599,15 @@ public class CommandDispatcher<S> {
             futures[i++] = future;
         }
 
-        final CompletableFuture<Suggestions> result = new CompletableFuture<>();
-        CompletableFuture.allOf(futures).thenRun(() -> {
+        return CompletableFuture.allOf(futures).handle((voidResult, exception) -> {
             final List<Suggestions> suggestions = new ArrayList<>();
             for (final CompletableFuture<Suggestions> future : futures) {
-                suggestions.add(future.join());
+                if (!future.isCompletedExceptionally()) {
+                    suggestions.add(future.join());
+                }
             }
-            result.complete(Suggestions.merge(fullInput, suggestions));
+            return Suggestions.merge(fullInput, suggestions);
         });
-
-        return result;
     }
 
     /**

src/test/java/com/mojang/brigadier/CommandDispatcherTest.java

@@ -7,6 +7,8 @@ import com.google.common.collect.Lists;
 import com.mojang.brigadier.context.CommandContext;
 import com.mojang.brigadier.context.CommandContextBuilder;
 import com.mojang.brigadier.exceptions.CommandSyntaxException;
+import com.mojang.brigadier.suggestion.Suggestion;
+import com.mojang.brigadier.suggestion.Suggestions;
 import com.mojang.brigadier.tree.LiteralCommandNode;
 import org.junit.Before;
 import org.junit.Test;
@@ -32,6 +34,10 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.util.Collection;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.Collectors;
+
 @RunWith(MockitoJUnitRunner.class)
 public class CommandDispatcherTest {
     private CommandDispatcher<Object> subject;
@@ -400,4 +406,24 @@ public class CommandDispatcherTest {
     public void testFindNodeDoesntExist() {
         assertThat(subject.findNode(Lists.newArrayList("foo", "bar")), is(nullValue()));
     }
+
+    @SuppressWarnings("unchecked")
+    @Test
+    public void testCompletionWithErroredFutureReturnsCompletedFuture() {
+        final LiteralCommandNode<Object> bar = literal("bar").build();
+        final LiteralCommandNode<Object> baz = mock(LiteralCommandNode.class);
+        when(baz.getLiteral()).thenReturn("baz");
+        when(baz.listSuggestions(any(), any())).thenAnswer(x -> {
+            final CompletableFuture<Suggestions> future = new CompletableFuture<>();
+            future.completeExceptionally(new IllegalArgumentException());
+            return future;
+        });
+        subject.register(literal("foo").then(bar).then(baz));
+
+        final ParseResults<Object> parseResults = subject.parse("foo b", source);
+        final Suggestions suggestions = subject.getCompletionSuggestions(parseResults).join();
+        final Collection<String> suggestionCollection = suggestions.getList().stream().map(Suggestion::getText).collect(Collectors.toList());
+        assertThat(Lists.newArrayList("bar"), is(suggestionCollection));
+    }
+
 }