From 32f148e9b197719b1991fd3745c08607d4289aaa Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Thu, 8 Feb 2024 12:18:58 +0100 Subject: [PATCH 1/5] Added Supplier defaults for getParsedOptionValue --- .../org/apache/commons/cli/CommandLine.java | 49 ++++++++++++++++++- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index 7f1bf4820..f5c3feae8 100644 --- a/src/main/java/org/apache/commons/cli/CommandLine.java +++ b/src/main/java/org/apache/commons/cli/CommandLine.java @@ -412,6 +412,21 @@ public T getParsedOptionValue(final char opt, final T defaultValue) throws P return getParsedOptionValue(String.valueOf(opt), defaultValue); } + /** + * Gets a version of this {@code Option} converted to a particular type. + * + * @param opt the name of the option. + * @param defaultValue the default value to return if opt is not set. + * @param The return type for the method. + * @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.7.0 + */ + public T getParsedOptionValue(final char opt, final Supplier defaultValue) throws ParseException { + return getParsedOptionValue(String.valueOf(opt), defaultValue); + } + /** * Gets a version of this {@code Option} converted to a particular type. * @@ -437,15 +452,30 @@ public T getParsedOptionValue(final Option option) throws ParseException { * @see PatternOptionBuilder * @since 1.7.0 */ - @SuppressWarnings("unchecked") public T getParsedOptionValue(final Option option, final T defaultValue) throws ParseException { + return getParsedOptionValue(option, () -> defaultValue); + } + + /** + * Gets a version of this {@code Option} converted to a particular type. + * + * @param option the name of the option. + * @param defaultValue the default value to return if opt is not set. + * @param The return type for the method. + * @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.7.0 + */ + @SuppressWarnings("unchecked") + public T getParsedOptionValue(final Option option, final Supplier defaultValue) throws ParseException { if (option == null) { return null; } final String res = getOptionValue(option); try { - return res == null ? defaultValue : (T) option.getConverter().apply(res); + return res == null ? defaultValue.get() : (T) option.getConverter().apply(res); } catch (final Throwable e) { throw ParseException.wrap(e); } @@ -480,6 +510,21 @@ public T getParsedOptionValue(final String opt, final T defaultValue) throws return getParsedOptionValue(resolveOption(opt), defaultValue); } + /** + * Gets a version of this {@code Option} converted to a particular type. + * + * @param opt the name of the option. + * @param defaultValue the default value to return if opt is not set. + * @param The return type for the method. + * @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.7.0 + */ + public T getParsedOptionValue(final String opt, final Supplier defaultValue) throws ParseException { + return getParsedOptionValue(resolveOption(opt), defaultValue); + } + /** * jkeyes - commented out until it is implemented properly *

From 7bd3f73d04ba708976c761af8af1ee82599a4259 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 9 Feb 2024 09:29:37 +0100 Subject: [PATCH 2/5] added tests --- .../org/apache/commons/cli/CommandLine.java | 5 +---- .../apache/commons/cli/CommandLineTest.java | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index f5c3feae8..a88604d52 100644 --- a/src/main/java/org/apache/commons/cli/CommandLine.java +++ b/src/main/java/org/apache/commons/cli/CommandLine.java @@ -469,10 +469,7 @@ public T getParsedOptionValue(final Option option, final T defaultValue) thr */ @SuppressWarnings("unchecked") public T getParsedOptionValue(final Option option, final Supplier defaultValue) throws ParseException { - if (option == null) { - return null; - } - final String res = getOptionValue(option); + final String res = option == null ? null : getOptionValue(option); try { return res == null ? defaultValue.get() : (T) option.getConverter().apply(res); diff --git a/src/test/java/org/apache/commons/cli/CommandLineTest.java b/src/test/java/org/apache/commons/cli/CommandLineTest.java index 4522fb030..e27ad4a4e 100644 --- a/src/test/java/org/apache/commons/cli/CommandLineTest.java +++ b/src/test/java/org/apache/commons/cli/CommandLineTest.java @@ -165,6 +165,26 @@ public void testGetParsedOptionValueWithOption() throws Exception { assertEquals("foo", cmd.getParsedOptionValue(optF)); } + @Test + public void testGetParsedOptionValueUsingDefault() throws Exception { + final Options options = new Options(); + final Option optI = Option.builder("i").hasArg().type(Number.class).build(); + final Option optF = Option.builder("f").hasArg().build(); + options.addOption(optI); + options.addOption(optF); + + final CommandLineParser parser = new DefaultParser(); + final CommandLine cmd = parser.parse(options, new String[] {"-i", "123"}); + + assertEquals(123, ((Number) cmd.getParsedOptionValue(optI)).intValue()); + assertEquals("foo", cmd.getParsedOptionValue(optF, "foo")); + assertEquals("foo", cmd.getParsedOptionValue(optF, ()->"foo")); + assertEquals("foo", cmd.getParsedOptionValue("f", "foo")); + assertEquals("foo", cmd.getParsedOptionValue("f", ()->"foo")); + assertEquals("foo", cmd.getParsedOptionValue('f', "foo")); + assertEquals("foo", cmd.getParsedOptionValue('f', ()->"foo")); + } + @Test public void testNullhOption() throws Exception { final Options options = new Options(); From 929edcd32959d8f47055c64c626fbdf6338e8b30 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Wed, 14 Feb 2024 09:46:06 +0100 Subject: [PATCH 3/5] Fixed test issues --- src/main/java/org/apache/commons/cli/CommandLine.java | 2 +- src/test/java/org/apache/commons/cli/CommandLineTest.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index a88604d52..139e0a143 100644 --- a/src/main/java/org/apache/commons/cli/CommandLine.java +++ b/src/main/java/org/apache/commons/cli/CommandLine.java @@ -438,7 +438,7 @@ public T getParsedOptionValue(final char opt, final Supplier defaultValue * @since 1.5.0 */ public T getParsedOptionValue(final Option option) throws ParseException { - return getParsedOptionValue(option, null); + return getParsedOptionValue(option, ()-> null); } /** diff --git a/src/test/java/org/apache/commons/cli/CommandLineTest.java b/src/test/java/org/apache/commons/cli/CommandLineTest.java index e27ad4a4e..7dc543127 100644 --- a/src/test/java/org/apache/commons/cli/CommandLineTest.java +++ b/src/test/java/org/apache/commons/cli/CommandLineTest.java @@ -178,15 +178,15 @@ public void testGetParsedOptionValueUsingDefault() throws Exception { assertEquals(123, ((Number) cmd.getParsedOptionValue(optI)).intValue()); assertEquals("foo", cmd.getParsedOptionValue(optF, "foo")); - assertEquals("foo", cmd.getParsedOptionValue(optF, ()->"foo")); + assertEquals("foo", cmd.getParsedOptionValue(optF, ()-> "foo")); assertEquals("foo", cmd.getParsedOptionValue("f", "foo")); - assertEquals("foo", cmd.getParsedOptionValue("f", ()->"foo")); + assertEquals("foo", cmd.getParsedOptionValue("f", ()-> "foo")); assertEquals("foo", cmd.getParsedOptionValue('f', "foo")); - assertEquals("foo", cmd.getParsedOptionValue('f', ()->"foo")); + assertEquals("foo", cmd.getParsedOptionValue('f', ()-> "foo")); } @Test - public void testNullhOption() throws Exception { + public void testNullOption() throws Exception { final Options options = new Options(); final Option optI = Option.builder("i").hasArg().type(Number.class).build(); final Option optF = Option.builder("f").hasArg().build(); From bbd14e6f9beebbeeaa5008fabcfbc91725ba60a7 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Thu, 15 Feb 2024 11:41:59 +0100 Subject: [PATCH 4/5] fixed style issues and added test for null supplier --- .../org/apache/commons/cli/CommandLine.java | 7 +++++-- .../apache/commons/cli/CommandLineTest.java | 19 ++++++++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index 139e0a143..b2f3a3d5c 100644 --- a/src/main/java/org/apache/commons/cli/CommandLine.java +++ b/src/main/java/org/apache/commons/cli/CommandLine.java @@ -438,7 +438,7 @@ public T getParsedOptionValue(final char opt, final Supplier defaultValue * @since 1.5.0 */ public T getParsedOptionValue(final Option option) throws ParseException { - return getParsedOptionValue(option, ()-> null); + return getParsedOptionValue(option, () -> null); } /** @@ -472,7 +472,10 @@ public T getParsedOptionValue(final Option option, final Supplier default final String res = option == null ? null : getOptionValue(option); try { - return res == null ? defaultValue.get() : (T) option.getConverter().apply(res); + if (res == null) { + return defaultValue == null ? null : defaultValue.get(); + } + return (T) option.getConverter().apply(res); } catch (final Throwable e) { throw ParseException.wrap(e); } diff --git a/src/test/java/org/apache/commons/cli/CommandLineTest.java b/src/test/java/org/apache/commons/cli/CommandLineTest.java index 7dc543127..88e4047b4 100644 --- a/src/test/java/org/apache/commons/cli/CommandLineTest.java +++ b/src/test/java/org/apache/commons/cli/CommandLineTest.java @@ -22,6 +22,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more import static org.junit.Assert.assertNull; import java.util.Properties; +import java.util.function.Supplier; import org.junit.Test; @@ -150,6 +151,18 @@ public void testGetParsedOptionValueWithChar() throws Exception { assertEquals("foo", cmd.getParsedOptionValue('f')); } + @Test + public void testGetParsedOptionValueWithNullSupplier() throws Exception { + final Options options = new Options(); + options.addOption(Option.builder("i").hasArg().type(Number.class).build()); + + final CommandLineParser parser = new DefaultParser(); + final CommandLine cmd = parser.parse(options, new String[0]); + final Supplier supplier = null; + + assertNull(cmd.getParsedOptionValue('i', supplier)); + } + @Test public void testGetParsedOptionValueWithOption() throws Exception { final Options options = new Options(); @@ -178,11 +191,11 @@ public void testGetParsedOptionValueUsingDefault() throws Exception { assertEquals(123, ((Number) cmd.getParsedOptionValue(optI)).intValue()); assertEquals("foo", cmd.getParsedOptionValue(optF, "foo")); - assertEquals("foo", cmd.getParsedOptionValue(optF, ()-> "foo")); + assertEquals("foo", cmd.getParsedOptionValue(optF, () -> "foo")); assertEquals("foo", cmd.getParsedOptionValue("f", "foo")); - assertEquals("foo", cmd.getParsedOptionValue("f", ()-> "foo")); + assertEquals("foo", cmd.getParsedOptionValue("f", () -> "foo")); assertEquals("foo", cmd.getParsedOptionValue('f', "foo")); - assertEquals("foo", cmd.getParsedOptionValue('f', ()-> "foo")); + assertEquals("foo", cmd.getParsedOptionValue('f', () -> "foo")); } @Test From c36c7f7f117d0fd20be3ebba98c08942b6d99886 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 16 Feb 2024 15:00:34 +0100 Subject: [PATCH 5/5] added additional supplier testing --- .../apache/commons/cli/CommandLineTest.java | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/test/java/org/apache/commons/cli/CommandLineTest.java b/src/test/java/org/apache/commons/cli/CommandLineTest.java index 88e4047b4..98c02ca8c 100644 --- a/src/test/java/org/apache/commons/cli/CommandLineTest.java +++ b/src/test/java/org/apache/commons/cli/CommandLineTest.java @@ -151,18 +151,6 @@ public void testGetParsedOptionValueWithChar() throws Exception { assertEquals("foo", cmd.getParsedOptionValue('f')); } - @Test - public void testGetParsedOptionValueWithNullSupplier() throws Exception { - final Options options = new Options(); - options.addOption(Option.builder("i").hasArg().type(Number.class).build()); - - final CommandLineParser parser = new DefaultParser(); - final CommandLine cmd = parser.parse(options, new String[0]); - final Supplier supplier = null; - - assertNull(cmd.getParsedOptionValue('i', supplier)); - } - @Test public void testGetParsedOptionValueWithOption() throws Exception { final Options options = new Options(); @@ -188,14 +176,27 @@ public void testGetParsedOptionValueUsingDefault() throws Exception { final CommandLineParser parser = new DefaultParser(); final CommandLine cmd = parser.parse(options, new String[] {"-i", "123"}); + final Supplier nullSupplier = null; assertEquals(123, ((Number) cmd.getParsedOptionValue(optI)).intValue()); assertEquals("foo", cmd.getParsedOptionValue(optF, "foo")); assertEquals("foo", cmd.getParsedOptionValue(optF, () -> "foo")); + assertNull(cmd.getParsedOptionValue(optF, null)); + assertNull(cmd.getParsedOptionValue(optF, nullSupplier)); + assertNull(cmd.getParsedOptionValue(optF, () -> null)); + assertEquals("foo", cmd.getParsedOptionValue("f", "foo")); assertEquals("foo", cmd.getParsedOptionValue("f", () -> "foo")); + assertNull(cmd.getParsedOptionValue("f", null)); + assertNull(cmd.getParsedOptionValue("f", nullSupplier)); + assertNull(cmd.getParsedOptionValue("f", () -> null)); + assertEquals("foo", cmd.getParsedOptionValue('f', "foo")); assertEquals("foo", cmd.getParsedOptionValue('f', () -> "foo")); + assertNull(cmd.getParsedOptionValue('f', null)); + assertNull(cmd.getParsedOptionValue('f', nullSupplier)); + assertNull(cmd.getParsedOptionValue('f', () -> null)); + } @Test