From 9d65991ae8323f4ef3d45f3b76ff821eace3aef3 Mon Sep 17 00:00:00 2001 From: Nathan Adams Date: Tue, 31 Oct 2017 17:03:52 +0100 Subject: [PATCH] Fixed competing overloads not allowing other ones to try to parse --- .../mojang/brigadier/CommandDispatcher.java | 68 +++++++++++++------ .../com/mojang/brigadier/StringReader.java | 5 ++ .../brigadier/CommandDispatcherTest.java | 60 +++++++++++++++- .../CommandDispatcherUsagesTest.java | 6 +- 4 files changed, 110 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/mojang/brigadier/CommandDispatcher.java b/src/main/java/com/mojang/brigadier/CommandDispatcher.java index 1f1c1c8..6163bd4 100644 --- a/src/main/java/com/mojang/brigadier/CommandDispatcher.java +++ b/src/main/java/com/mojang/brigadier/CommandDispatcher.java @@ -16,7 +16,9 @@ import com.mojang.brigadier.tree.RootCommandNode; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Deque; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Function; @@ -103,22 +105,34 @@ public class CommandDispatcher { return result; } - public ParseResults parse(final String command, final S source) throws CommandSyntaxException { + public ParseResults parse(final String command, final S source) { final StringReader reader = new StringReader(command); final CommandContextBuilder context = new CommandContextBuilder<>(this, source); - return parseNodes(root, reader, context, context, null); + return parseNodes(root, reader, context); } - private ParseResults parseNodes(final CommandNode node, final StringReader reader, final CommandContextBuilder contextBuilder, CommandContextBuilder rootContext, final CommandContextBuilder parentContext) throws CommandSyntaxException { - final S source = contextBuilder.getSource(); - final Map, CommandSyntaxException> errors = Maps.newHashMap(); + private static class PartialParse { + public final CommandContextBuilder context; + public final ParseResults parse; + + private PartialParse(final CommandContextBuilder context, final ParseResults parse) { + this.context = context; + this.parse = parse; + } + } + + private ParseResults parseNodes(final CommandNode node, final StringReader originalReader, final CommandContextBuilder contextSoFar) { + final S source = contextSoFar.getSource(); + final Map, CommandSyntaxException> errors = Maps.newLinkedHashMap(); + final List> potentials = Lists.newArrayList(); + final int cursor = originalReader.getCursor(); for (final CommandNode child : node.getChildren()) { if (!child.canUse(source)) { continue; } - final CommandContextBuilder context = contextBuilder.copy(); - final int cursor = reader.getCursor(); + final CommandContextBuilder context = contextSoFar.copy(); + final StringReader reader = new StringReader(originalReader); try { child.parse(reader, context); if (reader.canRead()) { @@ -132,34 +146,46 @@ public class CommandDispatcher { continue; } - if (rootContext == contextBuilder) { - rootContext = context; - } - - if (parentContext != null) { - parentContext.withChild(context); - } - context.withCommand(child.getCommand()); if (reader.canRead()) { reader.skip(); if (child.getRedirect() != null) { final CommandContextBuilder childContext = new CommandContextBuilder<>(this, source); childContext.withNode(child.getRedirect(), ""); - return parseNodes(child.getRedirect(), reader, childContext, rootContext, context); + final ParseResults parse = parseNodes(child.getRedirect(), reader, childContext); + context.withChild(parse.getContext()); + return new ParseResults<>(context, parse.getReader(), parse.getExceptions()); } else { - return parseNodes(child, reader, context, rootContext, parentContext); + final ParseResults parse = parseNodes(child, reader, context); + potentials.add(new PartialParse<>(context, parse)); } } else { - return new ParseResults<>(rootContext); + potentials.add(new PartialParse<>(context, new ParseResults<>(context, reader, Collections.emptyMap()))); } } - if (parentContext != null) { - parentContext.withChild(contextBuilder); + if (!potentials.isEmpty()) { + final List> sorted = Lists.newArrayList(potentials); + sorted.sort((a, b) -> { + if (!a.parse.getReader().canRead() && b.parse.getReader().canRead()) { + return -1; + } + if (a.parse.getReader().canRead() && !b.parse.getReader().canRead()) { + return 1; + } + if (a.parse.getExceptions().isEmpty() && !b.parse.getExceptions().isEmpty()) { + return -1; + } + if (!a.parse.getExceptions().isEmpty() && b.parse.getExceptions().isEmpty()) { + return 1; + } + return 0; + }); + final PartialParse likely = sorted.get(0); + return likely.parse; } - return new ParseResults<>(rootContext, reader, errors); + return new ParseResults<>(contextSoFar, originalReader, errors); } public String[] getAllUsage(final CommandNode node, final S source, final boolean restricted) { diff --git a/src/main/java/com/mojang/brigadier/StringReader.java b/src/main/java/com/mojang/brigadier/StringReader.java index e972f82..007061f 100644 --- a/src/main/java/com/mojang/brigadier/StringReader.java +++ b/src/main/java/com/mojang/brigadier/StringReader.java @@ -22,6 +22,11 @@ public class StringReader implements ImmutableStringReader { private final String string; private int cursor; + public StringReader(final StringReader other) { + this.string = other.string; + this.cursor = other.cursor; + } + public StringReader(final String string) { this.string = string; } diff --git a/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java b/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java index 69f659f..43f0182 100644 --- a/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java +++ b/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java @@ -175,7 +175,61 @@ public class CommandDispatcherTest { @SuppressWarnings("unchecked") @Test - public void testExecuteRedirected() throws Exception { + public void testExecuteAmbiguiousParentSubcommand() throws Exception { + final Command subCommand = mock(Command.class); + when(subCommand.run(any())).thenReturn(100); + + subject.register( + literal("test") + .then( + argument("incorrect", integer()) + .executes(command) + ) + .then( + argument("right", integer()) + .then( + argument("sub", integer()) + .executes(subCommand) + ) + ) + ); + + assertThat(subject.execute("test 1 2", source), is(100)); + verify(subCommand).run(any(CommandContext.class)); + verify(command, never()).run(any()); + } + + @SuppressWarnings("unchecked") + @Test + public void testExecuteAmbiguiousParentSubcommandViaRedirect() throws Exception { + final Command subCommand = mock(Command.class); + when(subCommand.run(any())).thenReturn(100); + + final LiteralCommandNode real = subject.register( + literal("test") + .then( + argument("incorrect", integer()) + .executes(command) + ) + .then( + argument("right", integer()) + .then( + argument("sub", integer()) + .executes(subCommand) + ) + ) + ); + + subject.register(literal("redirect").redirect(real)); + + assertThat(subject.execute("redirect 1 2", source), is(100)); + verify(subCommand).run(any(CommandContext.class)); + verify(command, never()).run(any()); + } + + @SuppressWarnings("unchecked") + @Test + public void testExecuteRedirectedMultipleTimes() throws Exception { subject.register(literal("actual").executes(command)); subject.register(literal("redirected").redirect(subject.getRoot(), Collections::singleton)); @@ -199,7 +253,7 @@ public class CommandDispatcherTest { @SuppressWarnings("unchecked") @Test - public void testExecuteRedirectedMultipleTimes() throws Exception { + public void testExecuteRedirected() throws Exception { final Function, Collection> modifier = mock(Function.class); final Object source1 = new Object(); final Object source2 = new Object(); @@ -237,7 +291,7 @@ public class CommandDispatcherTest { } catch (final CommandSyntaxException ex) { assertThat(ex.getType(), is(CommandDispatcher.ERROR_UNKNOWN_COMMAND)); assertThat(ex.getData(), is(Collections.emptyMap())); - assertThat(ex.getCursor(), is(0)); + assertThat(ex.getCursor(), is(5)); } } diff --git a/src/test/java/com/mojang/brigadier/CommandDispatcherUsagesTest.java b/src/test/java/com/mojang/brigadier/CommandDispatcherUsagesTest.java index e0ee6b6..20c19a8 100644 --- a/src/test/java/com/mojang/brigadier/CommandDispatcherUsagesTest.java +++ b/src/test/java/com/mojang/brigadier/CommandDispatcherUsagesTest.java @@ -99,11 +99,7 @@ public class CommandDispatcherUsagesTest { } private CommandNode get(final String command) { - try { - return Iterators.getLast(subject.parse(command, source).getContext().getNodes().keySet().iterator()); - } catch (final CommandSyntaxException e) { - throw new AssertionError("get() failed unexpectedly", e); - } + return Iterators.getLast(subject.parse(command, source).getContext().getNodes().keySet().iterator()); } @Test