#416 bugfix: positional parameters in @command methods should get the…

…ir starting index from their method parameter position, but their max index from the type
remkop · Sep 6, 2018 · b0c2aee · b0c2aee
1 parent 54f7cef
commit b0c2aee
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 10 deletions.
diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java
@@ -2457,7 +2457,7 @@ private static class NoCompletionCandidates implements Iterable<String> {
          * a subset of the command line arguments to this field. The default is "*", meaning all command line arguments.
          * @return an index or range specifying which of the command line arguments should be assigned to this field
          */
-        String index() default "*";
+        String index() default "";
 
         /** Description of the parameter(s), used when generating the usage documentation.
          * <p>
@@ -3154,12 +3154,15 @@ private static Range parameterArity(TypedMember member) {
         public static Range parameterIndex(Field field) { return parameterIndex(new TypedMember(field)); }
         private static Range parameterIndex(TypedMember member) {
             if (member.isAnnotationPresent(Parameters.class)) {
-                return Range.valueOf(member.getAnnotation(Parameters.class).index());
-            } else {
-                return member.isMethodParameter()
-                        ? Range.valueOf("" + ((MethodParam) member.accessible).position)
-                        : new Range(0, 0, false, true, "0");
+                Range result = Range.valueOf(member.getAnnotation(Parameters.class).index());
+                if (!result.isUnspecified) { return result; }
+            }
+            if (member.isMethodParameter()) {
+                int min = ((MethodParam) member.accessible).position;
+                int max = member.isMultiValue() ? Integer.MAX_VALUE : min;
+                return new Range(min, max, member.isMultiValue(), false, "");
             }
+            return Range.valueOf("*"); // the default
         }
         static Range adjustForType(Range result, TypedMember member) {
             return result.isUnspecified ? defaultArity(member) : result;
@@ -5199,7 +5202,7 @@ void addAll(String[] unmatched) {
             }
         }
         /** mock java.lang.reflect.Parameter (not available before Java 8) */
-        private static class MethodParam extends AccessibleObject {
+        static class MethodParam extends AccessibleObject {
             final Method method;
             final int paramIndex;
             final String name;

diff --git a/src/test/java/picocli/CommandLineCommandMethodTest.java b/src/test/java/picocli/CommandLineCommandMethodTest.java
@@ -129,9 +129,11 @@ public void testAnnotateMethod_unannotatedPositional_indexByParameterOrder() thr
         String[] labels = { "<arg0>", "<arg1>", "<arg2>", "<arg3>", "<arg4>"};
         assertEquals(positionals.size(), labels.length);
 
+        String[] ranges = { "0", "1", "2", "3..*", "4..*" };
+
         for (int i = 0; i < positionals.size(); i++) {
             Model.PositionalParamSpec positional = positionals.get(i);
-            assertEquals(positional.paramLabel() + " at index " + i, CommandLine.Range.valueOf(i + ""), positional.index());
+            assertEquals(positional.paramLabel() + " at index " + i, CommandLine.Range.valueOf(ranges[i]), positional.index());
             assertEquals(labels[i], positional.paramLabel());
         }
     }
@@ -151,9 +153,11 @@ public void testAnnotateMethod_unannotatedPositionalMixedWithOptions_indexByPara
         String[] labels = { "<arg0>", "<arg3>", "<arg4>"};
         assertEquals(positionals.size(), labels.length);
 
+        String[] ranges = { "0", "1..*", "2..*" };
+
         for (int i = 0; i < positionals.size(); i++) {
             Model.PositionalParamSpec positional = positionals.get(i);
-            assertEquals(positional.paramLabel() + " at index " + i, CommandLine.Range.valueOf(i + ""), positional.index());
+            assertEquals(positional.paramLabel() + " at index " + i, CommandLine.Range.valueOf(ranges[i]), positional.index());
             assertEquals(labels[i], positional.paramLabel());
         }
 
@@ -451,7 +455,7 @@ void push(@Option(names = {"-f", "--force"}) boolean force,
         }
     }
     @Test
-    public void testGitsageHelpMessage() {
+    public void testGitUsageHelpMessage() {
         CommandLine cmd = new CommandLine(new Git());
         String expected = String.format("" +
                 "Usage: git [-hV] [--git-dir=<path>] [COMMAND]%n" +
@@ -466,6 +470,35 @@ public void testGitsageHelpMessage() {
         assertEquals(expected, cmd.getUsageMessage());
     }
 
+    @Test
+    public void testParamIndex() {
+        CommandLine git = new CommandLine(new Git());
+        CommandLine clone = git.getSubcommands().get("clone");
+        Model.PositionalParamSpec repo = clone.getCommandSpec().positionalParameters().get(0);
+        assertEquals(CommandLine.Range.valueOf("0"), repo.index());
+    }
+
+    @Command
+    static class AnnotatedParams {
+        @Command
+        public void method(@Parameters int a,
+                           @Parameters int b,
+                           @Parameters int c,
+                           int x,
+                           int y,
+                           int z) {}
+    }
+
+    @Test
+    public void testParamIndexAnnotatedAndUnAnnotated() {
+        CommandLine git = new CommandLine(new AnnotatedParams());
+        CommandLine method = git.getSubcommands().get("method");
+        List<Model.PositionalParamSpec> positionals = method.getCommandSpec().positionalParameters();
+        for (int i = 0; i < positionals.size(); i++) {
+            assertEquals(CommandLine.Range.valueOf("" + i), positionals.get(i).index());
+        }
+    }
+
     private static Set<String> set(String... elements) {
         return new HashSet<String>(Arrays.asList(elements));
     }