From b8b6097878e73c6eac7db1278291b55851936d66 Mon Sep 17 00:00:00 2001
From: Velvet Toroyashi <42438262+VelvetToroyashi@users.noreply.github.com>
Date: Sat, 16 Mar 2024 20:08:05 -0400
Subject: [PATCH] refactor(trees): Respect immutability (sorta) when merging
 commands

This commit changes command merging to respect immutability; this comes at the cost of significantly more allocations which should be GC'd.
---
 Remora.Commands/Trees/CommandTreeBuilder.cs | 102 ++++++++++----------
 1 file changed, 53 insertions(+), 49 deletions(-)

diff --git a/Remora.Commands/Trees/CommandTreeBuilder.cs b/Remora.Commands/Trees/CommandTreeBuilder.cs
index 2d67219..e20442b 100644
--- a/Remora.Commands/Trees/CommandTreeBuilder.cs
+++ b/Remora.Commands/Trees/CommandTreeBuilder.cs
@@ -22,14 +22,12 @@
 
 using System;
 using System.Collections.Generic;
-using System.ComponentModel;
 using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
 using JetBrains.Annotations;
-using Microsoft.Extensions.DependencyInjection;
 using OneOf;
 using Remora.Commands.Attributes;
 using Remora.Commands.Builders;
@@ -93,10 +91,8 @@ public CommandTree Build()
         var rootChildren = new List<IChildNode>();
         var rootNode = new RootNode(rootChildren);
 
-        rootChildren.AddRange(ToChildNodes(_registeredModuleTypes, rootNode));
-
         var builtCommands = _registeredBuilders.Select(rb => rb.Match(cb => cb.Build(rootNode), gb => (IChildNode)gb.Build(rootNode)));
-        var topLevelCommands = BindDynamicCommands(rootChildren, builtCommands.ToList());
+        var topLevelCommands = BindDynamicCommands(ToChildNodes(_registeredModuleTypes, rootNode).ToArray(), builtCommands.ToList(), rootNode);
 
         rootChildren.AddRange(topLevelCommands);
 
@@ -108,81 +104,89 @@ public CommandTree Build()
     /// </summary>
     /// <param name="nodes">The nodes to bind to.</param>
     /// <param name="values">The values to bind.</param>
+    /// <param name="root">The root node to bind groups and commands to.</param>
     /// <returns>Any nodes that could not be bound, and thusly should be added directly to the root as-is.</returns>
-    private IEnumerable<IChildNode> BindDynamicCommands(IReadOnlyList<IChildNode> nodes, List<IChildNode> values)
+    private IEnumerable<IChildNode> BindDynamicCommands(IReadOnlyList<IChildNode> nodes, List<IChildNode> values, IParentNode root)
     {
         if (!values.Any())
         {
             yield break;
         }
 
-        for (int i = values.Count; i <= 0; i--)
-        {
-            // Child nodes are never merged; thunk it.
-            if (values[i] is not IParentNode)
-            {
-                yield return values[i];
-                values.RemoveAt(i);
-            }
-        }
-
-        var unified = nodes.Select(n => (node: n, dynamic: false)).Concat(values.Select(n => (node: n, dynamic: true)));
-
-        var grouped = unified.GroupBy(g => g.node is GroupNode gn ? gn.Key : string.Empty);
-
-        foreach (var group in grouped)
+        for (int i = values.Count - 1; i >= 0; i--)
         {
-            var dynamic = group.FirstOrDefault(v => v.dynamic).node;
-            var normal = group.FirstOrDefault(v => !v.dynamic).node;
+            var current = values[i];
 
-            if (dynamic is null || normal is null)
+            // Return top-level commands (non-group nodes) as-is.
+            if (current is IParentNode)
             {
-                yield return dynamic is null ? normal! : dynamic!;
                 continue;
             }
 
-            if (normal is not GroupNode ngn)
-            {
-                yield return dynamic;
-                continue;
-            }
+            values.RemoveAt(i);
+            yield return current;
+        }
 
-            if (dynamic is not GroupNode dgn)
-            {
-                // N.B This relies on the registration logic of Remora.Commands !!
-                // If `ToChildNodes` changes, so should this.
-                ((List<IChildNode>)ngn.Children).Add(dynamic);
-                continue;
-            }
+        if (values.Count is 0)
+        {
+            yield break;
+        }
 
-            MergeRecursively(ngn, dgn.Children);
+        var groups = nodes.OfType<GroupNode>()
+                           .Concat(values.Cast<GroupNode>())
+                           .GroupBy(g => g.Key)
+                           .Select(g => MergeRecursively(g.ToArray(), root));
+
+        foreach (var group in groups)
+        {
+            yield return group;
         }
     }
 
-    private void MergeRecursively(GroupNode group, IEnumerable<IChildNode> children)
+    private GroupNode MergeRecursively(IReadOnlyList<IChildNode> children, IParentNode parent)
     {
-        var groupChildren = (List<IChildNode>)group.Children;
+        var childNodes = new List<IChildNode>();
+        var groupNodesFromChildren = children.OfType<GroupNode>().ToArray();
+
+        var name = groupNodesFromChildren.Select(g => g.Key).FirstOrDefault(n => !string.IsNullOrWhiteSpace(n)) ?? string.Empty;
+        var description = groupNodesFromChildren.Select(g => g.Description).FirstOrDefault(d => !string.IsNullOrWhiteSpace(d)) ?? Constants.DefaultDescription;
+
+        var group = new GroupNode
+        (
+            groupNodesFromChildren.SelectMany(t => t.GroupTypes).ToArray(),
+            childNodes,
+            parent,
+            name,
+            groupNodesFromChildren.SelectMany(g => g.Aliases).ToArray(),
+            groupNodesFromChildren.SelectMany(g => g.Attributes).ToArray(),
+            groupNodesFromChildren.SelectMany(g => g.Conditions).ToArray(),
+            description
+        );
 
         foreach (var child in children)
         {
             if (child is not GroupNode cgn)
             {
-                groupChildren.Add(child);
+                childNodes.Add(child);
                 continue;
             }
 
-            ((List<Type>)group.GroupTypes).Add(typeof(GroupBuilder));
-            ((List<Attribute>)group.Attributes).AddRange(cgn.Attributes);
-            ((List<ConditionAttribute>)group.Conditions).AddRange(cgn.Conditions);
+            // Parity with ToChildNodes; if the group's name is empty
+            // Nest it directly under the parent.
+            if (string.IsNullOrWhiteSpace(cgn.Key))
+            {
+                childNodes.AddRange(cgn.Children);
+                continue;
+            }
 
-            foreach (var groupChild in group.Children)
+            // Otherwise, merge, recursively.
+            if (name == child.Key)
             {
-                if (groupChild.Key == child.Key && groupChild is GroupNode gcgn)
-                {
-                    MergeRecursively(gcgn, cgn.Children);
-                }
+                childNodes.Add(MergeRecursively(cgn.Children, group));
             }
         }
+
+        return group;
     }
 
     /// <summary>