From 72f105014af1d33f93ef2a247f55d04562e1a4e2 Mon Sep 17 00:00:00 2001 From: Pasqual Koschmieder Date: Mon, 28 Nov 2022 19:51:36 +0100 Subject: [PATCH] fix invalid suggestions when parsing of argument fails (#401) --- .../cloud/commandframework/CommandTree.java | 24 ++++++++- .../CommandSuggestionsTest.java | 49 ++++++++++++++++++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/cloud-core/src/main/java/cloud/commandframework/CommandTree.java b/cloud-core/src/main/java/cloud/commandframework/CommandTree.java index 8b901e49..3194db03 100644 --- a/cloud-core/src/main/java/cloud/commandframework/CommandTree.java +++ b/cloud-core/src/main/java/cloud/commandframework/CommandTree.java @@ -659,9 +659,22 @@ public final class CommandTree { // START: Parsing commandContext.setCurrentArgument(child.getValue()); final ArgumentParseResult result = child.getValue().getParser().parse(commandContext, commandQueue); - if (result.getParsedValue().isPresent() && !commandQueue.isEmpty()) { - commandContext.store(child.getValue().getName(), result.getParsedValue().get()); + final Optional parsedValue = result.getParsedValue(); + 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); + } 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 } @@ -670,6 +683,13 @@ public final class CommandTree { commandQueue.clear(); 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 commandContext.setCurrentArgument(child.getValue()); return child.getValue().getSuggestionsProvider().apply(commandContext, this.stringOrEmpty(commandQueue.peek())); diff --git a/cloud-core/src/test/java/cloud/commandframework/CommandSuggestionsTest.java b/cloud-core/src/test/java/cloud/commandframework/CommandSuggestionsTest.java index 1753a5c3..3f711fc6 100644 --- a/cloud-core/src/test/java/cloud/commandframework/CommandSuggestionsTest.java +++ b/cloud-core/src/test/java/cloud/commandframework/CommandSuggestionsTest.java @@ -24,6 +24,7 @@ package cloud.commandframework; import cloud.commandframework.arguments.compound.ArgumentTriplet; +import cloud.commandframework.arguments.parser.ArgumentParseResult; import cloud.commandframework.arguments.standard.BooleanArgument; import cloud.commandframework.arguments.standard.EnumArgument; import cloud.commandframework.arguments.standard.IntegerArgument; @@ -128,6 +129,18 @@ public class CommandSuggestionsTest { manager.command(manager.commandBuilder("literal_with_variable") .literal("vici") .literal("later")); + + manager.command(manager.commandBuilder("cmd_with_multiple_args") + .argument(IntegerArgument.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 @@ -378,6 +391,7 @@ public class CommandSuggestionsTest { Assertions.assertEquals(Collections.singletonList("literal"), suggestions9); } + @Test void testLiteralWithVariable() { final String input = "literal_with_variable "; final List suggestions = manager.suggest(new TestCommandSender(), input); @@ -390,7 +404,7 @@ public class CommandSuggestionsTest { Assertions.assertEquals(Arrays.asList("vici", "vidi"), suggestions3); final String input4 = "literal_with_variable vidi"; final List 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 List suggestions5 = manager.suggest(new TestCommandSender(), input5); Assertions.assertEquals(Collections.singletonList("now"), suggestions5); @@ -399,6 +413,39 @@ public class CommandSuggestionsTest { Assertions.assertEquals(Collections.singletonList("later"), suggestions6); } + @Test + void testInvalidArgumentShouldNotCauseFurtherCompletion() { + // pass preprocess + final String input = "cmd_with_multiple_args 512 "; + final List suggestions = manager.suggest(new TestCommandSender(), input); + Assertions.assertEquals(Arrays.asList("foo", "bar"), suggestions); + final String input2 = "cmd_with_multiple_args 512 BAR "; + final List suggestions2 = manager.suggest(new TestCommandSender(), input2); + Assertions.assertEquals(Collections.singletonList("world"), suggestions2); + final String input3 = "cmd_with_multiple_args test "; + final List suggestions3 = manager.suggest(new TestCommandSender(), input3); + Assertions.assertEquals(Collections.emptyList(), suggestions3); + final String input4 = "cmd_with_multiple_args 512 f"; + final List suggestions4 = manager.suggest(new TestCommandSender(), input4); + Assertions.assertEquals(Collections.singletonList("foo"), suggestions4); + final String input5 = "cmd_with_multiple_args world f"; + final List suggestions5 = manager.suggest(new TestCommandSender(), input5); + Assertions.assertEquals(Collections.emptyList(), suggestions5); + // trigger preprocess fail + final String input6 = "cmd_with_multiple_args 1024"; + final List suggestions6 = manager.suggest(new TestCommandSender(), input6); + Assertions.assertEquals(11, suggestions6.size()); + final String input7 = "cmd_with_multiple_args 1024 "; + final List suggestions7 = manager.suggest(new TestCommandSender(), input7); + Assertions.assertEquals(Collections.emptyList(), suggestions7); + final String input8 = "cmd_with_multiple_args 1024 f"; + final List suggestions8 = manager.suggest(new TestCommandSender(), input8); + Assertions.assertEquals(Collections.emptyList(), suggestions8); + final String input9 = "cmd_with_multiple_args 1024 foo w"; + final List suggestions9 = manager.suggest(new TestCommandSender(), input9); + Assertions.assertEquals(Collections.emptyList(), suggestions9); + } + @Test void testFlagYieldingGreedyStringFollowedByFlagArgument() { // Arrange