From 073ee9aa294772fbd2948ffcbc160f7d2413d90c Mon Sep 17 00:00:00 2001
From: Daniel Naylor <git@drnaylor.co.uk>
Date: Sun, 25 Apr 2021 13:05:01 +0100
Subject: [PATCH] Add redirects to the equality checks for nodes.

They are added as reference equality checks rather than standard object equality checks because of a stack overflow that can occur if a redirect is to a parent of the current node. If two nodes are redirected to two different nodes that otherwise look the same, then they will not be equal, which is fine because if one of the nodes gains a new child then they won't be equal any more - so this is a sensible check to have.

Test included to demonstrate the change.
---
 .../java/com/mojang/brigadier/tree/CommandNode.java   | 11 ++++++++---
 .../brigadier/tree/ArgumentCommandNodeTest.java       |  9 +++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/main/java/com/mojang/brigadier/tree/CommandNode.java b/src/main/java/com/mojang/brigadier/tree/CommandNode.java
index 15b584aa..c522c31e 100644
--- a/src/main/java/com/mojang/brigadier/tree/CommandNode.java
+++ b/src/main/java/com/mojang/brigadier/tree/CommandNode.java
@@ -128,13 +128,18 @@ public boolean equals(final Object o) {
 
         if (!children.equals(that.children)) return false;
         if (command != null ? !command.equals(that.command) : that.command != null) return false;
-
-        return true;
+        // Because the primary use of a redirect is to be able to create commands that effectively loop,
+        // we can't do a standard equality check on them as we could very well end up with a stack overflow
+        // if a branch loops back to this node. However, they are still important here as two nodes that
+        // only differ by where they redirect to should not be considered the same node...
+        //
+        // Hence, we do a reference equality check.
+        return redirect == that.redirect;
     }
 
     @Override
     public int hashCode() {
-        return 31 * children.hashCode() + (command != null ? command.hashCode() : 0);
+        return 31 * children.hashCode() + (command != null ? command.hashCode() : 0) + System.identityHashCode(redirect);
     }
 
     public Predicate<S> getRequirement() {
diff --git a/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java b/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java
index e7525e55..55ad255b 100644
--- a/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java
+++ b/src/test/java/com/mojang/brigadier/tree/ArgumentCommandNodeTest.java
@@ -15,8 +15,10 @@
 import org.junit.Test;
 
 import static com.mojang.brigadier.arguments.IntegerArgumentType.integer;
+import static com.mojang.brigadier.builder.LiteralArgumentBuilder.literal;
 import static com.mojang.brigadier.builder.RequiredArgumentBuilder.argument;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.mock;
 
@@ -87,6 +89,13 @@ public void testEquals() throws Exception {
             .testEquals();
     }
 
+    @Test
+    public void testNodesWithDifferentRedirectsAreNotEqual() throws Exception {
+        final CommandNode<Object> redirectTarget = argument("baz", integer()).build();
+        assertThat(argument("foo", integer()).redirect(redirectTarget).build(), not(argument("foo", integer()).build()));
+        assertThat(literal("foo").redirect(redirectTarget).build(), not(literal("foo").build()));
+    }
+
     @Test
     public void testCreateBuilder() throws Exception {
         final RequiredArgumentBuilder<Object, Integer> builder = node.createBuilder();