Never prompt for password if it was passed as an argument (fixes #195)

src/main/java/com/beust/jcommander/JCommander.java

@@ -18,20 +18,42 @@
 
 package com.beust.jcommander;
 
-import com.beust.jcommander.FuzzyMap.IKey;
-import com.beust.jcommander.converters.*;
-import com.beust.jcommander.internal.*;
-
 import java.io.BufferedReader;
 import java.io.IOException;
-import java.lang.reflect.*;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
 import java.nio.charset.Charset;
 import java.nio.file.Files;
 import java.nio.file.Paths;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.EnumSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
 import java.util.ResourceBundle;
 import java.util.concurrent.CopyOnWriteArrayList;
 
+import com.beust.jcommander.FuzzyMap.IKey;
+import com.beust.jcommander.converters.DefaultListConverter;
+import com.beust.jcommander.converters.EnumConverter;
+import com.beust.jcommander.converters.IParameterSplitter;
+import com.beust.jcommander.converters.NoConverter;
+import com.beust.jcommander.converters.StringConverter;
+import com.beust.jcommander.internal.Console;
+import com.beust.jcommander.internal.DefaultConsole;
+import com.beust.jcommander.internal.DefaultConverterFactory;
+import com.beust.jcommander.internal.JDK6Console;
+import com.beust.jcommander.internal.Lists;
+import com.beust.jcommander.internal.Maps;
+import com.beust.jcommander.internal.Nullable;
+
 /**
  * The main class for JCommander. It's responsible for parsing the object that contains
  * all the annotated fields, parse the command line and assign the fields with the correct
@@ -642,12 +664,7 @@ private void parseValues(String[] args, boolean validate) {
 
                 if (pd != null) {
                     if (pd.getParameter().password()) {
-                        //
-                        // Password option, use the Console to retrieve the password
-                        //
-                        char[] password = readPassword(pd.getDescription(), pd.getParameter().echoInput());
-                        pd.addValue(new String(password));
-                        requiredFields.remove(pd.getParameterized());
+                        increment = processPassword(args, i, pd, validate);
                     } else {
                         if (pd.getParameter().variableArity()) {
                             //
@@ -760,6 +777,34 @@ public int processVariableArity(String optionName, String[] options) {
 
     private final IVariableArity DEFAULT_VARIABLE_ARITY = new DefaultVariableArity();
 
+    private final int determineArity(String[] args, int index, ParameterDescription pd, IVariableArity va) {
+        List<String> currentArgs = Lists.newArrayList();
+        for (int j = index + 1; j < args.length; j++) {
+            currentArgs.add(args[j]);
+        }
+        return va.processVariableArity(pd.getParameter().names()[0],
+                currentArgs.toArray(new String[0]));
+    }
+
+    /**
+     * @return the number of options that were processed.
+     */
+    private int processPassword(String[] args, int index, ParameterDescription pd, boolean validate) {
+        final int passwordArity = determineArity(args, index, pd, DEFAULT_VARIABLE_ARITY);
+        if (passwordArity == 0) {
+            // password option with password not specified, use the Console to retrieve the password
+            char[] password = readPassword(pd.getDescription(), pd.getParameter().echoInput());
+            pd.addValue(new String(password));
+            requiredFields.remove(pd.getParameterized());
+            return 1;
+        } else if (passwordArity == 1) {
+            // password option with password specified
+            return processFixedArity(args, index, pd, validate, List.class, 1);
+        } else {
+            throw new ParameterException("Password parameter must have at most 1 argument.");
+        }
+    }
+
     /**
      * @return the number of options that were processed.
      */
@@ -772,13 +817,7 @@ private int processVariableArity(String[] args, int index, ParameterDescription
             va = (IVariableArity) arg;
         }
 
-        List<String> currentArgs = Lists.newArrayList();
-        for (int j = index + 1; j < args.length; j++) {
-            currentArgs.add(args[j]);
-        }
-        int arity = va.processVariableArity(pd.getParameter().names()[0],
-                currentArgs.toArray(new String[0]));
-
+        int arity = determineArity(args, index, pd, va);
         int result = processFixedArity(args, index, pd, validate, List.class, arity);
         return result;
     }

src/test/java/com/beust/jcommander/JCommanderTest.java

@@ -18,8 +18,64 @@
 
 package com.beust.jcommander;
 
-import com.beust.jcommander.args.*;
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStreamWriter;
+import java.math.BigDecimal;
+import java.nio.charset.Charset;
+import java.text.ParseException;
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.EnumSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.ResourceBundle;
+import java.util.TreeSet;
+
+import com.beust.jcommander.args.AlternateNamesForListArgs;
+import com.beust.jcommander.args.Args1;
+import com.beust.jcommander.args.Args1Setter;
+import com.beust.jcommander.args.Args2;
+import com.beust.jcommander.args.ArgsArityString;
+import com.beust.jcommander.args.ArgsBooleanArity;
+import com.beust.jcommander.args.ArgsBooleanArity0;
+import com.beust.jcommander.args.ArgsConverter;
+import com.beust.jcommander.args.ArgsEnum;
 import com.beust.jcommander.args.ArgsEnum.ChoiceType;
+import com.beust.jcommander.args.ArgsEquals;
+import com.beust.jcommander.args.ArgsHelp;
+import com.beust.jcommander.args.ArgsI18N1;
+import com.beust.jcommander.args.ArgsI18N2;
+import com.beust.jcommander.args.ArgsI18N2New;
+import com.beust.jcommander.args.ArgsInherited;
+import com.beust.jcommander.args.ArgsList;
+import com.beust.jcommander.args.ArgsLongCommandDescription;
+import com.beust.jcommander.args.ArgsLongDescription;
+import com.beust.jcommander.args.ArgsLongMainParameterDescription;
+import com.beust.jcommander.args.ArgsMainParameter1;
+import com.beust.jcommander.args.ArgsMaster;
+import com.beust.jcommander.args.ArgsMultipleUnparsed;
+import com.beust.jcommander.args.ArgsOutOfMemory;
+import com.beust.jcommander.args.ArgsPrivate;
+import com.beust.jcommander.args.ArgsRequired;
+import com.beust.jcommander.args.ArgsSlave;
+import com.beust.jcommander.args.ArgsSlaveBogus;
+import com.beust.jcommander.args.ArgsValidate1;
+import com.beust.jcommander.args.ArgsWithSet;
+import com.beust.jcommander.args.Arity1;
+import com.beust.jcommander.args.HiddenArgs;
+import com.beust.jcommander.args.SeparatorColon;
+import com.beust.jcommander.args.SeparatorEqual;
+import com.beust.jcommander.args.SeparatorMixed;
+import com.beust.jcommander.args.SlashSeparator;
+import com.beust.jcommander.args.VariableArity;
 import com.beust.jcommander.command.CommandAdd;
 import com.beust.jcommander.command.CommandCommit;
 import com.beust.jcommander.command.CommandMain;
@@ -29,14 +85,6 @@
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
-import java.io.*;
-import java.math.BigDecimal;
-import java.nio.charset.Charset;
-import java.text.ParseException;
-import java.text.SimpleDateFormat;
-import java.util.*;
-import java.util.ResourceBundle;
-
 @Test
 public class JCommanderTest {
 
@@ -818,32 +866,6 @@ public void commandsWithSamePrefixAsOptionsShouldWork() {
         jc.parse("--configure");
     }
 
-    // Tests:
-    // required unparsed parameter
-    @Test(enabled = false,
-            description = "For some reason, this test still asks the password on stdin")
-    public void askedRequiredPassword() {
-        class A {
-            @Parameter(names = {"--password", "-p"}, description = "Private key password",
-                    password = true, required = true)
-            public String password;
-
-            @Parameter(names = {"--port", "-o"}, description = "Port to bind server to",
-                    required = true)
-            public int port;
-        }
-        A a = new A();
-        InputStream stdin = System.in;
-        try {
-            System.setIn(new ByteArrayInputStream("password".getBytes()));
-            new JCommander(a, "--port", "7", "--password");
-            Assert.assertEquals(a.port, 7);
-            Assert.assertEquals(a.password, "password");
-        } finally {
-            System.setIn(stdin);
-        }
-    }
-
     public void dynamicParameters() {
         class Command {
             @DynamicParameter(names = {"-P"}, description = "Additional command parameters")

src/test/java/com/beust/jcommander/PasswordTest.java

@@ -0,0 +1,111 @@
+package com.beust.jcommander;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+
+import org.testng.Assert;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+public class PasswordTest {
+
+    @DataProvider(name = "args")
+    public Object[][] createArgs() {
+        return new Object[][] {
+                { new PasswordTestingArgs() },
+                { new OptionalPasswordTestingArgs() },
+        };
+    }
+
+    public interface Args {
+
+        String getPassword();
+
+        int getPort();
+
+    }
+
+    public static class PasswordTestingArgs implements PasswordTest.Args {
+        @Parameter(names = {"--password", "-p"}, description = "Private key password",
+                password = true, required = true)
+        public String password;
+
+        @Parameter(names = {"--port", "-o"}, description = "Port to bind server to",
+                required = true)
+        public int port;
+
+        @Override
+        public String getPassword() {
+            return password;
+        }
+
+        @Override
+        public int getPort() {
+            return port;
+        }
+    }
+
+    @Test(dataProvider = "args")
+    public void passwordNotAsked(Args a) {
+        String expectedPassword = "somepassword";
+        int expectedPort = 7;
+        new JCommander(a, "--password", expectedPassword, "--port", String.valueOf(7));
+        Assert.assertEquals(a.getPort(), expectedPort);
+        Assert.assertEquals(a.getPassword(), expectedPassword);
+    }
+
+    @Test(dataProvider = "args", expectedExceptions = ParameterException.class)
+    public void passwordWithExcessiveArity(Args a) {
+        new JCommander(a, "--password", "somepassword", "someotherarg", "--port", String.valueOf(7));
+    }
+
+    @Test(dataProvider = "args")
+    public void passwordAsked(Args a) {
+        InputStream stdin = System.in;
+        String password = "password";
+        int port = 7;
+        try {
+            System.setIn(new ByteArrayInputStream(password.getBytes()));
+            new JCommander(a, "--port", String.valueOf(port), "--password");
+            Assert.assertEquals(a.getPort(), port);
+            Assert.assertEquals(a.getPassword(), password);
+        } finally {
+            System.setIn(stdin);
+        }
+    }
+
+    public static class OptionalPasswordTestingArgs implements PasswordTest.Args {
+        @Parameter(names = {"--password", "-p"}, description = "Private key password",
+                password = true)
+        public String password;
+
+        @Parameter(names = {"--port", "-o"}, description = "Port to bind server to",
+                required = true)
+        public int port;
+
+        @Override
+        public String getPassword() {
+            return password;
+        }
+
+        @Override
+        public int getPort() {
+            return port;
+        }
+    }
+
+    @Test
+    public void passwordOptionalNotProvided() {
+        Args a = new OptionalPasswordTestingArgs();
+        new JCommander(a, "--port", "7");
+        Assert.assertEquals(a.getPort(), 7);
+        Assert.assertEquals(a.getPassword(), null);
+    }
+
+    @Test(expectedExceptions = ParameterException.class)
+    public void passwordRequredNotProvided() {
+        Args a = new PasswordTestingArgs();
+        new JCommander(a, "--port", "7");
+    }
+
+}