From 8ae247bb6a1a36045711e5bfca4f65a0a67d3917 Mon Sep 17 00:00:00 2001 From: Nathan Adams Date: Tue, 7 Nov 2017 15:53:12 +0100 Subject: [PATCH] Don't store string arguments in contexts, ranges are more useful --- .../mojang/brigadier/CommandDispatcher.java | 11 ++-- .../brigadier/context/CommandContext.java | 15 +++-- .../context/CommandContextBuilder.java | 38 +++++-------- .../brigadier/context/ParsedArgument.java | 44 +++++---------- .../mojang/brigadier/context/StringRange.java | 56 +++++++++++++++++++ .../brigadier/tree/ArgumentCommandNode.java | 2 +- .../brigadier/tree/LiteralCommandNode.java | 3 +- .../brigadier/CommandDispatcherTest.java | 16 +++--- .../brigadier/context/CommandContextTest.java | 23 +++----- .../brigadier/context/ParsedArgumentTest.java | 2 +- .../tree/ArgumentCommandNodeTest.java | 2 +- .../tree/LiteralCommandNodeTest.java | 2 +- .../brigadier/tree/RootCommandNodeTest.java | 2 +- 13 files changed, 122 insertions(+), 94 deletions(-) create mode 100644 src/main/java/com/mojang/brigadier/context/StringRange.java diff --git a/src/main/java/com/mojang/brigadier/CommandDispatcher.java b/src/main/java/com/mojang/brigadier/CommandDispatcher.java index efdd8db..5bca273 100644 --- a/src/main/java/com/mojang/brigadier/CommandDispatcher.java +++ b/src/main/java/com/mojang/brigadier/CommandDispatcher.java @@ -7,6 +7,7 @@ import com.google.common.collect.Sets; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.context.CommandContext; import com.mojang.brigadier.context.CommandContextBuilder; +import com.mojang.brigadier.context.StringRange; import com.mojang.brigadier.exceptions.CommandSyntaxException; import com.mojang.brigadier.exceptions.SimpleCommandExceptionType; import com.mojang.brigadier.tree.CommandNode; @@ -70,7 +71,7 @@ public class CommandDispatcher { if (parse.getReader().canRead()) { if (parse.getExceptions().size() == 1) { throw parse.getExceptions().values().iterator().next(); - } else if (parse.getContext().getInput().isEmpty()) { + } else if (parse.getContext().getRange().isEmpty()) { throw ERROR_UNKNOWN_COMMAND.createWithContext(parse.getReader()); } else { throw ERROR_UNKNOWN_ARGUMENT.createWithContext(parse.getReader()); @@ -111,7 +112,7 @@ public class CommandDispatcher { public ParseResults parse(final String command, final S source) { final StringReader reader = new StringReader(command); - final CommandContextBuilder context = new CommandContextBuilder<>(this, source); + final CommandContextBuilder context = new CommandContextBuilder<>(this, source, 0); return parseNodes(root, reader, context); } @@ -154,8 +155,8 @@ public class CommandDispatcher { if (reader.canRead()) { reader.skip(); if (child.getRedirect() != null) { - final CommandContextBuilder childContext = new CommandContextBuilder<>(this, source); - childContext.withNode(child.getRedirect(), ""); + final CommandContextBuilder childContext = new CommandContextBuilder<>(this, source, reader.getCursor()); + childContext.withNode(child.getRedirect(), new StringRange(reader.getCursor(), reader.getCursor())); final ParseResults parse = parseNodes(child.getRedirect(), reader, childContext); context.withChild(parse.getContext()); return new ParseResults<>(context, parse.getReader(), parse.getExceptions()); @@ -317,7 +318,7 @@ public class CommandDispatcher { public String[] getCompletionSuggestions(final String command, final S source) { final StringReader reader = new StringReader(command); - final Set nodes = findSuggestions(root, reader, new CommandContextBuilder<>(this, source), Sets.newLinkedHashSet()); + final Set nodes = findSuggestions(root, reader, new CommandContextBuilder<>(this, source, 0), Sets.newLinkedHashSet()); return nodes.toArray(new String[nodes.size()]); } diff --git a/src/main/java/com/mojang/brigadier/context/CommandContext.java b/src/main/java/com/mojang/brigadier/context/CommandContext.java index f17b29a..0dc6505 100644 --- a/src/main/java/com/mojang/brigadier/context/CommandContext.java +++ b/src/main/java/com/mojang/brigadier/context/CommandContext.java @@ -11,16 +11,16 @@ public class CommandContext { private final S source; private final Command command; private final Map> arguments; - private final Map, String> nodes; - private final String input; + private final Map, StringRange> nodes; + private final StringRange range; private final CommandContext child; - public CommandContext(final S source, final Map> arguments, final Command command, final Map, String> nodes, final String input, final CommandContext child) { + public CommandContext(final S source, final Map> arguments, final Command command, final Map, StringRange> nodes, final StringRange range, final CommandContext child) { this.source = source; this.arguments = arguments; this.command = command; this.nodes = nodes; - this.input = input; + this.range = range; this.child = child; } @@ -78,12 +78,11 @@ public class CommandContext { return result; } - public String getInput() { - return input; + public StringRange getRange() { + return range; } - public Map, String> getNodes() { + public Map, StringRange> getNodes() { return nodes; } - } diff --git a/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java b/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java index a27c69b..a4bc14d 100644 --- a/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java +++ b/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java @@ -9,15 +9,17 @@ import java.util.Map; public class CommandContextBuilder { private final Map> arguments = Maps.newLinkedHashMap(); - private final Map, String> nodes = Maps.newLinkedHashMap(); + private final Map, StringRange> nodes = Maps.newLinkedHashMap(); private final CommandDispatcher dispatcher; private S source; private Command command; private CommandContextBuilder child; + private StringRange range; - public CommandContextBuilder(final CommandDispatcher dispatcher, final S source) { + public CommandContextBuilder(final CommandDispatcher dispatcher, final S source, final int start) { this.dispatcher = dispatcher; this.source = source; + this.range = new StringRange(start, start); } public CommandContextBuilder withSource(final S source) { @@ -43,17 +45,19 @@ public class CommandContextBuilder { return this; } - public CommandContextBuilder withNode(final CommandNode node, final String raw) { - nodes.put(node, raw); + public CommandContextBuilder withNode(final CommandNode node, final StringRange range) { + nodes.put(node, range); + this.range = new StringRange(Math.min(this.range.getStart(), range.getStart()), Math.max(this.range.getEnd(), range.getEnd())); return this; } public CommandContextBuilder copy() { - final CommandContextBuilder copy = new CommandContextBuilder<>(dispatcher, source); + final CommandContextBuilder copy = new CommandContextBuilder<>(dispatcher, source, range.getStart()); copy.command = command; copy.arguments.putAll(arguments); copy.nodes.putAll(nodes); copy.child = child; + copy.range = range; return copy; } @@ -70,31 +74,19 @@ public class CommandContextBuilder { return command; } - public String getInput() { - final StringBuilder result = new StringBuilder(); - boolean first = true; - for (final String node : nodes.values()) { - if (first) { - if (!node.isEmpty()) { - first = false; - } - } else { - result.append(CommandDispatcher.ARGUMENT_SEPARATOR); - } - result.append(node); - } - return result.toString(); - } - - public Map, String> getNodes() { + public Map, StringRange> getNodes() { return nodes; } public CommandContext build() { - return new CommandContext<>(source, arguments, command, nodes, getInput(), child == null ? null : child.build()); + return new CommandContext<>(source, arguments, command, nodes, range, child == null ? null : child.build()); } public CommandDispatcher getDispatcher() { return dispatcher; } + + public StringRange getRange() { + return range; + } } diff --git a/src/main/java/com/mojang/brigadier/context/ParsedArgument.java b/src/main/java/com/mojang/brigadier/context/ParsedArgument.java index 97f7b75..f822441 100644 --- a/src/main/java/com/mojang/brigadier/context/ParsedArgument.java +++ b/src/main/java/com/mojang/brigadier/context/ParsedArgument.java @@ -1,28 +1,18 @@ package com.mojang.brigadier.context; -import com.mojang.brigadier.ImmutableStringReader; +import java.util.Objects; public class ParsedArgument { - private final int start; - private final int end; + private final StringRange range; private final T result; public ParsedArgument(final int start, final int end, final T result) { - this.start = start; - this.end = end; + this.range = new StringRange(start, end); this.result = result; } - public String getRaw(final ImmutableStringReader reader) { - return reader.getString().substring(start, end); - } - - public int getStart() { - return start; - } - - public int getEnd() { - return end; + public StringRange getRange() { + return range; } public T getResult() { @@ -31,24 +21,18 @@ public class ParsedArgument { @Override public boolean equals(final Object o) { - if (this == o) return true; - if (!(o instanceof ParsedArgument)) return false; - - final ParsedArgument that = (ParsedArgument) o; - - if (start != that.start) return false; - if (end != that.end) return false; - if (!result.equals(that.result)) return false; - - return true; + if (this == o) { + return true; + } + if (!(o instanceof ParsedArgument)) { + return false; + } + final ParsedArgument that = (ParsedArgument) o; + return Objects.equals(range, that.range) && Objects.equals(result, that.result); } @Override public int hashCode() { - int result = start; - result = 31 * result + end; - result = 31 * result + this.result.hashCode(); - return result; + return Objects.hash(range, result); } - } diff --git a/src/main/java/com/mojang/brigadier/context/StringRange.java b/src/main/java/com/mojang/brigadier/context/StringRange.java new file mode 100644 index 0000000..5d47c1f --- /dev/null +++ b/src/main/java/com/mojang/brigadier/context/StringRange.java @@ -0,0 +1,56 @@ +package com.mojang.brigadier.context; + +import com.mojang.brigadier.ImmutableStringReader; + +import java.util.Objects; + +public class StringRange { + private final int start; + private final int end; + + public StringRange(final int start, final int end) { + this.start = start; + this.end = end; + } + + public int getStart() { + return start; + } + + public int getEnd() { + return end; + } + + public String get(final ImmutableStringReader reader) { + return reader.getString().substring(start, end); + } + + public String get(final String string) { + return string.substring(start, end); + } + + public boolean isEmpty() { + return start == end; + } + + public int getLength() { + return end - start; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (!(o instanceof StringRange)) { + return false; + } + final StringRange that = (StringRange) o; + return start == that.start && end == that.end; + } + + @Override + public int hashCode() { + return Objects.hash(start, end); + } +} diff --git a/src/main/java/com/mojang/brigadier/tree/ArgumentCommandNode.java b/src/main/java/com/mojang/brigadier/tree/ArgumentCommandNode.java index 1dffc0f..b9ef2b9 100644 --- a/src/main/java/com/mojang/brigadier/tree/ArgumentCommandNode.java +++ b/src/main/java/com/mojang/brigadier/tree/ArgumentCommandNode.java @@ -53,7 +53,7 @@ public class ArgumentCommandNode extends CommandNode { final ParsedArgument parsed = new ParsedArgument<>(start, reader.getCursor(), result); contextBuilder.withArgument(name, parsed); - contextBuilder.withNode(this, parsed.getRaw(reader)); + contextBuilder.withNode(this, parsed.getRange()); } @Override diff --git a/src/main/java/com/mojang/brigadier/tree/LiteralCommandNode.java b/src/main/java/com/mojang/brigadier/tree/LiteralCommandNode.java index 9f21c3f..c0bb9fe 100644 --- a/src/main/java/com/mojang/brigadier/tree/LiteralCommandNode.java +++ b/src/main/java/com/mojang/brigadier/tree/LiteralCommandNode.java @@ -6,6 +6,7 @@ import com.mojang.brigadier.StringReader; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.context.CommandContext; import com.mojang.brigadier.context.CommandContextBuilder; +import com.mojang.brigadier.context.StringRange; import com.mojang.brigadier.exceptions.CommandSyntaxException; import com.mojang.brigadier.exceptions.ParameterizedCommandExceptionType; @@ -45,7 +46,7 @@ public class LiteralCommandNode extends CommandNode { } } - contextBuilder.withNode(this, literal); + contextBuilder.withNode(this, new StringRange(start, reader.getCursor())); } @Override diff --git a/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java b/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java index 506973c..651dfdd 100644 --- a/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java +++ b/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java @@ -233,18 +233,19 @@ public class CommandDispatcherTest { subject.register(literal("actual").executes(command)); subject.register(literal("redirected").redirect(subject.getRoot(), Collections::singleton)); - final ParseResults parse = subject.parse("redirected redirected actual", source); - assertThat(parse.getContext().getInput(), equalTo("redirected")); + final String input = "redirected redirected actual"; + final ParseResults parse = subject.parse(input, source); + assertThat(parse.getContext().getRange().get(input), equalTo("redirected")); assertThat(parse.getContext().getNodes().size(), is(1)); final CommandContextBuilder child1 = parse.getContext().getChild(); assertThat(child1, is(notNullValue())); - assertThat(child1.getInput(), equalTo("redirected")); + assertThat(child1.getRange().get(input), equalTo("redirected")); assertThat(child1.getNodes().size(), is(2)); final CommandContextBuilder child2 = child1.getChild(); assertThat(child2, is(notNullValue())); - assertThat(child2.getInput(), equalTo("actual")); + assertThat(child2.getRange().get(input), equalTo("actual")); assertThat(child2.getNodes().size(), is(2)); assertThat(subject.execute(parse), is(42)); @@ -263,14 +264,15 @@ public class CommandDispatcherTest { subject.register(literal("actual").executes(command)); subject.register(literal("redirected").redirect(subject.getRoot(), modifier)); - final ParseResults parse = subject.parse("redirected actual", source); - assertThat(parse.getContext().getInput(), equalTo("redirected")); + final String input = "redirected actual"; + final ParseResults parse = subject.parse(input, source); + assertThat(parse.getContext().getRange().get(input), equalTo("redirected")); assertThat(parse.getContext().getNodes().size(), is(1)); assertThat(parse.getContext().getSource(), is(source)); final CommandContextBuilder parent = parse.getContext().getChild(); assertThat(parent, is(notNullValue())); - assertThat(parent.getInput(), equalTo("actual")); + assertThat(parent.getRange().get(input), equalTo("actual")); assertThat(parent.getNodes().size(), is(2)); assertThat(parent.getSource(), is(source)); diff --git a/src/test/java/com/mojang/brigadier/context/CommandContextTest.java b/src/test/java/com/mojang/brigadier/context/CommandContextTest.java index 74cd717..d7b9429 100644 --- a/src/test/java/com/mojang/brigadier/context/CommandContextTest.java +++ b/src/test/java/com/mojang/brigadier/context/CommandContextTest.java @@ -27,7 +27,7 @@ public class CommandContextTest { @Before public void setUp() throws Exception { - builder = new CommandContextBuilder<>(dispatcher, source); + builder = new CommandContextBuilder<>(dispatcher, source, 0); } @Test(expected = IllegalArgumentException.class) @@ -61,20 +61,13 @@ public class CommandContextTest { final CommandNode node = mock(CommandNode.class); final CommandNode otherNode = mock(CommandNode.class); new EqualsTester() - .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source).build(), new CommandContextBuilder<>(dispatcher, source).build()) - .addEqualityGroup(new CommandContextBuilder<>(dispatcher, otherSource).build(), new CommandContextBuilder<>(dispatcher, otherSource).build()) - .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source).withCommand(command).build(), new CommandContextBuilder<>(dispatcher, source).withCommand(command).build()) - .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source).withCommand(otherCommand).build(), new CommandContextBuilder<>(dispatcher, source).withCommand(otherCommand).build()) - .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source).withArgument("foo", new ParsedArgument<>(0, 1, 123)).build(), new CommandContextBuilder<>(dispatcher, source).withArgument("foo", new ParsedArgument<>(0, 1, 123)).build()) - .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source).withNode(node, "foo").withNode(otherNode, "bar").build(), new CommandContextBuilder<>(dispatcher, source).withNode(node, "foo").withNode(otherNode, "bar").build()) - .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source).withNode(otherNode, "bar").withNode(node, "foo").build(), new CommandContextBuilder<>(dispatcher, source).withNode(otherNode, "bar").withNode(node, "foo").build()) + .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source, 0).build(), new CommandContextBuilder<>(dispatcher, source, 0).build()) + .addEqualityGroup(new CommandContextBuilder<>(dispatcher, otherSource, 0).build(), new CommandContextBuilder<>(dispatcher, otherSource, 0).build()) + .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source, 0).withCommand(command).build(), new CommandContextBuilder<>(dispatcher, source, 0).withCommand(command).build()) + .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source, 0).withCommand(otherCommand).build(), new CommandContextBuilder<>(dispatcher, source, 0).withCommand(otherCommand).build()) + .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source, 0).withArgument("foo", new ParsedArgument<>(0, 1, 123)).build(), new CommandContextBuilder<>(dispatcher, source, 0).withArgument("foo", new ParsedArgument<>(0, 1, 123)).build()) + .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source, 0).withNode(node, new StringRange(0, 3)).withNode(otherNode, new StringRange(4, 6)).build(), new CommandContextBuilder<>(dispatcher, source, 0).withNode(node, new StringRange(0, 3)).withNode(otherNode, new StringRange(4, 6)).build()) + .addEqualityGroup(new CommandContextBuilder<>(dispatcher, source, 0).withNode(otherNode, new StringRange(0, 3)).withNode(node, new StringRange(4, 6)).build(), new CommandContextBuilder<>(dispatcher, source, 0).withNode(otherNode, new StringRange(0, 3)).withNode(node, new StringRange(4, 6)).build()) .testEquals(); } - - @Test - public void testGetInput() throws Exception { - final CommandContext context = builder.withNode(literal("foo").build(), "foo").withNode(argument("bar", integer()).build(), "100").withNode(literal("baz").build(), "baz").build(); - - assertThat(context.getInput(), is("foo 100 baz")); - } } \ No newline at end of file diff --git a/src/test/java/com/mojang/brigadier/context/ParsedArgumentTest.java b/src/test/java/com/mojang/brigadier/context/ParsedArgumentTest.java index 5942437..cb225fc 100644 --- a/src/test/java/com/mojang/brigadier/context/ParsedArgumentTest.java +++ b/src/test/java/com/mojang/brigadier/context/ParsedArgumentTest.java @@ -22,6 +22,6 @@ public class ParsedArgumentTest { public void getRaw() throws Exception { final StringReader reader = new StringReader("0123456789"); final ParsedArgument argument = new ParsedArgument<>(2, 5, ""); - assertThat(argument.getRaw(reader), equalTo("234")); + assertThat(argument.getRange().get(reader), equalTo("234")); } } \ No newline at end of file diff --git a/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java b/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java index 42ee9e9..ec11cfb 100644 --- a/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java +++ b/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java @@ -34,7 +34,7 @@ public class ArgumentCommandNodeTest extends AbstractCommandNodeTest { @Before public void setUp() throws Exception { node = argument("foo", integer()).build(); - contextBuilder = new CommandContextBuilder<>(new CommandDispatcher<>(), new Object()); + contextBuilder = new CommandContextBuilder<>(new CommandDispatcher<>(), new Object(), 0); } @Test diff --git a/src/test/java/com/mojang/brigadier/tree/LiteralCommandNodeTest.java b/src/test/java/com/mojang/brigadier/tree/LiteralCommandNodeTest.java index 683d963..b719213 100644 --- a/src/test/java/com/mojang/brigadier/tree/LiteralCommandNodeTest.java +++ b/src/test/java/com/mojang/brigadier/tree/LiteralCommandNodeTest.java @@ -35,7 +35,7 @@ public class LiteralCommandNodeTest extends AbstractCommandNodeTest { @Before public void setUp() throws Exception { node = literal("foo").build(); - contextBuilder = new CommandContextBuilder<>(new CommandDispatcher<>(), new Object()); + contextBuilder = new CommandContextBuilder<>(new CommandDispatcher<>(), new Object(), 0); } @Test diff --git a/src/test/java/com/mojang/brigadier/tree/RootCommandNodeTest.java b/src/test/java/com/mojang/brigadier/tree/RootCommandNodeTest.java index 3d2d193..ab183fa 100644 --- a/src/test/java/com/mojang/brigadier/tree/RootCommandNodeTest.java +++ b/src/test/java/com/mojang/brigadier/tree/RootCommandNodeTest.java @@ -32,7 +32,7 @@ public class RootCommandNodeTest extends AbstractCommandNodeTest { @Test public void testParse() throws Exception { final StringReader reader = new StringReader("hello world"); - node.parse(reader, new CommandContextBuilder<>(new CommandDispatcher<>(), new Object())); + node.parse(reader, new CommandContextBuilder<>(new CommandDispatcher<>(), new Object(), 0)); assertThat(reader.getCursor(), is(0)); }