fix invalid suggestions when parsing of argument fails (#401)

This commit is contained in:
Pasqual Koschmieder 2022-11-28 19:51:36 +01:00 committed by Jason
parent b19ec931cb
commit 72f105014a
2 changed files with 70 additions and 3 deletions

View file

@ -659,9 +659,22 @@ public final class CommandTree<C> {
// START: Parsing // START: Parsing
commandContext.setCurrentArgument(child.getValue()); commandContext.setCurrentArgument(child.getValue());
final ArgumentParseResult<?> result = child.getValue().getParser().parse(commandContext, commandQueue); final ArgumentParseResult<?> result = child.getValue().getParser().parse(commandContext, commandQueue);
if (result.getParsedValue().isPresent() && !commandQueue.isEmpty()) { final Optional<?> parsedValue = result.getParsedValue();
commandContext.store(child.getValue().getName(), result.getParsedValue().get()); final boolean parseSuccess = parsedValue.isPresent();
if (parseSuccess && !commandQueue.isEmpty()) {
// the current argument at the position is parsable and there are more arguments following
commandContext.store(child.getValue().getName(), parsedValue.get());
return this.getSuggestions(commandContext, commandQueue, child); return this.getSuggestions(commandContext, commandQueue, child);
} else if (!parseSuccess && commandQueueOriginal.size() > 1) {
// at this point there should normally be no need to reset the command queue as we expect
// users to only take out an argument if the parse succeeded. Just to be sure we reset anyway
commandQueue.clear();
commandQueue.addAll(commandQueueOriginal);
// there are more arguments following but the current argument isn't matching - there
// is no need to collect any further suggestions
return Collections.emptyList();
} }
// END: Parsing // END: Parsing
} }
@ -670,6 +683,13 @@ public final class CommandTree<C> {
commandQueue.clear(); commandQueue.clear();
commandQueue.addAll(commandQueueOriginal); commandQueue.addAll(commandQueueOriginal);
if (!preParseSuccess && commandQueue.size() > 1) {
// The preprocessor denied the argument, and there are more arguments following the current one
// Therefore we shouldn't list the suggestions of the current argument, as clearly the suggestions of
// one of the following arguments is requested
return Collections.emptyList();
}
// Fallback: use suggestion provider of argument // Fallback: use suggestion provider of argument
commandContext.setCurrentArgument(child.getValue()); commandContext.setCurrentArgument(child.getValue());
return child.getValue().getSuggestionsProvider().apply(commandContext, this.stringOrEmpty(commandQueue.peek())); return child.getValue().getSuggestionsProvider().apply(commandContext, this.stringOrEmpty(commandQueue.peek()));

View file

@ -24,6 +24,7 @@
package cloud.commandframework; package cloud.commandframework;
import cloud.commandframework.arguments.compound.ArgumentTriplet; import cloud.commandframework.arguments.compound.ArgumentTriplet;
import cloud.commandframework.arguments.parser.ArgumentParseResult;
import cloud.commandframework.arguments.standard.BooleanArgument; import cloud.commandframework.arguments.standard.BooleanArgument;
import cloud.commandframework.arguments.standard.EnumArgument; import cloud.commandframework.arguments.standard.EnumArgument;
import cloud.commandframework.arguments.standard.IntegerArgument; import cloud.commandframework.arguments.standard.IntegerArgument;
@ -128,6 +129,18 @@ public class CommandSuggestionsTest {
manager.command(manager.commandBuilder("literal_with_variable") manager.command(manager.commandBuilder("literal_with_variable")
.literal("vici") .literal("vici")
.literal("later")); .literal("later"));
manager.command(manager.commandBuilder("cmd_with_multiple_args")
.argument(IntegerArgument.<TestCommandSender>of("number").addPreprocessor((ctx, input) -> {
String argument = input.peek();
if (argument == null || !argument.equals("1024")) {
return ArgumentParseResult.success(true);
} else {
return ArgumentParseResult.failure(new NullPointerException());
}
}))
.argument(EnumArgument.of(TestEnum.class, "enum"))
.literal("world"));
} }
@Test @Test
@ -378,6 +391,7 @@ public class CommandSuggestionsTest {
Assertions.assertEquals(Collections.singletonList("literal"), suggestions9); Assertions.assertEquals(Collections.singletonList("literal"), suggestions9);
} }
@Test
void testLiteralWithVariable() { void testLiteralWithVariable() {
final String input = "literal_with_variable "; final String input = "literal_with_variable ";
final List<String> suggestions = manager.suggest(new TestCommandSender(), input); final List<String> suggestions = manager.suggest(new TestCommandSender(), input);
@ -390,7 +404,7 @@ public class CommandSuggestionsTest {
Assertions.assertEquals(Arrays.asList("vici", "vidi"), suggestions3); Assertions.assertEquals(Arrays.asList("vici", "vidi"), suggestions3);
final String input4 = "literal_with_variable vidi"; final String input4 = "literal_with_variable vidi";
final List<String> suggestions4 = manager.suggest(new TestCommandSender(), input4); final List<String> suggestions4 = manager.suggest(new TestCommandSender(), input4);
Assertions.assertEquals(Collections.emptyList(), suggestions4); Assertions.assertEquals(Collections.singletonList("vidi"), suggestions4);
final String input5 = "literal_with_variable vidi "; final String input5 = "literal_with_variable vidi ";
final List<String> suggestions5 = manager.suggest(new TestCommandSender(), input5); final List<String> suggestions5 = manager.suggest(new TestCommandSender(), input5);
Assertions.assertEquals(Collections.singletonList("now"), suggestions5); Assertions.assertEquals(Collections.singletonList("now"), suggestions5);
@ -399,6 +413,39 @@ public class CommandSuggestionsTest {
Assertions.assertEquals(Collections.singletonList("later"), suggestions6); Assertions.assertEquals(Collections.singletonList("later"), suggestions6);
} }
@Test
void testInvalidArgumentShouldNotCauseFurtherCompletion() {
// pass preprocess
final String input = "cmd_with_multiple_args 512 ";
final List<String> suggestions = manager.suggest(new TestCommandSender(), input);
Assertions.assertEquals(Arrays.asList("foo", "bar"), suggestions);
final String input2 = "cmd_with_multiple_args 512 BAR ";
final List<String> suggestions2 = manager.suggest(new TestCommandSender(), input2);
Assertions.assertEquals(Collections.singletonList("world"), suggestions2);
final String input3 = "cmd_with_multiple_args test ";
final List<String> suggestions3 = manager.suggest(new TestCommandSender(), input3);
Assertions.assertEquals(Collections.emptyList(), suggestions3);
final String input4 = "cmd_with_multiple_args 512 f";
final List<String> suggestions4 = manager.suggest(new TestCommandSender(), input4);
Assertions.assertEquals(Collections.singletonList("foo"), suggestions4);
final String input5 = "cmd_with_multiple_args world f";
final List<String> suggestions5 = manager.suggest(new TestCommandSender(), input5);
Assertions.assertEquals(Collections.emptyList(), suggestions5);
// trigger preprocess fail
final String input6 = "cmd_with_multiple_args 1024";
final List<String> suggestions6 = manager.suggest(new TestCommandSender(), input6);
Assertions.assertEquals(11, suggestions6.size());
final String input7 = "cmd_with_multiple_args 1024 ";
final List<String> suggestions7 = manager.suggest(new TestCommandSender(), input7);
Assertions.assertEquals(Collections.emptyList(), suggestions7);
final String input8 = "cmd_with_multiple_args 1024 f";
final List<String> suggestions8 = manager.suggest(new TestCommandSender(), input8);
Assertions.assertEquals(Collections.emptyList(), suggestions8);
final String input9 = "cmd_with_multiple_args 1024 foo w";
final List<String> suggestions9 = manager.suggest(new TestCommandSender(), input9);
Assertions.assertEquals(Collections.emptyList(), suggestions9);
}
@Test @Test
void testFlagYieldingGreedyStringFollowedByFlagArgument() { void testFlagYieldingGreedyStringFollowedByFlagArgument() {
// Arrange // Arrange