Two nodes at the same position that can both parse is no longer supported. Separated ERROR_UNKNOWN_COMMAND into ERROR_UNKNOWN_ARGUMENT

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

@@ -7,6 +7,7 @@ import com.mojang.brigadier.builder.LiteralArgumentBuilder;
 import com.mojang.brigadier.context.CommandContext;
 import com.mojang.brigadier.context.CommandContextBuilder;
 import com.mojang.brigadier.exceptions.CommandException;
+import com.mojang.brigadier.exceptions.ParameterizedCommandExceptionType;
 import com.mojang.brigadier.exceptions.SimpleCommandExceptionType;
 import com.mojang.brigadier.tree.CommandNode;
 import com.mojang.brigadier.tree.LiteralCommandNode;
@@ -18,8 +19,10 @@ import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 public class CommandDispatcher<S> {
-    public static final SimpleCommandExceptionType ERROR_UNKNOWN_COMMAND = new SimpleCommandExceptionType("command.unknown", "Unknown command");
+    public static final SimpleCommandExceptionType ERROR_UNKNOWN_COMMAND = new SimpleCommandExceptionType("command.unknown.command", "Unknown command");
-    public static final SimpleCommandExceptionType ERROR_IMPERMISSIBLE = new SimpleCommandExceptionType("command.impermissible", "Command not allowed");
+    public static final ParameterizedCommandExceptionType ERROR_UNKNOWN_ARGUMENT = new ParameterizedCommandExceptionType("command.unknown.argument", "Incorrect argument for command, couldn't parse: ${argument}", "argument");
+    public static final SimpleCommandExceptionType ERROR_IMPERMISSIBLE = new SimpleCommandExceptionType("command.impermissible", "You are not allowed to use this command");
+
     public static final String ARGUMENT_SEPARATOR = " ";
     private static final String USAGE_OPTIONAL_OPEN = "[";
     private static final String USAGE_OPTIONAL_CLOSE = "]";
@@ -61,27 +64,33 @@ public class CommandDispatcher<S> {
                 exception = ERROR_IMPERMISSIBLE.create();
                 continue;
             }
+            CommandContextBuilder<S> context = contextBuilder.copy();
+            String remaining;
             try {
-                CommandContextBuilder<S> context = contextBuilder.copy();
+                remaining = child.parse(command, context);
-                String remaining = child.parse(command, context);
-                if (child.getCommand() != null) {
-                    context.withCommand(child.getCommand());
-                }
-                if (remaining.isEmpty()) {
-                    return context;
-                } else {
-                    return parseNodes(child, remaining.substring(1), context);
-                }
             } catch (CommandException ex) {
                 exception = ex;
+                continue;
+            }
+
+            if (child.getCommand() != null) {
+                context.withCommand(child.getCommand());
+            }
+            if (remaining.isEmpty()) {
+                return context;
+            } else {
+                return parseNodes(child, remaining.substring(1), context);
             }
         }
 
         if (command.length() > 0) {
-            if (exception != null) {
+            if (node == root && (exception == null || exception.getType() != ERROR_IMPERMISSIBLE)) {
+                throw ERROR_UNKNOWN_COMMAND.create();
+            }
+            if (exception != null && node.getChildren().size() == 1) {
                 throw exception;
             }
-            throw ERROR_UNKNOWN_COMMAND.create();
+            throw ERROR_UNKNOWN_ARGUMENT.create(command);
         }
 
         return contextBuilder;

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

@@ -5,6 +5,7 @@ import com.mojang.brigadier.arguments.IntegerArgumentType;
 import com.mojang.brigadier.context.CommandContext;
 import com.mojang.brigadier.exceptions.CommandException;
 import com.mojang.brigadier.exceptions.CommandExceptionType;
+import com.mojang.brigadier.tree.LiteralCommandNode;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -56,45 +57,11 @@ public class CommandDispatcherTest {
         verify(command, times(2)).run(any(CommandContext.class));
     }
 
-    @SuppressWarnings("unchecked")
-    @Test
-    public void testCreateAndExecuteOverlappingCommands() throws Exception {
-        Command<Object> one = mock(Command.class);
-        Command<Object> two = mock(Command.class);
-        Command<Object> three = mock(Command.class);
-
-        when(one.run(any())).thenReturn(111);
-        when(two.run(any())).thenReturn(222);
-        when(three.run(any())).thenReturn(333);
-
-        subject.register(
-            literal("foo").then(
-                argument("one", integer()).then(
-                    literal("one").executes(one)
-                )
-            ).then(
-                argument("two", integer()).then(
-                    literal("two").executes(two)
-                )
-            ).then(
-                argument("three", integer()).then(
-                    literal("three").executes(three)
-                )
-            )
-        );
-
-        assertThat(subject.execute("foo 1 one", source), is(111));
-        verify(one).run(any(CommandContext.class));
-
-        assertThat(subject.execute("foo 2 two", source), is(222));
-        verify(two).run(any(CommandContext.class));
-
-        assertThat(subject.execute("foo 3 three", source), is(333));
-        verify(three).run(any(CommandContext.class));
-    }
-
     @Test
     public void testExecuteUnknownCommand() throws Exception {
+        subject.register(literal("bar"));
+        subject.register(literal("baz"));
+
         try {
             subject.execute("foo", source);
             fail();
@@ -138,8 +105,38 @@ public class CommandDispatcherTest {
             subject.execute("foo bar", source);
             fail();
         } catch (CommandException ex) {
-            assertThat(ex.getType(), is(CommandDispatcher.ERROR_UNKNOWN_COMMAND));
+            assertThat(ex.getType(), is(CommandDispatcher.ERROR_UNKNOWN_ARGUMENT));
-            assertThat(ex.getData(), is(Collections.<String, Object>emptyMap()));
+            assertThat(ex.getData(), is(Collections.singletonMap("argument", "bar")));
+        }
+    }
+
+    @Test
+    public void testExecuteIncorrectLiteral() throws Exception {
+        subject.register(literal("foo").executes(command).then(literal("bar")));
+
+        try {
+            subject.execute("foo baz", source);
+            fail();
+        } catch (CommandException ex) {
+            assertThat(ex.getType(), is(LiteralCommandNode.ERROR_INCORRECT_LITERAL));
+            assertThat(ex.getData(), is(Collections.singletonMap("expected", "bar")));
+        }
+    }
+
+    @Test
+    public void testExecuteAmbiguousIncorrectArgument() throws Exception {
+        subject.register(
+            literal("foo").executes(command)
+                .then(literal("bar"))
+                .then(literal("baz"))
+        );
+
+        try {
+            subject.execute("foo unknown", source);
+            fail();
+        } catch (CommandException ex) {
+            assertThat(ex.getType(), is(CommandDispatcher.ERROR_UNKNOWN_ARGUMENT));
+            assertThat(ex.getData(), is(Collections.singletonMap("argument", "unknown")));
         }
     }