Implemented merging of CommandNode children

src/main/java/net/minecraft/commands/builder/ArgumentBuilder.java

@@ -4,7 +4,7 @@ import net.minecraft.commands.Command;
 import net.minecraft.commands.tree.CommandNode;
 import net.minecraft.commands.tree.RootCommandNode;
 
-import java.util.List;
+import java.util.Collection;
 
 public abstract class ArgumentBuilder<T extends ArgumentBuilder<?>> {
     private final RootCommandNode arguments = new RootCommandNode();
@@ -17,7 +17,7 @@ public abstract class ArgumentBuilder<T extends ArgumentBuilder<?>> {
         return getThis();
     }
 
-    public List<CommandNode> getArguments() {
+    public Collection<CommandNode> getArguments() {
         return arguments.getChildren();
     }
 

src/main/java/net/minecraft/commands/tree/ArgumentCommandNode.java

@@ -25,6 +25,11 @@ public class ArgumentCommandNode<T> extends CommandNode {
         return type;
     }
 
+    @Override
+    protected Object getMergeKey() {
+        return name;
+    }
+
     @Override
     public String parse(String command, CommandContextBuilder contextBuilder) throws IllegalArgumentSyntaxException, ArgumentValidationException {
         ParsedArgument<T> parsed = type.parse(command);
@@ -48,16 +53,14 @@ public class ArgumentCommandNode<T> extends CommandNode {
 
         if (!name.equals(that.name)) return false;
         if (!type.equals(that.type)) return false;
-        if (!getChildren().equals(that.getChildren())) return false;
-
-        return true;
+        return super.equals(o);
     }
 
     @Override
     public int hashCode() {
         int result = name.hashCode();
         result = 31 * result + type.hashCode();
-        result = 31 * result + getChildren().hashCode();
+        result = 31 * super.hashCode();
         return result;
     }
 }

src/main/java/net/minecraft/commands/tree/CommandNode.java

@@ -1,16 +1,17 @@
 package net.minecraft.commands.tree;
 
-import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import net.minecraft.commands.Command;
 import net.minecraft.commands.context.CommandContextBuilder;
 import net.minecraft.commands.exceptions.ArgumentValidationException;
 import net.minecraft.commands.exceptions.IllegalArgumentSyntaxException;
 
-import java.util.List;
+import java.util.Collection;
+import java.util.Map;
 
 public abstract class CommandNode {
-    private final Command command;
-    private final List<CommandNode> children = Lists.newArrayList();
+    private final Map<Object, CommandNode> children = Maps.newLinkedHashMap();
+    private Command command;
 
     protected CommandNode(Command command) {
         this.command = command;
@@ -20,13 +21,44 @@ public abstract class CommandNode {
         return command;
     }
 
-    public List<CommandNode> getChildren() {
-        return children;
+    public Collection<CommandNode> getChildren() {
+        return children.values();
     }
 
     public void addChild(CommandNode node) {
-        children.add(node);
+        CommandNode child = children.get(node.getMergeKey());
+        if (child != null) {
+            // We've found something to merge onto
+            if (node.getCommand() != null) {
+                child.command = node.getCommand();
+            }
+            for (CommandNode grandchild : node.getChildren()) {
+                child.addChild(grandchild);
+            }
+        } else {
+            children.put(node.getMergeKey(), node);
+        }
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof CommandNode)) return false;
+
+        CommandNode that = (CommandNode) o;
+
+        if (!children.equals(that.children)) return false;
+        if (command != null ? !command.equals(that.command) : that.command != null) return false;
+
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        return 31 * children.hashCode() + (command != null ? command.hashCode() : 0);
+    }
+
+    protected abstract Object getMergeKey();
+
     public abstract String parse(String command, CommandContextBuilder contextBuilder) throws IllegalArgumentSyntaxException, ArgumentValidationException;
 }

src/main/java/net/minecraft/commands/tree/LiteralCommandNode.java

@@ -18,6 +18,11 @@ public class LiteralCommandNode extends CommandNode {
         return literal;
     }
 
+    @Override
+    protected Object getMergeKey() {
+        return literal;
+    }
+
     @Override
     public String parse(String command, CommandContextBuilder contextBuilder) throws IllegalArgumentSyntaxException, ArgumentValidationException {
         String expected = literal + (command.length() > literal.length() ? CommandDispatcher.ARGUMENT_SEPARATOR : "");
@@ -38,15 +43,13 @@ public class LiteralCommandNode extends CommandNode {
         LiteralCommandNode that = (LiteralCommandNode) o;
 
         if (!literal.equals(that.literal)) return false;
-        if (!getChildren().equals(that.getChildren())) return false;
-
-        return true;
+        return super.equals(o);
     }
 
     @Override
     public int hashCode() {
         int result = literal.hashCode();
-        result = 31 * result + getChildren().hashCode();
+        result = 31 * result + super.hashCode();
         return result;
     }
 }

src/main/java/net/minecraft/commands/tree/RootCommandNode.java

@@ -9,6 +9,11 @@ public class RootCommandNode extends CommandNode {
         super(null);
     }
 
+    @Override
+    protected Object getMergeKey() {
+        throw new UnsupportedOperationException("Cannot add a RootCommandNode as a child to any other CommandNode");
+    }
+
     @Override
     public String parse(String command, CommandContextBuilder contextBuilder) throws IllegalArgumentSyntaxException, ArgumentValidationException {
         return command;
@@ -18,16 +23,6 @@ public class RootCommandNode extends CommandNode {
     public boolean equals(Object o) {
         if (this == o) return true;
         if (!(o instanceof RootCommandNode)) return false;
-
-        RootCommandNode that = (RootCommandNode) o;
-
-        if (!getChildren().equals(that.getChildren())) return false;
-
-        return true;
-    }
-
-    @Override
-    public int hashCode() {
-        return getChildren().hashCode();
+        return super.equals(o);
     }
 }

src/test/java/net/minecraft/commands/tree/AbstractCommandNodeTest.java

@@ -0,0 +1,66 @@
+package net.minecraft.commands.tree;
+
+import net.minecraft.commands.Command;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import static net.minecraft.commands.builder.LiteralArgumentBuilder.literal;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+
+@RunWith(MockitoJUnitRunner.class)
+public abstract class AbstractCommandNodeTest {
+    @Mock private Command command;
+
+    protected abstract CommandNode getCommandNode();
+
+    @Test
+    public void testAddChild() throws Exception {
+        CommandNode node = getCommandNode();
+
+        node.addChild(literal("child1").build());
+        node.addChild(literal("child2").build());
+        node.addChild(literal("child1").build());
+
+        assertThat(node.getChildren(), hasSize(2));
+    }
+
+    @Test
+    public void testAddChildMergesGrandchildren() throws Exception {
+        CommandNode node = getCommandNode();
+
+        node.addChild(literal("child").then(
+            literal("grandchild1")
+        ).build());
+
+        node.addChild(literal("child").then(
+            literal("grandchild2")
+        ).build());
+
+        assertThat(node.getChildren(), hasSize(1));
+        assertThat(node.getChildren().iterator().next().getChildren(), hasSize(2));
+    }
+
+    @Test
+    public void testAddChildPreservesCommand() throws Exception {
+        CommandNode node = getCommandNode();
+
+        node.addChild(literal("child").executes(command).build());
+        node.addChild(literal("child").build());
+
+        assertThat(node.getChildren().iterator().next().getCommand(), is(command));
+    }
+
+    @Test
+    public void testAddChildOverwritesCommand() throws Exception {
+        CommandNode node = getCommandNode();
+
+        node.addChild(literal("child").build());
+        node.addChild(literal("child").executes(command).build());
+
+        assertThat(node.getChildren().iterator().next().getCommand(), is(command));
+    }
+}

src/test/java/net/minecraft/commands/tree/ArgumentCommandNodeTest.java

@@ -1,6 +1,7 @@
 package net.minecraft.commands.tree;
 
 import com.google.common.testing.EqualsTester;
+import net.minecraft.commands.Command;
 import net.minecraft.commands.context.CommandContextBuilder;
 import net.minecraft.commands.exceptions.IllegalArgumentSyntaxException;
 import org.junit.Before;
@@ -10,11 +11,17 @@ import static net.minecraft.commands.arguments.IntegerArgumentType.integer;
 import static net.minecraft.commands.builder.RequiredArgumentBuilder.argument;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
 
-public class ArgumentCommandNodeTest {
+public class ArgumentCommandNodeTest extends AbstractCommandNodeTest {
     ArgumentCommandNode node;
     CommandContextBuilder contextBuilder;
 
+    @Override
+    protected CommandNode getCommandNode() {
+        return node;
+    }
+
     @Before
     public void setUp() throws Exception {
         node = argument("foo", integer()).build();
@@ -44,11 +51,17 @@ public class ArgumentCommandNodeTest {
 
     @Test
     public void testEquals() throws Exception {
+        Command command = mock(Command.class);
+
         new EqualsTester()
             .addEqualityGroup(
                 argument("foo", integer()).build(),
                 argument("foo", integer()).build()
             )
+            .addEqualityGroup(
+                argument("foo", integer()).executes(command).build(),
+                argument("foo", integer()).executes(command).build()
+            )
             .addEqualityGroup(
                 argument("bar", integer(-100, 100)).build(),
                 argument("bar", integer(-100, 100)).build()

src/test/java/net/minecraft/commands/tree/LiteralCommandNodeTest.java

@@ -1,6 +1,7 @@
 package net.minecraft.commands.tree;
 
 import com.google.common.testing.EqualsTester;
+import net.minecraft.commands.Command;
 import net.minecraft.commands.context.CommandContextBuilder;
 import net.minecraft.commands.exceptions.IllegalArgumentSyntaxException;
 import org.junit.Before;
@@ -9,11 +10,17 @@ import org.junit.Test;
 import static net.minecraft.commands.builder.LiteralArgumentBuilder.literal;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
 
-public class LiteralCommandNodeTest {
+public class LiteralCommandNodeTest extends AbstractCommandNodeTest {
     LiteralCommandNode node;
     CommandContextBuilder contextBuilder;
 
+    @Override
+    protected CommandNode getCommandNode() {
+        return node;
+    }
+
     @Before
     public void setUp() throws Exception {
         node = literal("foo").build();
@@ -42,11 +49,17 @@ public class LiteralCommandNodeTest {
 
     @Test
     public void testEquals() throws Exception {
+        Command command = mock(Command.class);
+
         new EqualsTester()
             .addEqualityGroup(
                 literal("foo").build(),
                 literal("foo").build()
             )
+            .addEqualityGroup(
+                literal("bar").executes(command).build(),
+                literal("bar").executes(command).build()
+            )
             .addEqualityGroup(
                 literal("bar").build(),
                 literal("bar").build()

src/test/java/net/minecraft/commands/tree/RootCommandNodeTest.java

@@ -9,9 +9,14 @@ import static net.minecraft.commands.builder.LiteralArgumentBuilder.literal;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
 
-public class RootCommandNodeTest {
+public class RootCommandNodeTest extends AbstractCommandNodeTest {
     RootCommandNode node;
 
+    @Override
+    protected CommandNode getCommandNode() {
+        return node;
+    }
+
     @Before
     public void setUp() throws Exception {
         node = new RootCommandNode();
@@ -22,6 +27,11 @@ public class RootCommandNodeTest {
         assertThat(node.parse("foo bar baz", new CommandContextBuilder()), is("foo bar baz"));
     }
 
+    @Test(expected = UnsupportedOperationException.class)
+    public void testAddChildNoRoot() throws Exception {
+        node.addChild(new RootCommandNode());
+    }
+
     @Test
     public void testEquals() throws Exception {
         new EqualsTester()