Allow @mixin to be used for methods as well

knutwalker · knutwalker · commit f7ce65d4d0ea · 2018-10-22T10:39:49.000+02:00
diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java
@@ -2847,7 +2847,7 @@ private static class NoCompletionCandidates implements Iterable<String> {
      * @since 3.0
      */
     @Retention(RetentionPolicy.RUNTIME)
-    @Target(ElementType.FIELD)
+    @Target({ElementType.FIELD, ElementType.PARAMETER})
     public @interface Mixin {
         /** Optionally specify a name that the mixin object can be retrieved with from the {@code CommandSpec}.
          * If not specified the name of the annotated field is used.
@@ -3718,15 +3718,28 @@ private void initResourceBundle(ResourceBundle bundle) {
              * (class methods annotated with {@code @Command}) as subcommands.
              *
              * @return this {@link CommandSpec} object for method chaining
+             * @see #addMethodSubcommands(IFactory)
              * @see #addSubcommand(String, CommandLine)
              * @since 3.6.0
              */
             public CommandSpec addMethodSubcommands() {
+                addMethodSubcommands(new DefaultFactory());
+                return this;
+            }
+
+            /** Reflects on the class of the {@linkplain #userObject() user object} and registers any command methods
+             * (class methods annotated with {@code @Command}) as subcommands.
+             *
+             * @return this {@link CommandSpec} object for method chaining
+             * @see #addSubcommand(String, CommandLine)
+             * @since 3.7.0
+             */
+            public CommandSpec addMethodSubcommands(IFactory factory) {
                 if (userObject() instanceof Method) {
                      throw new UnsupportedOperationException("cannot discover methods of non-class: " + userObject());
                 }
                 for (Method method : getCommandMethods(userObject().getClass(), null)) {
-                    CommandLine cmd = new CommandLine(method);
+                    CommandLine cmd = new CommandLine(method, factory);
                     addSubcommand(cmd.getCommandName(), cmd);
                 }
                 return this;
@@ -3852,9 +3865,41 @@ public CommandSpec addMixin(String name, CommandSpec mixin) {
              * @return an immutable list of all options and positional parameters for this command. */
             public List<ArgSpec> args() { return Collections.unmodifiableList(args); }
             Object[] argValues() {
-                int shift = mixinStandardHelpOptions() ? 2 : 0;
-                Object[] values = new Object[args.size() - shift];
-                for (int i = 0; i < values.length; i++) { values[i] = args.get(i + shift).getValue(); }
+                Map<Class<?>, CommandSpec> allMixins = null;
+                int argsLength = args.size();
+                int shift = 0;
+                for (Map.Entry<String, CommandSpec> mixinEntry : mixins.entrySet()) {
+                    if (mixinEntry.getKey().equals(AutoHelpMixin.KEY)) {
+                        shift = 2;
+                        argsLength -= shift;
+                        continue;
+                    }
+                    CommandSpec mixin = mixinEntry.getValue();
+                    int mixinArgs = mixin.args.size();
+                    argsLength -= (mixinArgs - 1); // sub 1 less b/c that's the mixin
+                    if (allMixins == null) {
+                        allMixins = new IdentityHashMap<Class<?>, CommandSpec>(mixins.size());
+                    }
+                    allMixins.put(mixin.userObject.getClass(), mixin);
+                }
+
+                Object[] values = new Object[argsLength];
+                if (allMixins == null) {
+                    for (int i = 0; i < values.length; i++) { values[i] = args.get(i + shift).getValue(); }
+                } else {
+                    int argIndex = shift;
+                    Class<?>[] methodParams = ((Method) userObject).getParameterTypes();
+                    for (int i = 0; i < methodParams.length; i++) {
+                        final Class<?> param = methodParams[i];
+                        CommandSpec mixin = allMixins.remove(param);
+                        if (mixin == null) {
+                            values[i] = args.get(argIndex++).getValue();
+                        } else {
+                            values[i] = mixin.userObject;
+                            argIndex += mixin.args.size();
+                        }
+                    }
+                }
                 return values;
             }
 
@@ -5640,7 +5685,7 @@ static boolean isAnnotated(AnnotatedElement e) {
             boolean isAnnotationPresent(Class<? extends Annotation> annotationClass) { return accessible.isAnnotationPresent(annotationClass); }
             <T extends Annotation> T getAnnotation(Class<T> annotationClass) { return accessible.getAnnotation(annotationClass); }
             String name()            { return name; }
-            boolean isArgSpec()      { return isOption() || isParameter() || isMethodParameter(); }
+            boolean isArgSpec()      { return isOption() || isParameter() || (isMethodParameter() && !isMixin()); }
             boolean isOption()       { return isAnnotationPresent(Option.class); }
             boolean isParameter()    { return isAnnotationPresent(Parameters.class); }
             boolean isMixin()        { return isAnnotationPresent(Mixin.class); }
@@ -5903,7 +5948,7 @@ private static void initSubcommands(Command cmd, CommandSpec parent, IFactory fa
                     }
                 }
                 if (cmd.addMethodSubcommands() && !(parent.userObject() instanceof Method)) {
-                    parent.addMethodSubcommands();
+                    parent.addMethodSubcommands(factory);
                 }
             }
             static void initParentCommand(Object subcommand, Object parent) {
@@ -5971,7 +6016,7 @@ private static boolean initFromMethodParameters(Object scope, Method method, Com
                 int optionCount = 0;
                 for (int i = 0, count = method.getParameterTypes().length; i < count; i++) {
                     MethodParam param = new MethodParam(method, i);
-                    if (param.isAnnotationPresent(Option.class)) {
+                    if (param.isAnnotationPresent(Option.class) || param.isAnnotationPresent(Mixin.class)) {
                         optionCount++;
                     } else {
                         param.position = i - optionCount;
diff --git a/src/test/java/picocli/CommandLineCommandMethodTest.java b/src/test/java/picocli/CommandLineCommandMethodTest.java
@@ -22,6 +22,7 @@
 import org.junit.contrib.java.lang.system.ProvideSystemProperty;
 import org.junit.contrib.java.lang.system.SystemErrRule;
 import org.junit.contrib.java.lang.system.SystemOutRule;
+import picocli.CommandLine.Mixin;
 import picocli.CommandLineTest.CompactFields;
 
 import java.io.ByteArrayOutputStream;
@@ -166,6 +167,226 @@ public void testAnnotateMethod_unannotatedPositionalMixedWithOptions_indexByPara
         assertEquals(String.class, spec.findOption("-c").type());
     }
 
+    @Command static class SomeMixin {
+        @Option(names = "-a") int a;
+        @Option(names = "-b") long b;
+    }
+
+    static class UnannotatedClassWithMixinParameters {
+        @Command
+        void withMixin(@Mixin SomeMixin mixin) {
+        }
+
+        @Command
+        void posAndMixin(int[] x, @Mixin SomeMixin mixin) {
+        }
+
+        @Command
+        void posAndOptAndMixin(int[] x, @Option(names = "-y") String[] y, @Mixin SomeMixin mixin) {
+        }
+
+        @Command
+        void mixinFirst(@Mixin SomeMixin mixin, int[] x, @Option(names = "-y") String[] y) {
+        }
+    }
+
+    @Test
+    public void testAnnotateMethod_mixinParameter() {
+        Method m = CommandLine.getCommandMethods(UnannotatedClassWithMixinParameters.class, "withMixin").get(0);
+        CommandLine cmd = new CommandLine(m);
+        Model.CommandSpec spec = cmd.getCommandSpec();
+        assertEquals(1, spec.mixins().size());
+        spec = spec.mixins().get("arg0");
+        assertEquals(SomeMixin.class, spec.userObject().getClass());
+    }
+
+    @Test
+    public void testAnnotateMethod_positionalAndMixinParameter() {
+        Method m = CommandLine.getCommandMethods(UnannotatedClassWithMixinParameters.class, "posAndMixin").get(0);
+        CommandLine cmd = new CommandLine(m);
+        Model.CommandSpec spec = cmd.getCommandSpec();
+        assertEquals(1, spec.mixins().size());
+        assertEquals(1, spec.positionalParameters().size());
+        spec = spec.mixins().get("arg1");
+        assertEquals(SomeMixin.class, spec.userObject().getClass());
+    }
+
+    @Test
+    public void testAnnotateMethod_positionalAndOptionsAndMixinParameter() {
+        Method m = CommandLine.getCommandMethods(UnannotatedClassWithMixinParameters.class, "posAndOptAndMixin").get(0);
+        CommandLine cmd = new CommandLine(m);
+        Model.CommandSpec spec = cmd.getCommandSpec();
+        assertEquals(1, spec.mixins().size());
+        assertEquals(1, spec.positionalParameters().size());
+        assertEquals(3, spec.options().size());
+        spec = spec.mixins().get("arg2");
+        assertEquals(SomeMixin.class, spec.userObject().getClass());
+    }
+
+    @Test
+    public void testAnnotateMethod_mixinParameterFirst() {
+        Method m = CommandLine.getCommandMethods(UnannotatedClassWithMixinParameters.class, "mixinFirst").get(0);
+        CommandLine cmd = new CommandLine(m);
+        Model.CommandSpec spec = cmd.getCommandSpec();
+        assertEquals(1, spec.mixins().size());
+        assertEquals(1, spec.positionalParameters().size());
+        assertEquals(3, spec.options().size());
+        spec = spec.mixins().get("arg0");
+        assertEquals(SomeMixin.class, spec.userObject().getClass());
+    }
+
+    static class UnannotatedClassWithMixinAndOptionsAndPositionals {
+        @Command(name="sum")
+        long sum(@Option(names = "-y") String[] y, @Mixin SomeMixin subMixin, int[] x) {
+            return y.length + subMixin.a + subMixin.b + x.length;
+        }
+    }
+
+    @Test
+    public void testUnannotatedCommandWithMixin() throws Exception {
+        Method m = CommandLine.getCommandMethods(UnannotatedClassWithMixinAndOptionsAndPositionals.class, "sum").get(0);
+        CommandLine commandLine = new CommandLine(m);
+        List<CommandLine> parsed = commandLine.parse("-y foo -y bar -a 7 -b 11 13 42".split(" "));
+        assertEquals(1, parsed.size());
+
+        // get method args
+        Object[] methodArgValues = parsed.get(0).getCommandSpec().argValues();
+        assertNotNull(methodArgValues);
+
+        // verify args
+        String[] arg0 = (String[]) methodArgValues[0];
+        assertArrayEquals(new String[] {"foo", "bar"}, arg0);
+        SomeMixin arg1 = (SomeMixin) methodArgValues[1];
+        assertEquals(7, arg1.a);
+        assertEquals(11, arg1.b);
+        int[] arg2 = (int[]) methodArgValues[2];
+        assertArrayEquals(new int[] {13, 42}, arg2);
+
+        // verify method is callable with args
+        long result = (Long) m.invoke(new UnannotatedClassWithMixinAndOptionsAndPositionals(), methodArgValues);
+        assertEquals(22, result);
+
+        // verify same result with result handler
+        List<Object> results = new CommandLine.RunLast().handleParseResult(parsed, System.out, CommandLine.Help.Ansi.OFF);
+        assertEquals(1, results.size());
+        assertEquals(22L, results.get(0));
+    }
+
+    @Command
+    static class AnnotatedClassWithMixinParameters {
+        @Mixin SomeMixin mixin;
+
+        @Command(name="sum")
+        long sum(@Option(names = "-y") String[] y, @Mixin SomeMixin subMixin, int[] x) {
+            return mixin.a + mixin.b + y.length + subMixin.a + subMixin.b + x.length;
+        }
+    }
+
+    @Test
+    public void testAnnotatedSubcommandWithDoubleMixin() throws Exception {
+        AnnotatedClassWithMixinParameters command = new AnnotatedClassWithMixinParameters();
+        CommandLine commandLine = new CommandLine(command);
+        List<CommandLine> parsed = commandLine.parse("-a 3 -b 5 sum -y foo -y bar -a 7 -b 11 13 42".split(" "));
+        assertEquals(2, parsed.size());
+
+        // get method args
+        Object[] methodArgValues = parsed.get(1).getCommandSpec().argValues();
+        assertNotNull(methodArgValues);
+
+        // verify args
+        String[] arg0 = (String[]) methodArgValues[0];
+        assertArrayEquals(new String[] {"foo", "bar"}, arg0);
+        SomeMixin arg1 = (SomeMixin) methodArgValues[1];
+        assertEquals(7, arg1.a);
+        assertEquals(11, arg1.b);
+        int[] arg2 = (int[]) methodArgValues[2];
+        assertArrayEquals(new int[] {13, 42}, arg2);
+
+        // verify method is callable with args
+        Method m = AnnotatedClassWithMixinParameters.class.getDeclaredMethod("sum", String[].class, SomeMixin.class, int[].class);
+        long result = (Long) m.invoke(command, methodArgValues);
+        assertEquals(30, result);
+
+        // verify same result with result handler
+        List<Object> results = new CommandLine.RunLast().handleParseResult(parsed, System.out, CommandLine.Help.Ansi.OFF);
+        assertEquals(1, results.size());
+        assertEquals(30L, results.get(0));
+    }
+
+    @Command static class OtherMixin {
+        @Option(names = "-c") int c;
+    }
+
+    static class AnnotatedClassWithMultipleMixinParameters {
+        @Command(name="sum")
+        long sum(@Mixin SomeMixin mixin1, @Mixin OtherMixin mixin2) {
+            return mixin1.a + mixin1.b + mixin2.c;
+        }
+    }
+
+    @Test
+    public void testAnnotatedMethodMultipleMixinsSubcommandWithDoubleMixin() throws Exception {
+        Method m = CommandLine.getCommandMethods(AnnotatedClassWithMultipleMixinParameters.class, "sum").get(0);
+        CommandLine commandLine = new CommandLine(m);
+        List<CommandLine> parsed = commandLine.parse("-a 3 -b 5 -c 7".split(" "));
+        assertEquals(1, parsed.size());
+
+        // get method args
+        Object[] methodArgValues = parsed.get(0).getCommandSpec().argValues();
+        assertNotNull(methodArgValues);
+
+        // verify args
+        SomeMixin arg0 = (SomeMixin) methodArgValues[0];
+        assertEquals(3, arg0.a);
+        assertEquals(5, arg0.b);
+        OtherMixin arg1 = (OtherMixin) methodArgValues[1];
+        assertEquals(7, arg1.c);
+
+        // verify method is callable with args
+        long result = (Long) m.invoke(new AnnotatedClassWithMultipleMixinParameters(), methodArgValues);
+        assertEquals(15, result);
+
+        // verify same result with result handler
+        List<Object> results = new CommandLine.RunLast().handleParseResult(parsed, System.out, CommandLine.Help.Ansi.OFF);
+        assertEquals(1, results.size());
+        assertEquals(15L, results.get(0));
+    }
+
+    @Command static class EmptyMixin {}
+
+    static class AnnotatedClassWithMultipleEmptyParameters {
+        @Command(name="sum")
+        long sum(@Option(names = "-a") int a, @Mixin EmptyMixin mixin) {
+            return a;
+        }
+    }
+
+    @Test
+    public void testAnnotatedMethodMultipleMixinsSubcommandWithEmptyMixin() throws Exception {
+        Method m = CommandLine.getCommandMethods(AnnotatedClassWithMultipleEmptyParameters.class, "sum").get(0);
+        CommandLine commandLine = new CommandLine(m);
+        List<CommandLine> parsed = commandLine.parse("-a 3".split(" "));
+        assertEquals(1, parsed.size());
+
+        // get method args
+        Object[] methodArgValues = parsed.get(0).getCommandSpec().argValues();
+        assertNotNull(methodArgValues);
+
+        // verify args
+        int arg0 = (Integer) methodArgValues[0];
+        assertEquals(3, arg0);
+        EmptyMixin arg1 = (EmptyMixin) methodArgValues[1];
+
+        // verify method is callable with args
+        long result = (Long) m.invoke(new AnnotatedClassWithMultipleEmptyParameters(), methodArgValues);
+        assertEquals(3, result);
+
+        // verify same result with result handler
+        List<Object> results = new CommandLine.RunLast().handleParseResult(parsed, System.out, CommandLine.Help.Ansi.OFF);
+        assertEquals(1, results.size());
+        assertEquals(3L, results.get(0));
+    }
+
     @Test
     public void testAnnotateMethod_annotated() throws Exception {
         Method m = CommandLine.getCommandMethods(MethodApp.class, "run2").get(0);
diff --git a/src/test/java/picocli/CommandLineMixinTest.java b/src/test/java/picocli/CommandLineMixinTest.java
@@ -92,6 +92,20 @@ class ValidMixin { // valid command because it has @Parameters annotation
         commandLine.addMixin("valid", new ValidMixin()); // no exception
     }
 
+    @Test
+    public void testAddMixinMustBeValidCommand_SubCommandMethod() {
+        @Command class ValidMixin {  // valid command because it has @Command annotation
+        }
+        @Command class Receiver {
+            @Command void sub(@Mixin ValidMixin mixin) {
+            }
+        }
+        CommandLine commandLine = new CommandLine(new Receiver(), new InnerClassFactory(this));
+        CommandSpec commandSpec = commandLine.getCommandSpec().subcommands().get("sub").getCommandSpec().mixins().get("arg0");
+        assertEquals(ValidMixin.class, commandSpec.userObject().getClass());
+        commandLine.addMixin("valid", new ValidMixin()); // no exception
+    }
+
     @Test
     public void testMixinAnnotationRejectedIfNotAValidCommand() {
         class Invalid {}
