Support optional command line arguments with empty values

Spring Framework provides two implementations of the
CommandLinePropertySource API: SimpleCommandLinePropertySource and
JOptCommandLinePropertySource.

Prior to this commit, JOptCommandLinePropertySource supported empty
values for optional arguments; whereas, SimpleCommandLinePropertySource
did not.

This commit modifies the implementation of SimpleCommandLinePropertySource
to allow empty values for optional arguments.

Closes gh-24464

spring-core/src/main/java/org/springframework/core/env/SimpleCommandLineArgsParser.java

@@ -21,15 +21,20 @@ package org.springframework.core.env;
  * {@link CommandLineArgs} object.
  *
  * <h3>Working with option arguments</h3>
- * Option arguments must adhere to the exact syntax:
+ * <p>Option arguments must adhere to the exact syntax:
+ *
  * <pre class="code">--optName[=optValue]</pre>
- * That is, options must be prefixed with "{@code --}", and may or may not specify a value.
+ *
- * If a value is specified, the name and value must be separated <em>without spaces</em>
+ * <p>That is, options must be prefixed with "{@code --}" and may or may not
- * by an equals sign ("=").
+ * specify a value. If a value is specified, the name and value must be separated
+ * <em>without spaces</em> by an equals sign ("="). The value may optionally be
+ * an empty string.
  *
  * <h4>Valid examples of option arguments</h4>
  * <pre class="code">
  * --foo
+ * --foo=
+ * --foo=""
  * --foo=bar
  * --foo="bar then baz"
  * --foo=bar,baz,biz</pre>
@@ -42,11 +47,12 @@ package org.springframework.core.env;
  * --foo=bar --foo=baz --foo=biz</pre>
  *
  * <h3>Working with non-option arguments</h3>
- * Any and all arguments specified at the command line without the "{@code --}" option
+ * <p>Any and all arguments specified at the command line without the "{@code --}"
- * prefix will be considered as "non-option arguments" and made available through the
+ * option prefix will be considered as "non-option arguments" and made available
- * {@link CommandLineArgs#getNonOptionArgs()} method.
+ * through the {@link CommandLineArgs#getNonOptionArgs()} method.
  *
  * @author Chris Beams
+ * @author Sam Brannen
  * @since 3.1
  */
 class SimpleCommandLineArgsParser {
@@ -61,17 +67,18 @@ class SimpleCommandLineArgsParser {
 		CommandLineArgs commandLineArgs = new CommandLineArgs();
 		for (String arg : args) {
 			if (arg.startsWith("--")) {
-				String optionText = arg.substring(2, arg.length());
+				String optionText = arg.substring(2);
 				String optionName;
 				String optionValue = null;
-				if (optionText.contains("=")) {
+				int indexOfEqualsSign = optionText.indexOf('=');
-					optionName = optionText.substring(0, optionText.indexOf('='));
+				if (indexOfEqualsSign > -1) {
-					optionValue = optionText.substring(optionText.indexOf('=') + 1, optionText.length());
+					optionName = optionText.substring(0, indexOfEqualsSign);
+					optionValue = optionText.substring(indexOfEqualsSign + 1);
 				}
 				else {
 					optionName = optionText;
 				}
-				if (optionName.isEmpty() || (optionValue != null && optionValue.isEmpty())) {
+				if (optionName.isEmpty()) {
 					throw new IllegalArgumentException("Invalid argument syntax: " + arg);
 				}
 				commandLineArgs.addOptionArg(optionName, optionValue);

spring-core/src/main/java/org/springframework/core/env/SimpleCommandLinePropertySource.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -25,22 +25,28 @@ import org.springframework.util.StringUtils;
  * {@link CommandLinePropertySource} implementation backed by a simple String array.
  *
  * <h3>Purpose</h3>
- * This {@code CommandLinePropertySource} implementation aims to provide the simplest
+ * <p>This {@code CommandLinePropertySource} implementation aims to provide the simplest
- * possible approach to parsing command line arguments.  As with all {@code
+ * possible approach to parsing command line arguments. As with all {@code
  * CommandLinePropertySource} implementations, command line arguments are broken into two
  * distinct groups: <em>option arguments</em> and <em>non-option arguments</em>, as
- * described below <em>(some sections copied from Javadoc for {@link SimpleCommandLineArgsParser})</em>:
+ * described below <em>(some sections copied from Javadoc for
+ * {@link SimpleCommandLineArgsParser})</em>:
  *
  * <h3>Working with option arguments</h3>
- * Option arguments must adhere to the exact syntax:
+ * <p>Option arguments must adhere to the exact syntax:
+ *
  * <pre class="code">--optName[=optValue]</pre>
- * That is, options must be prefixed with "{@code --}", and may or may not specify a value.
+ *
- * If a value is specified, the name and value must be separated <em>without spaces</em>
+ * <p>That is, options must be prefixed with "{@code --}" and may or may not
- * by an equals sign ("=").
+ * specify a value. If a value is specified, the name and value must be separated
+ * <em>without spaces</em> by an equals sign ("="). The value may optionally be
+ * an empty string.
  *
  * <h4>Valid examples of option arguments</h4>
  * <pre class="code">
  * --foo
+ * --foo=
+ * --foo=""
  * --foo=bar
  * --foo="bar then baz"
  * --foo=bar,baz,biz</pre>
@@ -53,11 +59,11 @@ import org.springframework.util.StringUtils;
  * --foo=bar --foo=baz --foo=biz</pre>
  *
  * <h3>Working with non-option arguments</h3>
- * Any and all arguments specified at the command line without the "{@code --}" option
+ * <p>Any and all arguments specified at the command line without the "{@code --}"
- * prefix will be considered as "non-option arguments" and made available through the
+ * option prefix will be considered as "non-option arguments" and made available
- * {@link #getNonOptionArgs()} method.
+ * through the {@link CommandLineArgs#getNonOptionArgs()} method.
  *
- * <h2>Typical usage</h2>
+ * <h3>Typical usage</h3>
  * <pre class="code">
  * public static void main(String[] args) {
  *     PropertySource<?> ps = new SimpleCommandLinePropertySource(args);
@@ -71,7 +77,7 @@ import org.springframework.util.StringUtils;
  * <p>When more fully-featured command line parsing is necessary, consider using
  * the provided {@link JOptCommandLinePropertySource}, or implement your own
  * {@code CommandLinePropertySource} against the command line parsing library of your
- * choice!
+ * choice.
  *
  * @author Chris Beams
  * @since 3.1

spring-core/src/test/java/org/springframework/core/env/JOptCommandLinePropertySourceTests.java

@@ -28,6 +28,7 @@ import static org.assertj.core.api.Assertions.assertThat;
  * Unit tests for {@link JOptCommandLinePropertySource}.
  *
  * @author Chris Beams
+ * @author Sam Brannen
  * @since 3.1
  */
 class JOptCommandLinePropertySourceTests {
@@ -53,6 +54,17 @@ class JOptCommandLinePropertySourceTests {
 		assertThat(ps.getProperty("foo")).isEqualTo("");
 	}
 
+	@Test // gh-24464
+	void withOptionalArg_andArgIsEmpty() {
+		OptionParser parser = new OptionParser();
+		parser.accepts("foo").withOptionalArg();
+		OptionSet options = parser.parse("--foo=");
+
+		PropertySource<?> ps = new JOptCommandLinePropertySource(options);
+		assertThat(ps.containsProperty("foo")).isTrue();
+		assertThat(ps.getProperty("foo")).isEqualTo("");
+	}
+
 	@Test
 	void withNoArg() {
 		OptionParser parser = new OptionParser();
@@ -74,7 +86,7 @@ class JOptCommandLinePropertySourceTests {
 		OptionSet options = parser.parse("--foo=bar,baz,biz");
 
 		CommandLinePropertySource<?> ps = new JOptCommandLinePropertySource(options);
-		assertThat(ps.getOptionValues("foo")).isEqualTo(Arrays.asList("bar","baz","biz"));
+		assertThat(ps.getOptionValues("foo")).containsExactly("bar", "baz", "biz");
 		assertThat(ps.getProperty("foo")).isEqualTo("bar,baz,biz");
 	}
 
@@ -85,7 +97,7 @@ class JOptCommandLinePropertySourceTests {
 		OptionSet options = parser.parse("--foo=bar", "--foo=baz", "--foo=biz");
 
 		CommandLinePropertySource<?> ps = new JOptCommandLinePropertySource(options);
-		assertThat(ps.getOptionValues("foo")).isEqualTo(Arrays.asList("bar","baz","biz"));
+		assertThat(ps.getOptionValues("foo")).containsExactly("bar", "baz", "biz");
 		assertThat(ps.getProperty("foo")).isEqualTo("bar,baz,biz");
 	}
 
@@ -123,7 +135,7 @@ class JOptCommandLinePropertySourceTests {
 
 		assertThat(ps.containsProperty("nonOptionArgs")).isFalse();
 		assertThat(ps.getProperty("nonOptionArgs")).isNull();
-		assertThat(ps.getPropertyNames().length).isEqualTo(2);
+		assertThat(ps.getPropertyNames()).hasSize(2);
 	}
 
 	@Test

spring-core/src/test/java/org/springframework/core/env/SimpleCommandLineArgsParserTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -25,17 +25,24 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 
-class SimpleCommandLineParserTests {
+/**
+ * Unit tests for {@link SimpleCommandLineArgsParser}.
+ *
+ * @author Chris Beams
+ * @author Sam Brannen
+ */
+class SimpleCommandLineArgsParserTests {
+
+	private final SimpleCommandLineArgsParser parser = new SimpleCommandLineArgsParser();
+
 
 	@Test
 	void withNoOptions() {
-		SimpleCommandLineArgsParser parser = new SimpleCommandLineArgsParser();
 		assertThat(parser.parse().getOptionValues("foo")).isNull();
 	}
 
 	@Test
 	void withSingleOptionAndNoValue() {
-		SimpleCommandLineArgsParser parser = new SimpleCommandLineArgsParser();
 		CommandLineArgs args = parser.parse("--o1");
 		assertThat(args.containsOption("o1")).isTrue();
 		assertThat(args.getOptionValues("o1")).isEqualTo(Collections.EMPTY_LIST);
@@ -43,63 +50,52 @@ class SimpleCommandLineParserTests {
 
 	@Test
 	void withSingleOptionAndValue() {
-		SimpleCommandLineArgsParser parser = new SimpleCommandLineArgsParser();
 		CommandLineArgs args = parser.parse("--o1=v1");
 		assertThat(args.containsOption("o1")).isTrue();
-		assertThat(args.getOptionValues("o1").get(0)).isEqualTo("v1");
+		assertThat(args.getOptionValues("o1")).containsExactly("v1");
 	}
 
 	@Test
 	void withMixOfOptionsHavingValueAndOptionsHavingNoValue() {
-		SimpleCommandLineArgsParser parser = new SimpleCommandLineArgsParser();
 		CommandLineArgs args = parser.parse("--o1=v1", "--o2");
 		assertThat(args.containsOption("o1")).isTrue();
 		assertThat(args.containsOption("o2")).isTrue();
 		assertThat(args.containsOption("o3")).isFalse();
-		assertThat(args.getOptionValues("o1").get(0)).isEqualTo("v1");
+		assertThat(args.getOptionValues("o1")).containsExactly("v1");
 		assertThat(args.getOptionValues("o2")).isEqualTo(Collections.EMPTY_LIST);
 		assertThat(args.getOptionValues("o3")).isNull();
 	}
 
 	@Test
 	void withEmptyOptionText() {
-		SimpleCommandLineArgsParser parser = new SimpleCommandLineArgsParser();
+		assertThatIllegalArgumentException().isThrownBy(() -> parser.parse("--"));
-		assertThatIllegalArgumentException().isThrownBy(() ->
-				parser.parse("--"));
 	}
 
 	@Test
 	void withEmptyOptionName() {
-		SimpleCommandLineArgsParser parser = new SimpleCommandLineArgsParser();
+		assertThatIllegalArgumentException().isThrownBy(() -> parser.parse("--=v1"));
-		assertThatIllegalArgumentException().isThrownBy(() ->
-				parser.parse("--=v1"));
 	}
 
 	@Test
 	void withEmptyOptionValue() {
-		SimpleCommandLineArgsParser parser = new SimpleCommandLineArgsParser();
+		CommandLineArgs args = parser.parse("--o1=");
-		assertThatIllegalArgumentException().isThrownBy(() ->
+		assertThat(args.containsOption("o1")).isTrue();
-				parser.parse("--o1="));
+		assertThat(args.getOptionValues("o1")).containsExactly("");
 	}
 
 	@Test
 	void withEmptyOptionNameAndEmptyOptionValue() {
-		SimpleCommandLineArgsParser parser = new SimpleCommandLineArgsParser();
+		assertThatIllegalArgumentException().isThrownBy(() -> parser.parse("--="));
-		assertThatIllegalArgumentException().isThrownBy(() ->
-				parser.parse("--="));
 	}
 
 	@Test
 	void withNonOptionArguments() {
-		SimpleCommandLineArgsParser parser = new SimpleCommandLineArgsParser();
 		CommandLineArgs args = parser.parse("--o1=v1", "noa1", "--o2=v2", "noa2");
-		assertThat(args.getOptionValues("o1").get(0)).isEqualTo("v1");
+		assertThat(args.getOptionValues("o1")).containsExactly("v1");
-		assertThat(args.getOptionValues("o2").get(0)).isEqualTo("v2");
+		assertThat(args.getOptionValues("o2")).containsExactly("v2");
 
 		List<String> nonOptions = args.getNonOptionArgs();
-		assertThat(nonOptions.get(0)).isEqualTo("noa1");
+		assertThat(nonOptions).containsExactly("noa1", "noa2");
-		assertThat(nonOptions.get(1)).isEqualTo("noa2");
-		assertThat(nonOptions.size()).isEqualTo(2);
 	}
 
 	@Test

spring-core/src/test/java/org/springframework/core/env/SimpleCommandLinePropertySourceTests.java

@@ -26,6 +26,7 @@ import static org.assertj.core.api.Assertions.assertThat;
  * Unit tests for {@link SimpleCommandLinePropertySource}.
  *
  * @author Chris Beams
+ * @author Sam Brannen
  * @since 3.1
  */
 class SimpleCommandLinePropertySourceTests {
@@ -60,6 +61,14 @@ class SimpleCommandLinePropertySourceTests {
 		assertThat(ps.getProperty("o3")).isNull();
 	}
 
+	@Test // gh-24464
+	void withOptionalArg_andArgIsEmpty() {
+		EnumerablePropertySource<?> ps = new SimpleCommandLinePropertySource("--foo=");
+
+		assertThat(ps.containsProperty("foo")).isTrue();
+		assertThat(ps.getProperty("foo")).isEqualTo("");
+	}
+
 	@Test
 	void withDefaultNonOptionArgsNameAndNoNonOptionArgsPresent() {
 		EnumerablePropertySource<?> ps = new SimpleCommandLinePropertySource("--o1=v1", "--o2");
@@ -70,7 +79,7 @@ class SimpleCommandLinePropertySourceTests {
 
 		assertThat(ps.containsProperty("nonOptionArgs")).isFalse();
 		assertThat(ps.getProperty("nonOptionArgs")).isNull();
-		assertThat(ps.getPropertyNames().length).isEqualTo(2);
+		assertThat(ps.getPropertyNames()).hasSize(2);
 	}
 
 	@Test