Merge pull request #33304 from iocanel/fix-cli-plugin-args

Fix argument parsing for CLI plugins

devtools/cli/src/main/java/io/quarkus/cli/plugin/PluginCommandFactory.java

@@ -6,6 +6,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import io.quarkus.cli.common.OutputOptionMixin;
@@ -14,13 +15,17 @@
 import picocli.CommandLine;
 import picocli.CommandLine.Model.CommandSpec;
 import picocli.CommandLine.Model.ISetter;
-import picocli.CommandLine.Model.OptionSpec;
 import picocli.CommandLine.Model.PositionalParamSpec;
 
 public class PluginCommandFactory {
 
     private final OutputOptionMixin output;
 
+    //Testing
+    protected PluginCommandFactory() {
+        this(null);
+    }
+
     public PluginCommandFactory(OutputOptionMixin output) {
         this.output = output;
     }
@@ -44,14 +49,17 @@ private Optional<PluginCommand> createPluginCommand(Plugin plugin) {
      * Create a command for the specified plugin
      */
     public Optional<CommandSpec> createCommand(Plugin plugin) {
-        return createPluginCommand(plugin).map(command -> {
+        return createPluginCommand(plugin).map(createCommandSpec(plugin.getDescription().orElse("")));
+    }
+
+    public Function<PluginCommand, CommandSpec> createCommandSpec(String description) {
+        return command -> {
             CommandSpec spec = CommandSpec.wrapWithoutInspection(command);
-            String description = plugin.getDescription().orElse("");
             if (!StringUtil.isNullOrEmpty(description)) {
                 spec.usageMessage().description(description);
             }
             spec.parser().unmatchedArgumentsAllowed(true);
-            spec.addOption(OptionSpec.builder("options").type(Map.class).description("options").build()); //This is needed or options are ignored.
+            spec.parser().unmatchedOptionsArePositionalParams(true);
             spec.add(PositionalParamSpec.builder().type(String[].class).arity("0..*").description("Positional arguments")
                     .setter(new ISetter() {
                         @Override
@@ -67,7 +75,7 @@ public <T> T set(T value) throws Exception {
                         }
                     }).build());
             return spec;
-        });
+        };
     }
 
     /**

devtools/cli/src/test/java/io/quarkus/cli/plugin/PluginCommandFactoryTest.java

@@ -0,0 +1,198 @@
+package io.quarkus.cli.plugin;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.junit.jupiter.api.Test;
+
+import picocli.CommandLine;
+
+public class PluginCommandFactoryTest {
+
+    final PluginCommandFactory factory = new PluginCommandFactory();
+
+    @Test
+    void testNoArgs() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand));
+        //when
+        commandLine.execute(new String[0]);
+        //thew
+        assertEquals(0, rootCommand.getArguments().size());
+    }
+
+    @Test
+    void testSingleArg() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand));
+        //when
+        commandLine.execute("hello");
+        //then
+        assertEquals("hello", rootCommand.getArguments().get(0));
+    }
+
+    @Test
+    void testMultiArgs() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand));
+        //when
+        commandLine.execute("one", "two", "three");
+        //then
+        assertEquals("one", rootCommand.getArguments().get(0));
+        assertEquals("two", rootCommand.getArguments().get(1));
+        assertEquals("three", rootCommand.getArguments().get(2));
+    }
+
+    @Test
+    void testMultiArgsAndOptions() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand));
+        //when
+        commandLine.execute("one", "two", "three", "--depth", "5");
+        //then
+        assertEquals("one", rootCommand.getArguments().get(0));
+        assertEquals("two", rootCommand.getArguments().get(1));
+        assertEquals("three", rootCommand.getArguments().get(2));
+        assertEquals("--depth", rootCommand.getArguments().get(3));
+        assertEquals("5", rootCommand.getArguments().get(4));
+    }
+
+    @Test
+    void testSubCommandNoArgs() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        TestCommand subCommand = new TestCommand();
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand))
+                .addSubcommand("sub", new CommandLine(factory.createCommandSpec("sub command").apply(subCommand)));
+
+        //when
+        commandLine.execute(new String[0]);
+        //thew
+        assertEquals(0, rootCommand.getArguments().size());
+        assertEquals(0, subCommand.getArguments().size());
+    }
+
+    @Test
+    void testSubSingleArg() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        TestCommand subCommand = new TestCommand();
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand))
+                .addSubcommand("sub", new CommandLine(factory.createCommandSpec("sub command").apply(subCommand)));
+
+        //when
+        commandLine.execute("sub", "hello");
+        //then
+        assertEquals("hello", subCommand.getArguments().get(0));
+    }
+
+    @Test
+    void testSubMultiArgs() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        TestCommand subCommand = new TestCommand();
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand))
+                .addSubcommand("sub", new CommandLine(factory.createCommandSpec("sub command").apply(subCommand)));
+        //when
+        commandLine.execute("sub", "one", "two", "three");
+        //then
+        assertEquals("one", subCommand.getArguments().get(0));
+        assertEquals("two", subCommand.getArguments().get(1));
+        assertEquals("three", subCommand.getArguments().get(2));
+    }
+
+    @Test
+    void testSubMultiArgsAndOptions() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        TestCommand subCommand = new TestCommand();
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand))
+                .addSubcommand("sub", new CommandLine(factory.createCommandSpec("sub command").apply(subCommand)));
+        //when
+        commandLine.execute("sub", "one", "two", "three", "--depth", "5");
+        //then
+        assertEquals("one", subCommand.getArguments().get(0));
+        assertEquals("two", subCommand.getArguments().get(1));
+        assertEquals("three", subCommand.getArguments().get(2));
+        assertEquals("--depth", subCommand.getArguments().get(3));
+        assertEquals("5", subCommand.getArguments().get(4));
+    }
+
+    @Test
+    void testSecLevelSubCommandNoArgs() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        TestCommand subCommand = new TestCommand();
+        TestCommand secondLevelSubCommand = new TestCommand();
+
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand))
+                .addSubcommand("sub", new CommandLine(factory.createCommandSpec("sub command").apply(subCommand))
+                        .addSubcommand("sec-sub", new CommandLine(
+                                factory.createCommandSpec("secon level sub command").apply(secondLevelSubCommand))));
+
+        //when
+        commandLine.execute(new String[0]);
+        //thew
+        assertEquals(0, rootCommand.getArguments().size());
+        assertEquals(0, subCommand.getArguments().size());
+        assertEquals(0, secondLevelSubCommand.getArguments().size());
+    }
+
+    @Test
+    void testSecLevelSubSingleArg() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        TestCommand subCommand = new TestCommand();
+        TestCommand secondLevelSubCommand = new TestCommand();
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand))
+                .addSubcommand("sub", new CommandLine(factory.createCommandSpec("sub command").apply(subCommand))
+                        .addSubcommand("sec-sub", new CommandLine(
+                                factory.createCommandSpec("secon level sub command").apply(secondLevelSubCommand))));
+
+        //when
+        commandLine.execute("sub", "sec-sub", "hello");
+        //then
+        assertEquals("hello", secondLevelSubCommand.getArguments().get(0));
+    }
+
+    @Test
+    void testSecLevelSubMultiArgs() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        TestCommand subCommand = new TestCommand();
+        TestCommand secondLevelSubCommand = new TestCommand();
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand))
+                .addSubcommand("sub", new CommandLine(factory.createCommandSpec("sub command").apply(subCommand))
+                        .addSubcommand("sec-sub", new CommandLine(
+                                factory.createCommandSpec("secon level sub command").apply(secondLevelSubCommand))));
+        //when
+        commandLine.execute("sub", "sec-sub", "one", "two", "three");
+        //then
+        assertEquals("one", secondLevelSubCommand.getArguments().get(0));
+        assertEquals("two", secondLevelSubCommand.getArguments().get(1));
+        assertEquals("three", secondLevelSubCommand.getArguments().get(2));
+    }
+
+    @Test
+    void testSecLevelSubMultiArgsAndOptions() {
+        //given
+        TestCommand rootCommand = new TestCommand();
+        TestCommand subCommand = new TestCommand();
+        TestCommand secondLevelSubCommand = new TestCommand();
+        CommandLine commandLine = new CommandLine(factory.createCommandSpec("rootCommand command").apply(rootCommand))
+                .addSubcommand("sub", new CommandLine(factory.createCommandSpec("sub command").apply(subCommand))
+                        .addSubcommand("sec-sub", new CommandLine(
+                                factory.createCommandSpec("secon level sub command").apply(secondLevelSubCommand))));
+        //when
+        commandLine.execute("sub", "sec-sub", "one", "two", "three", "--depth", "5");
+        //then
+        assertEquals("one", secondLevelSubCommand.getArguments().get(0));
+        assertEquals("two", secondLevelSubCommand.getArguments().get(1));
+        assertEquals("three", secondLevelSubCommand.getArguments().get(2));
+        assertEquals("--depth", secondLevelSubCommand.getArguments().get(3));
+        assertEquals("5", secondLevelSubCommand.getArguments().get(4));
+    }
+}

devtools/cli/src/test/java/io/quarkus/cli/plugin/TestCommand.java

@@ -0,0 +1,52 @@
+package io.quarkus.cli.plugin;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import io.quarkus.cli.common.OutputOptionMixin;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+
+@Command
+public class TestCommand implements PluginCommand {
+
+    private static final String DEFAULT_CMD = "cmd";
+
+    private final String cmd;
+    private final List<String> arguments = new ArrayList<>();
+
+    public TestCommand() {
+        this(DEFAULT_CMD);
+    }
+
+    public TestCommand(String cmd) {
+        this.cmd = cmd;
+    }
+
+    @Override
+    public Integer call() throws Exception {
+        System.out.println("Calling " + cmd + " " + String.join(" ", arguments));
+        return CommandLine.ExitCode.OK;
+    }
+
+    @Override
+    public List<String> getCommand() {
+        return List.of(cmd);
+    }
+
+    @Override
+    public List<String> getArguments() {
+        return arguments;
+    }
+
+    @Override
+    public void useArguments(List<String> arguments) {
+        this.arguments.clear();
+        this.arguments.addAll(arguments);
+    }
+
+    @Override
+    public OutputOptionMixin getOutput() {
+        return null;
+    }
+}