Clean up location parser & suggestions, add more tests (#61)

* Add failing test for location flag suggestions

* Adjust LocationParser

* Add more location suggestion tests

* fix formatting

* remove test command

Author: jpenilla

cloud-bukkit/src/main/java/org/incendo/cloud/bukkit/parser/location/LocationCoordinateParser.java

@@ -61,11 +61,8 @@ public final class LocationCoordinateParser<C> implements ArgumentParser<C, Loca
         try {
             final boolean empty = commandInput.peekString().isEmpty() || commandInput.peek() == ' ';
             coordinate = empty ? 0 : commandInput.readDouble();
-
-            // You can have a prefix without a number, in which case we wouldn't consume the
-            // subsequent whitespace. We do it manually.
-            if (commandInput.hasRemainingInput() && commandInput.peek() == ' ') {
-                commandInput.read();
+            if (commandInput.hasRemainingInput()) {
+                commandInput.skipWhitespace();
             }
         } catch (final Exception e) {
             return ArgumentParseResult.failure(new DoubleParser.DoubleParseException(

cloud-bukkit/src/main/java/org/incendo/cloud/bukkit/parser/location/LocationParser.java

@@ -130,7 +130,12 @@ public final class LocationParser<C> implements ArgumentParser<C, Location>, Blo
         } else if (bukkitSender instanceof Entity) {
             originalLocation = ((Entity) bukkitSender).getLocation();
         } else {
-            originalLocation = new Location(Bukkit.getWorlds().get(0), 0, 0, 0);
+            if (Bukkit.getWorlds().isEmpty()) {
+                // For testing
+                originalLocation = new Location(null, 0, 0, 0);
+            } else {
+                originalLocation = new Location(Bukkit.getWorlds().get(0), 0, 0, 0);
+            }
         }
 
         if (((coordinates[0].type() == LocationCoordinateType.LOCAL)
@@ -216,19 +221,33 @@ private static float toRadians(final float degrees) {
             final @NonNull CommandContext<C> commandContext,
             final @NonNull CommandInput input
     ) {
-        final int toSkip = Math.min(components, input.remainingTokens()) - 1;
-        final StringBuilder prefix = new StringBuilder();
-        for (int i = 0; i < toSkip; i++) {
-            prefix.append(input.readStringSkipWhitespace()).append(" ");
+        final CommandInput inputCopy = input.copy();
+
+        int idx = input.cursor();
+        for (int i = 0; i < components; i++) {
+            idx = input.cursor();
+            if (!input.hasRemainingInput(true)) {
+                break;
+            }
+            final ArgumentParseResult<LocationCoordinate> coordinateResult = new LocationCoordinateParser<C>().parse(
+                commandContext,
+                input
+            );
+            if (coordinateResult.failure().isPresent()) {
+                break;
+            }
         }
+        input.cursor(idx);
 
         if (input.hasRemainingInput() && (input.peek() == '~' || input.peek() == '^')) {
-            prefix.append(input.read());
+            input.read();
         }
 
+        final String prefix = inputCopy.difference(input, true);
+
         return IntegerParser.getSuggestions(
-                SUGGESTION_RANGE,
-                input
+            SUGGESTION_RANGE,
+            input
         ).stream().map(string -> prefix + string).collect(Collectors.toList());
     }
 

cloud-bukkit/src/test/java/org/incendo/cloud/bukkit/parser/LocationArgumentTest.java

@@ -23,24 +23,38 @@
 //
 package org.incendo.cloud.bukkit.parser;
 
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
 import org.bukkit.Location;
 import org.bukkit.World;
 import org.bukkit.command.CommandSender;
 import org.bukkit.util.Vector;
 import org.checkerframework.checker.nullness.qual.NonNull;
+import org.incendo.cloud.CommandManager;
+import org.incendo.cloud.bukkit.BukkitCommandContextKeys;
 import org.incendo.cloud.bukkit.parser.location.LocationParser;
 import org.incendo.cloud.bukkit.util.ServerTest;
 import org.incendo.cloud.context.CommandInput;
+import org.incendo.cloud.execution.ExecutionCoordinator;
+import org.incendo.cloud.internal.CommandRegistrationHandler;
 import org.incendo.cloud.parser.ArgumentParseResult;
+import org.incendo.cloud.parser.flag.CommandFlag;
+import org.incendo.cloud.suggestion.Suggestion;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.Mock;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
 import static org.mockito.Mockito.when;
 
@@ -49,6 +63,54 @@ class LocationArgumentTest extends ServerTest {
     @Mock
     private World world;
 
+    @ParameterizedTest
+    @MethodSource
+    void suggestions(final String input, final List<String> expectedSuggestions) {
+        final LocationParser<CommandSender> parser = new LocationParser<>();
+
+        final Set<String> suggestions =
+            StreamSupport.stream(parser.suggestionProvider()
+                    .suggestionsFuture(this.commandContext(), CommandInput.of(input)).join().spliterator(), false)
+                .map(Suggestion::suggestion)
+                .collect(Collectors.toSet());
+
+        assertEquals(new HashSet<>(expectedSuggestions), suggestions);
+    }
+
+    @ParameterizedTest
+    @MethodSource("suggestions")
+    void suggestionsFlag(final String input, final List<String> expectedSuggestions) {
+        final CommandManager<CommandSender> manager = new CommandManager<CommandSender>(
+            ExecutionCoordinator.simpleCoordinator(), CommandRegistrationHandler.nullCommandRegistrationHandler()
+        ) {
+            @Override
+            public boolean hasPermission(@NonNull CommandSender sender, @NonNull String permission) {
+                return true;
+            }
+        };
+        manager.registerCommandPreProcessor(ctx -> ctx.commandContext().store(BukkitCommandContextKeys.BUKKIT_COMMAND_SENDER, ctx.commandContext().sender()));
+        manager.command(
+            manager.commandBuilder("flag")
+                .flag(CommandFlag.builder("loc").withComponent(LocationParser.locationParser()).build())
+        );
+        final Function<String, String> flagSuggestion = s -> "flag --loc " + s;
+        assertEquals(
+            new HashSet<>(expectedSuggestions),
+            manager.suggestionFactory().suggestImmediately(this.commandContext().sender(), flagSuggestion.apply(input))
+                .list().stream().map(Suggestion::suggestion).collect(Collectors.toSet())
+        );
+    }
+
+    static Stream<Arguments> suggestions() {
+        return Stream.of(
+            arguments("", Arrays.asList("0", "1", "2", "3", "4", "5", "6", "7", "8", "9")),
+            arguments("1 ", Arrays.asList("1 0", "1 1", "1 2", "1 3", "1 4", "1 5", "1 6", "1 7", "1 8", "1 9")),
+            arguments("1 1", Arrays.asList("1 10", "1 11", "1 12", "1 13", "1 14", "1 15", "1 16", "1 17", "1 18", "1 19")),
+            arguments("1 1 ", Arrays.asList("1 1 0", "1 1 1", "1 1 2", "1 1 3", "1 1 4", "1 1 5", "1 1 6", "1 1 7", "1 1 8", "1 1 9")),
+            arguments("1 1 1", Arrays.asList("1 1 1", "1 1 10", "1 1 11", "1 1 12", "1 1 13", "1 1 14", "1 1 15", "1 1 16", "1 1 17", "1 1 18", "1 1 19"))
+        );
+    }
+
     @ParameterizedTest
     @MethodSource("Parse_HappyFlow_Success_Source")
     void Parse_HappyFlow_Success(final @NonNull String input, final @NonNull Vector expectedLocation) {
@@ -59,8 +121,8 @@ void Parse_HappyFlow_Success(final @NonNull String input, final @NonNull Vector
 
         // Act
         final ArgumentParseResult<Location> result = parser.parse(
-                this.commandContext(),
-                commandInput
+            this.commandContext(),
+            commandInput
         );
 
         // Assert
@@ -72,28 +134,28 @@ void Parse_HappyFlow_Success(final @NonNull String input, final @NonNull Vector
 
     static @NonNull Stream<@NonNull Arguments> Parse_HappyFlow_Success_Source() {
         return Stream.of(
-                arguments("~ ~ ~", new Vector(0, 0, 0)),
-                arguments("~10 ~10 ~10", new Vector(10, 10, 10)),
-                arguments("~-10 ~-10 ~-10", new Vector(-10, -10, -10)),
-                arguments("^ ^ ^", new Vector(0, 0, 0)),
-                arguments("^0 ^0 ^0", new Vector(0, 0, 0)),
-                arguments("0 0 0", new Vector(0, 0, 0)),
-                arguments("10 10 10", new Vector(10, 10, 10)),
-                arguments("-10 -10 -10", new Vector(-10, -10, -10))
+            arguments("~ ~ ~", new Vector(0, 0, 0)),
+            arguments("~10 ~10 ~10", new Vector(10, 10, 10)),
+            arguments("~-10 ~-10 ~-10", new Vector(-10, -10, -10)),
+            arguments("^ ^ ^", new Vector(0, 0, 0)),
+            arguments("^0 ^0 ^0", new Vector(0, 0, 0)),
+            arguments("0 0 0", new Vector(0, 0, 0)),
+            arguments("10 10 10", new Vector(10, 10, 10)),
+            arguments("-10 -10 -10", new Vector(-10, -10, -10))
         );
     }
 
     @ParameterizedTest
-    @ValueSource(strings = { "0 0", "not a location" })
+    @ValueSource(strings = {"0 0", "not a location"})
     void Parse_InvalidLocation_Failure(final @NonNull String input) {
         // Arrange
         final LocationParser<CommandSender> parser = new LocationParser<>();
         final CommandInput commandInput = CommandInput.of(input);
 
         // Act
         final ArgumentParseResult<Location> result = parser.parse(
-                this.commandContext(),
-                commandInput
+            this.commandContext(),
+            commandInput
         );
 
         // Assert