From e1f3164cba66b9ec83dfca320ea549d0f666fae5 Mon Sep 17 00:00:00 2001 From: laeubi Date: Wed, 15 Mar 2017 15:44:35 +0100 Subject: [PATCH] Fix for CLI-271 Add some Tests for coverage --- .../org/apache/commons/cli/CommandLine.java | 169 +++++++++++++++--- .../apache/commons/cli/CommandLineTest.java | 67 +++++++ .../org/apache/commons/cli/OptionTest.java | 8 + .../org/apache/commons/cli/ValueTest.java | 136 ++++++++++++++ .../apache/commons/cli/bug/BugCLI133Test.java | 2 +- 5 files changed, 358 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index 154738a25..9214111c8 100644 --- a/src/main/java/org/apache/commons/cli/CommandLine.java +++ b/src/main/java/org/apache/commons/cli/CommandLine.java @@ -55,6 +55,18 @@ protected CommandLine() { // nothing to do } + + /** + * Query to see if an option has been set. + * + * @param opt the option to check + * @return true if set, false if not + * @since 1.4 + */ + public boolean hasOption(Option opt) + { + return options.contains(opt); + } /** * Query to see if an option has been set. @@ -64,9 +76,9 @@ protected CommandLine() */ public boolean hasOption(String opt) { - return options.contains(resolveOption(opt)); + return hasOption(resolveOption(opt)); } - + /** * Query to see if an option has been set. * @@ -98,32 +110,62 @@ public Object getOptionObject(String opt) return null; } } - + /** * Return a version of this Option converted to a particular type. * - * @param opt the name of the option + * @param option the name of the option * @return the value parsed into a particular object * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder - * @since 1.2 + * @since 1.4 */ - public Object getParsedOptionValue(String opt) throws ParseException + public Object getParsedOptionValue(Option option) throws ParseException { - String res = getOptionValue(opt); - Option option = resolveOption(opt); - - if (option == null || res == null) + if (option == null) + { + return null; + } + String res = getOptionValue(option); + if (res == null) { return null; } - return TypeHandler.createValue(res, option.getType()); } + /** + * Return a version of this Option converted to a particular type. + * + * @param opt the name of the option + * @return the value parsed into a particular object + * @throws ParseException if there are problems turning the option value into the desired type + * @see PatternOptionBuilder + * @since 1.2 + */ + public Object getParsedOptionValue(String opt) throws ParseException + { + return getParsedOptionValue(resolveOption(opt)); + } + + /** + * Return a version of this Option converted to a particular type. + * + * @param opt the name of the option + * @return the value parsed into a particular object + * @throws ParseException if there are problems turning the option value into the desired type + * @see PatternOptionBuilder + * @since 1.2 + */ + public Object getParsedOptionValue(char opt) throws ParseException + { + return getParsedOptionValue(String.valueOf(opt)); + } + /** * Return the Object type of this Option. * + * @deprecated due to System.err message. Instead use getParsedOptionValue(char) * @param opt the name of the option * @return the type of opt */ @@ -131,6 +173,24 @@ public Object getOptionObject(char opt) { return getOptionObject(String.valueOf(opt)); } + + /** + * Retrieve the first argument, if any, of this option. + * + * @param option the name of the option + * @return Value of the argument if option is set, and has an argument, + * otherwise null. + * @since 1.4 + */ + public String getOptionValue(Option option) + { + if (option == null) + { + return null; + } + String[] values = getOptionValues(option); + return (values == null) ? null : values[0]; + } /** * Retrieve the first argument, if any, of this option. @@ -141,9 +201,7 @@ public Object getOptionObject(char opt) */ public String getOptionValue(String opt) { - String[] values = getOptionValues(opt); - - return (values == null) ? null : values[0]; + return getOptionValue(resolveOption(opt)); } /** @@ -157,29 +215,42 @@ public String getOptionValue(char opt) { return getOptionValue(String.valueOf(opt)); } - + /** * Retrieves the array of values, if any, of an option. * - * @param opt string name of the option + * @param option string name of the option * @return Values of the argument if option is set, and has an argument, * otherwise null. + * @since 1.4 */ - public String[] getOptionValues(String opt) + public String[] getOptionValues(Option option) { List values = new ArrayList(); - for (Option option : options) + for (Option processedOption : options) { - if (opt.equals(option.getOpt()) || opt.equals(option.getLongOpt())) + if (processedOption.equals(option)) { - values.addAll(option.getValuesList()); + values.addAll(processedOption.getValuesList()); } } return values.isEmpty() ? null : values.toArray(new String[values.size()]); } + /** + * Retrieves the array of values, if any, of an option. + * + * @param opt string name of the option + * @return Values of the argument if option is set, and has an argument, + * otherwise null. + */ + public String[] getOptionValues(String opt) + { + return getOptionValues(resolveOption(opt)); + } + /** * Retrieves the option object given the long or short option as a String * @@ -216,6 +287,22 @@ public String[] getOptionValues(char opt) { return getOptionValues(String.valueOf(opt)); } + + /** + * Retrieve the first argument, if any, of an option. + * + * @param option name of the option + * @param defaultValue is the default value to be returned if the option + * is not specified + * @return Value of the argument if option is set, and has an argument, + * otherwise defaultValue. + * @since 1.4 + */ + public String getOptionValue(Option option, String defaultValue) + { + String answer = getOptionValue(option); + return (answer != null) ? answer : defaultValue; + } /** * Retrieve the first argument, if any, of an option. @@ -228,9 +315,7 @@ public String[] getOptionValues(char opt) */ public String getOptionValue(String opt, String defaultValue) { - String answer = getOptionValue(opt); - - return (answer != null) ? answer : defaultValue; + return getOptionValue(resolveOption(opt), defaultValue); } /** @@ -246,6 +331,44 @@ public String getOptionValue(char opt, String defaultValue) { return getOptionValue(String.valueOf(opt), defaultValue); } + + /** + * Retrieve the map of values associated to the option. This is convenient + * for options specifying Java properties like -Dparam1=value1 + * -Dparam2=value2. The first argument of the option is the key, and + * the 2nd argument is the value. If the option has only one argument + * (-Dfoo) it is considered as a boolean flag and the value is + * "true". + * + * @param option name of the option + * @return The Properties mapped by the option, never null + * even if the option doesn't exists + * @since 1.4 + */ + public Properties getOptionProperties(Option option) + { + Properties props = new Properties(); + + for (Option processedOption : options) + { + if (processedOption.equals(option)) + { + List values = processedOption.getValuesList(); + if (values.size() >= 2) + { + // use the first 2 arguments as the key/value pair + props.put(values.get(0), values.get(1)); + } + else if (values.size() == 1) + { + // no explicit value, handle it as a boolean + props.put(values.get(0), "true"); + } + } + } + + return props; + } /** * Retrieve the map of values associated to the option. This is convenient diff --git a/src/test/java/org/apache/commons/cli/CommandLineTest.java b/src/test/java/org/apache/commons/cli/CommandLineTest.java index a25fb028e..8ef076891 100644 --- a/src/test/java/org/apache/commons/cli/CommandLineTest.java +++ b/src/test/java/org/apache/commons/cli/CommandLineTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import java.util.Properties; @@ -49,6 +50,31 @@ public void testGetOptionProperties() throws Exception assertEquals("property with long format", "bar", cl.getOptionProperties("property").getProperty("foo")); } + + @Test + public void testGetOptionPropertiesWithOption() throws Exception + { + String[] args = new String[] { "-Dparam1=value1", "-Dparam2=value2", "-Dparam3", "-Dparam4=value4", "-D", "--property", "foo=bar" }; + + Options options = new Options(); + Option option_D = OptionBuilder.withValueSeparator().hasOptionalArgs(2).create('D'); + Option option_property = OptionBuilder.withValueSeparator().hasArgs(2).withLongOpt("property").create(); + options.addOption(option_D); + options.addOption(option_property); + + Parser parser = new GnuParser(); + CommandLine cl = parser.parse(options, args); + + Properties props = cl.getOptionProperties(option_D); + assertNotNull("null properties", props); + assertEquals("number of properties in " + props, 4, props.size()); + assertEquals("property 1", "value1", props.getProperty("param1")); + assertEquals("property 2", "value2", props.getProperty("param2")); + assertEquals("property 3", "true", props.getProperty("param3")); + assertEquals("property 4", "value4", props.getProperty("param4")); + + assertEquals("property with long format", "bar", cl.getOptionProperties(option_property).getProperty("foo")); + } @Test public void testGetOptions() @@ -76,6 +102,47 @@ public void testGetParsedOptionValue() throws Exception { assertEquals(123, ((Number) cmd.getParsedOptionValue("i")).intValue()); assertEquals("foo", cmd.getParsedOptionValue("f")); } + + @Test + public void testGetParsedOptionValueWithChar() throws Exception { + Options options = new Options(); + options.addOption(Option.builder("i").hasArg().type(Number.class).build()); + options.addOption(Option.builder("f").hasArg().build()); + + CommandLineParser parser = new DefaultParser(); + CommandLine cmd = parser.parse(options, new String[] { "-i", "123", "-f", "foo" }); + + assertEquals(123, ((Number) cmd.getParsedOptionValue('i')).intValue()); + assertEquals("foo", cmd.getParsedOptionValue('f')); + } + + @Test + public void testGetParsedOptionValueWithOption() throws Exception { + Options options = new Options(); + Option opt_i = Option.builder("i").hasArg().type(Number.class).build(); + Option opt_f = Option.builder("f").hasArg().build(); + options.addOption(opt_i); + options.addOption(opt_f); + + CommandLineParser parser = new DefaultParser(); + CommandLine cmd = parser.parse(options, new String[] { "-i", "123", "-f", "foo" }); + + assertEquals(123, ((Number) cmd.getParsedOptionValue(opt_i)).intValue()); + assertEquals("foo", cmd.getParsedOptionValue(opt_f)); + } + + @Test + public void testNullhOption() throws Exception { + Options options = new Options(); + Option opt_i = Option.builder("i").hasArg().type(Number.class).build(); + Option opt_f = Option.builder("f").hasArg().build(); + options.addOption(opt_i); + options.addOption(opt_f); + CommandLineParser parser = new DefaultParser(); + CommandLine cmd = parser.parse(options, new String[] { "-i", "123", "-f", "foo" }); + assertNull(cmd.getOptionValue((Option)null)); + assertNull(cmd.getParsedOptionValue((Option)null)); + } @Test public void testBuilder() diff --git a/src/test/java/org/apache/commons/cli/OptionTest.java b/src/test/java/org/apache/commons/cli/OptionTest.java index bbb0c309b..e5a85999d 100644 --- a/src/test/java/org/apache/commons/cli/OptionTest.java +++ b/src/test/java/org/apache/commons/cli/OptionTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertTrue; @@ -71,6 +72,13 @@ public void testClone() assertEquals(0, a.getValuesList().size()); assertEquals(2, b.getValues().length); } + + @Test + public void testHashCode() { + assertNotEquals(Option.builder("test").build().hashCode(), Option.builder("test2").build().hashCode()) ; + assertNotEquals(Option.builder("test").build().hashCode(), Option.builder().longOpt("test").build().hashCode()) ; + assertNotEquals(Option.builder("test").build().hashCode(), Option.builder("test").longOpt("long test").build().hashCode()) ; + } private static class DefaultOption extends Option { diff --git a/src/test/java/org/apache/commons/cli/ValueTest.java b/src/test/java/org/apache/commons/cli/ValueTest.java index 14a841075..d7540e3ef 100644 --- a/src/test/java/org/apache/commons/cli/ValueTest.java +++ b/src/test/java/org/apache/commons/cli/ValueTest.java @@ -62,6 +62,13 @@ public void testShortNoArg() assertTrue( _cl.hasOption("a") ); assertNull( _cl.getOptionValue("a") ); } + + @Test + public void testShortNoArgWithOption() + { + assertTrue( _cl.hasOption(opts.getOption("a")) ); + assertNull( _cl.getOptionValue(opts.getOption("a")) ); + } @Test public void testShortWithArg() @@ -70,6 +77,14 @@ public void testShortWithArg() assertNotNull( _cl.getOptionValue("b") ); assertEquals( _cl.getOptionValue("b"), "foo"); } + + @Test + public void testShortWithArgWithOption() + { + assertTrue( _cl.hasOption(opts.getOption("b")) ); + assertNotNull( _cl.getOptionValue(opts.getOption("b")) ); + assertEquals( _cl.getOptionValue(opts.getOption("b")), "foo"); + } @Test public void testLongNoArg() @@ -77,6 +92,13 @@ public void testLongNoArg() assertTrue( _cl.hasOption("c") ); assertNull( _cl.getOptionValue("c") ); } + + @Test + public void testLongNoArgWithOption() + { + assertTrue( _cl.hasOption(opts.getOption("c")) ); + assertNull( _cl.getOptionValue(opts.getOption("c")) ); + } @Test public void testLongWithArg() @@ -85,6 +107,14 @@ public void testLongWithArg() assertNotNull( _cl.getOptionValue("d") ); assertEquals( _cl.getOptionValue("d"), "bar"); } + + @Test + public void testLongWithArgWithOption() + { + assertTrue( _cl.hasOption(opts.getOption("d")) ); + assertNotNull( _cl.getOptionValue(opts.getOption("d")) ); + assertEquals( _cl.getOptionValue(opts.getOption("d")), "bar"); + } @Test public void testShortOptionalArgNoValue() throws Exception @@ -96,6 +126,17 @@ public void testShortOptionalArgNoValue() throws Exception assertTrue( cmd.hasOption("e") ); assertNull( cmd.getOptionValue("e") ); } + + @Test + public void testShortOptionalArgNoValueWithOption() throws Exception + { + String[] args = new String[] { "-e" }; + + Parser parser = new PosixParser(); + CommandLine cmd = parser.parse(opts,args); + assertTrue( cmd.hasOption(opts.getOption("e")) ); + assertNull( cmd.getOptionValue(opts.getOption("e")) ); + } @Test public void testShortOptionalArgValue() throws Exception @@ -107,6 +148,17 @@ public void testShortOptionalArgValue() throws Exception assertTrue( cmd.hasOption("e") ); assertEquals( "everything", cmd.getOptionValue("e") ); } + + @Test + public void testShortOptionalArgValueWithOption() throws Exception + { + String[] args = new String[] { "-e", "everything" }; + + Parser parser = new PosixParser(); + CommandLine cmd = parser.parse(opts,args); + assertTrue( cmd.hasOption(opts.getOption("e")) ); + assertEquals( "everything", cmd.getOptionValue(opts.getOption("e")) ); + } @Test public void testLongOptionalNoValue() throws Exception @@ -118,6 +170,17 @@ public void testLongOptionalNoValue() throws Exception assertTrue( cmd.hasOption("fish") ); assertNull( cmd.getOptionValue("fish") ); } + + @Test + public void testLongOptionalNoValueWithOption() throws Exception + { + String[] args = new String[] { "--fish" }; + + Parser parser = new PosixParser(); + CommandLine cmd = parser.parse(opts,args); + assertTrue( cmd.hasOption(opts.getOption("fish")) ); + assertNull( cmd.getOptionValue(opts.getOption("fish")) ); + } @Test public void testLongOptionalArgValue() throws Exception @@ -129,6 +192,17 @@ public void testLongOptionalArgValue() throws Exception assertTrue( cmd.hasOption("fish") ); assertEquals( "face", cmd.getOptionValue("fish") ); } + + @Test + public void testLongOptionalArgValueWithOption() throws Exception + { + String[] args = new String[] { "--fish", "face" }; + + Parser parser = new PosixParser(); + CommandLine cmd = parser.parse(opts,args); + assertTrue( cmd.hasOption(opts.getOption("fish")) ); + assertEquals( "face", cmd.getOptionValue(opts.getOption("fish")) ); + } @Test public void testShortOptionalArgValues() throws Exception @@ -143,6 +217,20 @@ public void testShortOptionalArgValues() throws Exception assertEquals( "idea", cmd.getOptionValues("j")[1] ); assertEquals( cmd.getArgs().length, 0 ); } + + @Test + public void testShortOptionalArgValuesWithOption() throws Exception + { + String[] args = new String[] { "-j", "ink", "idea" }; + + Parser parser = new PosixParser(); + CommandLine cmd = parser.parse(opts,args); + assertTrue( cmd.hasOption(opts.getOption("j")) ); + assertEquals( "ink", cmd.getOptionValue(opts.getOption("j")) ); + assertEquals( "ink", cmd.getOptionValues(opts.getOption("j"))[0] ); + assertEquals( "idea", cmd.getOptionValues(opts.getOption("j"))[1] ); + assertEquals( cmd.getArgs().length, 0 ); + } @Test public void testLongOptionalArgValues() throws Exception @@ -157,6 +245,20 @@ public void testLongOptionalArgValues() throws Exception assertEquals( "garden", cmd.getOptionValues("gravy")[1] ); assertEquals( cmd.getArgs().length, 0 ); } + + @Test + public void testLongOptionalArgValuesWithOption() throws Exception + { + String[] args = new String[] { "--gravy", "gold", "garden" }; + + Parser parser = new PosixParser(); + CommandLine cmd = parser.parse(opts,args); + assertTrue( cmd.hasOption(opts.getOption("gravy")) ); + assertEquals( "gold", cmd.getOptionValue(opts.getOption("gravy")) ); + assertEquals( "gold", cmd.getOptionValues(opts.getOption("gravy"))[0] ); + assertEquals( "garden", cmd.getOptionValues(opts.getOption("gravy"))[1] ); + assertEquals( cmd.getArgs().length, 0 ); + } @Test public void testShortOptionalNArgValues() throws Exception @@ -173,6 +275,22 @@ public void testShortOptionalNArgValues() throws Exception assertEquals( "isotope", cmd.getArgs()[0] ); assertEquals( "ice", cmd.getArgs()[1] ); } + + @Test + public void testShortOptionalNArgValuesWithOption() throws Exception + { + String[] args = new String[] { "-i", "ink", "idea", "isotope", "ice" }; + + Parser parser = new PosixParser(); + CommandLine cmd = parser.parse(opts,args); + assertTrue( cmd.hasOption("i") ); + assertEquals( "ink", cmd.getOptionValue(opts.getOption("i")) ); + assertEquals( "ink", cmd.getOptionValues(opts.getOption("i"))[0] ); + assertEquals( "idea", cmd.getOptionValues(opts.getOption("i"))[1] ); + assertEquals( cmd.getArgs().length, 2 ); + assertEquals( "isotope", cmd.getArgs()[0] ); + assertEquals( "ice", cmd.getArgs()[1] ); + } @Test public void testLongOptionalNArgValues() throws Exception @@ -191,4 +309,22 @@ public void testLongOptionalNArgValues() throws Exception assertEquals( cmd.getArgs().length, 1 ); assertEquals( "head", cmd.getArgs()[0] ); } + + @Test + public void testLongOptionalNArgValuesWithOption() throws Exception + { + String[] args = new String[] { + "--hide", "house", "hair", "head" + }; + + Parser parser = new PosixParser(); + + CommandLine cmd = parser.parse(opts,args); + assertTrue( cmd.hasOption(opts.getOption("hide")) ); + assertEquals( "house", cmd.getOptionValue(opts.getOption("hide")) ); + assertEquals( "house", cmd.getOptionValues(opts.getOption("hide"))[0] ); + assertEquals( "hair", cmd.getOptionValues(opts.getOption("hide"))[1] ); + assertEquals( cmd.getArgs().length, 1 ); + assertEquals( "head", cmd.getArgs()[0] ); + } } diff --git a/src/test/java/org/apache/commons/cli/bug/BugCLI133Test.java b/src/test/java/org/apache/commons/cli/bug/BugCLI133Test.java index 8f6496ac9..f821a97b4 100644 --- a/src/test/java/org/apache/commons/cli/bug/BugCLI133Test.java +++ b/src/test/java/org/apache/commons/cli/bug/BugCLI133Test.java @@ -36,6 +36,6 @@ public void testOrder() throws ParseException { opts.addOption(optionA); PosixParser posixParser = new PosixParser(); CommandLine line = posixParser.parse(opts, null); - assertFalse(line.hasOption(null)); + assertFalse(line.hasOption((String)null)); } }