feat(core): make CommandComponent immutable

The "owning command" has been moved into the command node, as the node is always present when we access the command anyway. This allows us to only have final fields in the command component. This also means that we no longer need to copy the components before storing them in the command tree, and we can instead re-use the components.
Incendo · Jan 13, 2024 · aafa2dd · aafa2dd
1 parent e8dabd6
commit aafa2dd
Show file tree

Hide file tree

Showing 11 changed files with 68 additions and 110 deletions.
diff --git a/...ations/src/test/java/cloud/commandframework/annotations/feature/BuilderDecoratorTest.java b/...ations/src/test/java/cloud/commandframework/annotations/feature/BuilderDecoratorTest.java
@@ -60,7 +60,7 @@ void testDefaultDescription() {
         this.annotationParser.parse(new TestClass());
 
         // Assert
-        assertThat(this.commandManager.commandTree().getNamedNode("command").component().owningCommand().commandDescription())
+        assertThat(this.commandManager.commandTree().getNamedNode("command").command().commandDescription())
                 .isEqualTo(CommandDescription.commandDescription("default description"));
     }
 
@@ -74,7 +74,7 @@ void testDefaultPermission() {
         this.annotationParser.parse(new TestClass());
 
         // Assert
-        assertThat(this.commandManager.commandTree().getNamedNode("command").component().owningCommand().commandPermission())
+        assertThat(this.commandManager.commandTree().getNamedNode("command").command().commandPermission())
                 .isEqualTo(Permission.of("default.permission"));
     }
 

diff --git a/...tions/src/test/java/cloud/commandframework/annotations/feature/DescriptionMapperTest.java b/...tions/src/test/java/cloud/commandframework/annotations/feature/DescriptionMapperTest.java
@@ -63,7 +63,7 @@ void test() {
         // Assert
         final CommandNode<?> node = this.commandManager.commandTree().getNamedNode("command");
         final CommandNode<?> argumentNode = node.children().get(0);
-        assertThat(argumentNode.component().owningCommand().commandDescription().description()).isEqualTo(testDescription);
+        assertThat(argumentNode.command().commandDescription().description()).isEqualTo(testDescription);
         assertThat(node.children().get(0).component().description()).isEqualTo(testDescription);
     }
 

diff --git a/cloud-core/src/main/java/cloud/commandframework/Command.java b/cloud-core/src/main/java/cloud/commandframework/Command.java
@@ -1442,8 +1442,6 @@ private Builder(
 
         /**
          * Adds the given {@code argument} to the command.
-         * <p>
-         * The component will be copied using {@link CommandComponent#copy()} before being inserted into the command tree.
          *
          * @param argument argument to add
          * @return new builder instance with the command argument inserted into the argument list
@@ -1454,7 +1452,7 @@ private Builder(
                 final @NonNull CommandComponent<C> argument
         ) {
             final List<CommandComponent<C>> commandComponents = new ArrayList<>(this.commandComponents);
-            commandComponents.add(argument.copy());
+            commandComponents.add(argument);
             return new Builder<>(
                     this.commandManager,
                     this.commandMeta,
@@ -1469,8 +1467,6 @@ private Builder(
 
         /**
          * Adds the given {@code argument} to the command.
-         * <p>
-         * The component will be copied using {@link CommandComponent#copy()} before being inserted into the command tree.
          *
          * @param builder builder that builds the component to add
          * @return new builder instance with the command argument inserted into the argument list
@@ -2237,7 +2233,7 @@ private Builder(
         /**
          * Makes the current command be a proxy of the supplied command. T
          * <p>
-         * his means that all the proxied command's variable command arguments will be inserted into this
+         * This means that all the proxied command's variable command arguments will be inserted into this
          * builder instance, in the order they are declared in the proxied command. Furthermore,
          * the proxied command's command handler will be shown by the command that is currently
          * being built. If the current command builder does not have a permission node set, this
@@ -2252,12 +2248,15 @@ private Builder(
                 if (component.type() == CommandComponent.ComponentType.LITERAL) {
                     continue;
                 }
-                final CommandComponent<C> componentCopy = component.copy();
-                builder = builder.argument(componentCopy);
+                builder = builder.argument(component);
             }
             if (this.permission.permissionString().isEmpty()) {
                 builder = builder.permission(command.commandPermission());
             }
+            final Class<? extends C> senderType = command.senderType().orElse(null);
+            if (senderType != null) {
+                builder = builder.senderType(senderType);
+            }
             return builder.handler(command.commandExecutionHandler);
         }
 

diff --git a/cloud-core/src/main/java/cloud/commandframework/CommandComponent.java b/cloud-core/src/main/java/cloud/commandframework/CommandComponent.java
@@ -42,7 +42,6 @@
 import java.util.Collections;
 import java.util.Objects;
 import org.apiguardian.api.API;
-import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
 import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.checkerframework.common.returnsreceiver.qual.This;
@@ -68,8 +67,6 @@ public class CommandComponent<C> implements Comparable<CommandComponent<C>>, Pre
     private final SuggestionProvider<C> suggestionProvider;
     private final Collection<@NonNull ComponentPreprocessor<C>> componentPreprocessors;
 
-    private Command<C> owningCommand;
-
     /**
      * Creates a new mutable builder.
      *
@@ -247,29 +244,6 @@ public final boolean hasDefaultValue() {
         return this.suggestionProvider;
     }
 
-    /**
-     * Returns the command that owns this component.
-     *
-     * @return the owning command
-     */
-    public final @MonotonicNonNull Command<C> owningCommand() {
-        return this.owningCommand;
-    }
-
-    /**
-     * Sets the command that owns this component.
-     *
-     * @param owningCommand the command
-     * @throws IllegalStateException if {@link #owningCommand()} has already been set
-     */
-    @API(status = API.Status.INTERNAL, consumers = "cloud.commandframework.CommandTree")
-    final void owningCommand(final @NonNull Command<C> owningCommand) {
-        if (this.owningCommand != null) {
-            throw new IllegalStateException("Cannot replace owning command");
-        }
-        this.owningCommand = owningCommand;
-    }
-
     /**
      * Registers a new preprocessor.
      *
@@ -347,24 +321,6 @@ public final boolean equals(final Object o) {
         );
     }
 
-    /**
-     * Returns a copy of the component.
-     *
-     * @return copy of the component
-     */
-    public @NonNull CommandComponent<C> copy() {
-        return new CommandComponent<>(
-                this.name(),
-                this.parser(),
-                this.valueType(),
-                this.description(),
-                this.type(),
-                this.defaultValue(),
-                this.suggestionProvider(),
-                this.preprocessors()
-        );
-    }
-
     @Override
     public final int compareTo(final @NonNull CommandComponent<C> other) {
         if (this.type() == ComponentType.LITERAL) {

diff --git a/cloud-core/src/main/java/cloud/commandframework/CommandTree.java b/cloud-core/src/main/java/cloud/commandframework/CommandTree.java
@@ -245,7 +245,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
         // There are 0 or more static arguments as children. No variable child arguments are present
         if (root.children().isEmpty()) {
             final CommandComponent<C> rootComponent = root.component();
-            if (rootComponent == null || rootComponent.owningCommand() == null || !commandInput.isEmpty()) {
+            if (rootComponent == null || root.command() == null || !commandInput.isEmpty()) {
                 // Too many arguments. We have a unique path, so we can send the entire context
                 return CompletableFutures.failedFuture(
                         new InvalidSyntaxException(
@@ -255,7 +255,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
                         )
                 );
             }
-            return CompletableFuture.completedFuture(rootComponent.owningCommand());
+            return CompletableFuture.completedFuture(root.command());
         }
 
         CompletableFuture<Command<C>> childCompletable = CompletableFuture.completedFuture(null);
@@ -318,8 +318,8 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
 
                     // If we couldn't match a child, check if there's a command attached and execute it
                     final CommandComponent<C> rootComponent = root.component();
-                    if (rootComponent != null && rootComponent.owningCommand() != null && commandInput.isEmpty()) {
-                        final Command<C> command = rootComponent.owningCommand();
+                    if (rootComponent != null && root.command() != null && commandInput.isEmpty()) {
+                        final Command<C> command = root.command();
                         if (!this.commandManager().hasPermission(
                                 commandContext.sender(),
                                 command.commandPermission()
@@ -332,7 +332,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
                                     )
                             );
                         }
-                        return CompletableFuture.completedFuture(rootComponent.owningCommand());
+                        return CompletableFuture.completedFuture(root.command());
                     }
 
                     // We know that there's no command, and we also cannot match any of the children
@@ -410,25 +410,23 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
                     argumentValue = defaultValue.evaluateDefault(commandContext);
                 }
             } else if (!child.component().required()) {
-                if (childComponent.owningCommand() == null) {
+                if (child.command() == null) {
                     // If there are multiple children with different owning commands then it's ambiguous and
                     // not allowed, therefore we're able to pick any child command, as long as we can find it
                     CommandNode<C> node = child;
                     while (!node.isLeaf()) {
                         node = node.children().get(0);
                         final CommandComponent<C> nodeComponent = node.component();
-                        if (nodeComponent != null && nodeComponent.owningCommand() != null) {
-                            childComponent.owningCommand(nodeComponent.owningCommand());
+                        if (nodeComponent != null && node.command() != null) {
+                            child.command(node.command());
                         }
                     }
                 }
-                return CompletableFuture.completedFuture(childComponent.owningCommand());
+                return CompletableFuture.completedFuture(child.command());
             } else if (child.isLeaf()) {
                 final CommandComponent<C> rootComponent = root.component();
-                if (rootComponent == null || rootComponent.owningCommand() == null) {
-                    final List<CommandComponent<C>> components = Objects.requireNonNull(
-                            childComponent.owningCommand()
-                    ).components();
+                if (rootComponent == null || root.command() == null) {
+                    final List<CommandComponent<C>> components = Objects.requireNonNull(child.command()).components();
                     return CompletableFutures.failedFuture(
                             new InvalidSyntaxException(
                                     this.commandManager.commandSyntaxFormatter()
@@ -439,7 +437,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
                     );
                 }
 
-                final Command<C> command = rootComponent.owningCommand();
+                final Command<C> command = root.command();
                 if (this.commandManager().hasPermission(commandContext.sender(), command.commandPermission())) {
                     return CompletableFuture.completedFuture(command);
                 }
@@ -453,7 +451,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
             } else {
                 // The child is not a leaf, but may have an intermediary executor, attempt to use it
                 final CommandComponent<C> rootComponent = root.component();
-                if (rootComponent == null || rootComponent.owningCommand() == null) {
+                if (rootComponent == null || root.command() == null) {
                     // Child does not have a command, and so we cannot proceed
                     return CompletableFutures.failedFuture(
                             new InvalidSyntaxException(
@@ -466,7 +464,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
                 }
 
                 // If the sender has permission to use the command, then we're completely done
-                final Command<C> command = Objects.requireNonNull(rootComponent.owningCommand());
+                final Command<C> command = Objects.requireNonNull(root.command());
                 if (this.commandManager().hasPermission(commandContext.sender(), command.commandPermission())) {
                     return CompletableFuture.completedFuture(command);
                 }
@@ -501,7 +499,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
            commandContext.store(component.name(), value);
            if (child.isLeaf()) {
                if (commandInput.isEmpty()) {
-                   return CompletableFuture.completedFuture(component.owningCommand());
+                   return CompletableFuture.completedFuture(child.command());
                }
                return CompletableFutures.failedFuture(
                        new InvalidSyntaxException(
@@ -932,14 +930,14 @@ public void insertCommand(final @NonNull Command<C> command) {
 
             final CommandComponent<C> nodeComponent = node.component();
             if (nodeComponent != null) {
-                if (nodeComponent.owningCommand() != null) {
+                if (node.command() != null) {
                     throw new IllegalStateException(String.format(
                             "Duplicate command chains detected. Node '%s' already has an owning command (%s)",
-                            node, nodeComponent.owningCommand()
+                            node, node.command()
                     ));
                 }
 
-                nodeComponent.owningCommand(command);
+                node.command(command);
             }
 
             this.verifyAndRegister();
@@ -1022,10 +1020,10 @@ public void verifyAndRegister() {
 
         // Verify that all leaf nodes have command registered
         this.getLeaves(this.internalTree).forEach(leaf -> {
-            if (leaf.component().owningCommand() == null) {
+            if (leaf.command() == null) {
                 throw new NoCommandInLeafException(leaf.component());
             } else {
-                final Command<C> owningCommand = leaf.component().owningCommand();
+                final Command<C> owningCommand = leaf.command();
                 this.commandManager.commandRegistrationHandler().registerCommand(owningCommand);
             }
         });
@@ -1041,8 +1039,8 @@ public void verifyAndRegister() {
      */
     @SuppressWarnings("unchecked")
     private void propagateRequirements(final @NonNull CommandNode<C> leafNode) {
-        final Permission commandPermission = leafNode.component().owningCommand().commandPermission();
-        Class<?> senderType = leafNode.component().owningCommand().senderType().orElse(null);
+        final Permission commandPermission = leafNode.command().commandPermission();
+        Class<?> senderType = leafNode.command().senderType().orElse(null);
         if (senderType == null) {
             senderType = Object.class;
         }
@@ -1065,8 +1063,8 @@ private void propagateRequirements(final @NonNull CommandNode<C> leafNode) {
             }
 
             /* Now also check if there's a command handler attached to an upper level node */
-            if (commandArgumentNode.component() != null && commandArgumentNode.component().owningCommand() != null) {
-                final Command<C> command = commandArgumentNode.component().owningCommand();
+            if (commandArgumentNode.component() != null && commandArgumentNode.command() != null) {
+                final Command<C> command = commandArgumentNode.command();
                 if (this.commandManager().settings().get(ManagerSetting.ENFORCE_INTERMEDIARY_PERMISSIONS)) {
                     permission = command.commandPermission();
                 } else {
@@ -1230,7 +1228,7 @@ void deleteRecursively(
         }
 
         final @Nullable CommandComponent<C> component = node.component();
-        final @Nullable Command<C> owner = component == null ? null : component.owningCommand();
+        final @Nullable Command<C> owner = component == null ? null : node.command();
         if (owner != null) {
             commandConsumer.accept(owner);
         }

diff --git a/cloud-core/src/main/java/cloud/commandframework/TypedCommandComponent.java b/cloud-core/src/main/java/cloud/commandframework/TypedCommandComponent.java
@@ -72,18 +72,4 @@ public final class TypedCommandComponent<C, T> extends CommandComponent<C> imple
     public @NonNull CloudKey<T> key() {
         return CloudKey.of(this.name(), this.valueType());
     }
-
-    @Override
-    public @NonNull TypedCommandComponent<C, T> copy() {
-        return new TypedCommandComponent<>(
-                this.name(),
-                this.parser(),
-                this.valueType(),
-                this.description(),
-                this.type(),
-                this.defaultValue(),
-                this.suggestionProvider(),
-                this.preprocessors()
-        );
-    }
 }
diff --git a/cloud-core/src/main/java/cloud/commandframework/help/StandardHelpHandler.java b/cloud-core/src/main/java/cloud/commandframework/help/StandardHelpHandler.java
@@ -140,17 +140,15 @@ public StandardHelpHandler(
             ++index;
             traversedNodes.add(head.component());
 
-            if (head.component() != null && head.component().owningCommand() != null) {
+            if (head.component() != null && head.command() != null) {
                 if (head.isLeaf() || index == queryFragments.size()) {
-                    if (this.commandManager.hasPermission(query.sender(), head.component()
-                            .owningCommand()
-                            .commandPermission())) {
+                    if (this.commandManager.hasPermission(query.sender(), head.command().commandPermission())) {
                         return VerboseCommandResult.of(
                                 query,
                                 CommandEntry.of(
-                                        head.component().owningCommand(),
+                                        head.command(),
                                         this.commandManager.commandSyntaxFormatter()
-                                                .apply(query.sender(), head.component().owningCommand().components(), null)
+                                                .apply(query.sender(), head.command().components(), null)
                                 )
                         );
                     }
@@ -193,9 +191,9 @@ public StandardHelpHandler(
                     }
 
                     final List<CommandComponent<C>> traversedNodesSub = new LinkedList<>(traversedNodes);
-                    if (child.component() == null || child.component().owningCommand() == null
+                    if (child.component() == null || child.command() == null
                             || this.commandManager.hasPermission(query.sender(),
-                            child.component().owningCommand().commandPermission()
+                            child.command().commandPermission()
                     )) {
                         traversedNodesSub.add(child.component());
                         childSuggestions.add(this.commandManager.commandSyntaxFormatter()
@@ -241,7 +239,7 @@ protected boolean isNodeVisible(final @NonNull CommandNode<C> node) {
         /* Check node is itself a command that is visible */
         final CommandComponent<C> component = node.component();
         if (component != null) {
-            final Command<C> owningCommand = component.owningCommand();
+            final Command<C> owningCommand = node.command();
             if (owningCommand != null && this.commandFilter.test(owningCommand)) {
                 return true;
             }