Handle collection types in a parser

- Handle any option collection type so that list is generated for
  values, this then works well when actual type conversions happen.
- Fixes #630

Author: jvalkeal

spring-shell-core/src/main/java/org/springframework/shell/command/CommandParser.java

@@ -18,10 +18,12 @@
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.Deque;
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -466,6 +468,13 @@ private ConvertArgumentsHolder convertArguments(CommandOption option, List<Strin
 				else if (type != null && type.isArray()) {
 					value = arguments.stream().collect(Collectors.toList()).toArray();
 				}
+				// if it looks like type is a collection just get as list
+				// as conversion will happen later. we just need to know
+				// if user has Set, List, Collection, etc without worrying
+				// about generics.
+				else if (type != null && type.asCollection() != ResolvableType.NONE) {
+					value = arguments.stream().collect(Collectors.toList());
+				}
 				else {
 					if (!arguments.isEmpty()) {
 						if (arguments.size() == 1) {

spring-shell-core/src/test/java/org/springframework/shell/command/CommandParserTests.java

@@ -310,6 +310,39 @@ public void testLongOptionsWithArray() {
 		assertThat(results.results().get(0).value()).isEqualTo(new int[] { 1, 2 });
 	}
 
+	@Test
+	public void testLongOptionsWithStringArray() {
+		CommandOption option1 = longOption("arg1", ResolvableType.forType(String[].class));
+		List<CommandOption> options = Arrays.asList(option1);
+		String[] args = new String[]{"--arg1", "1", "2"};
+		CommandParserResults results = parser.parse(options, args);
+		assertThat(results.results()).hasSize(1);
+		assertThat(results.results().get(0).option()).isSameAs(option1);
+		assertThat(results.results().get(0).value()).isEqualTo(new String[] { "1", "2" });
+	}
+
+	@Test
+	public void testLongOptionsWithPlainList() {
+		CommandOption option1 = longOption("arg1", ResolvableType.forType(List.class));
+		List<CommandOption> options = Arrays.asList(option1);
+		String[] args = new String[]{"--arg1", "1", "2"};
+		CommandParserResults results = parser.parse(options, args);
+		assertThat(results.results()).hasSize(1);
+		assertThat(results.results().get(0).option()).isSameAs(option1);
+		assertThat(results.results().get(0).value()).isEqualTo(Arrays.asList("1", "2"));
+	}
+
+	@Test
+	public void testLongOptionsWithTypedList() {
+		CommandOption option1 = longOption("arg1", ResolvableType.forClassWithGenerics(List.class, String.class));
+		List<CommandOption> options = Arrays.asList(option1);
+		String[] args = new String[]{"--arg1", "1", "2"};
+		CommandParserResults results = parser.parse(options, args);
+		assertThat(results.results()).hasSize(1);
+		assertThat(results.results().get(0).option()).isSameAs(option1);
+		assertThat(results.results().get(0).value()).isEqualTo(Arrays.asList("1", "2"));
+	}
+
 	@Test
 	public void testArityErrors() {
 		CommandOption option1 = CommandOption.of(

spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/OptionTypeCommands.java

@@ -16,6 +16,9 @@
 package org.springframework.shell.samples.e2e;
 
 import java.io.PrintWriter;
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
 
 import org.springframework.context.annotation.Bean;
 import org.springframework.shell.command.CommandRegistration;
@@ -289,6 +292,96 @@ public CommandRegistration optionTypeIntArrayRegistration(CommandRegistration.Bu
 			.build();
 	}
 
+	//
+	// List<String>
+	//
+
+	@ShellMethod(key = LEGACY_ANNO + "option-type-string-list", group = GROUP)
+	public String optionTypeStringListAnnotation(
+		@ShellOption(help = "Desc arg1") List<String> arg1
+	) {
+		return "Hello " + arg1;
+	}
+
+	@Bean
+	public CommandRegistration optionTypeStringListRegistration(CommandRegistration.BuilderSupplier builder) {
+		return builder.get()
+			.command(REG, "option-type-string-list")
+			.group(GROUP)
+			.withOption()
+				.longNames("arg1")
+				.type(List.class)
+				.required()
+				.and()
+			.withTarget()
+				.function(ctx -> {
+					List<String> arg1 = ctx.getOptionValue("arg1");
+					return "Hello " + arg1;
+				})
+				.and()
+			.build();
+	}
+
+	//
+	// Set<String>
+	//
+
+	@ShellMethod(key = LEGACY_ANNO + "option-type-string-set", group = GROUP)
+	public String optionTypeStringSetAnnotation(
+		@ShellOption(help = "Desc arg1") Set<String> arg1
+	) {
+		return "Hello " + arg1;
+	}
+
+	@Bean
+	public CommandRegistration optionTypeStringSetRegistration(CommandRegistration.BuilderSupplier builder) {
+		return builder.get()
+			.command(REG, "option-type-string-set")
+			.group(GROUP)
+			.withOption()
+				.longNames("arg1")
+				.type(Set.class)
+				.required()
+				.and()
+			.withTarget()
+				.function(ctx -> {
+					Set<String> arg1 = ctx.getOptionValue("arg1");
+					return "Hello " + arg1;
+				})
+				.and()
+			.build();
+	}
+
+	//
+	// Collection<String>
+	//
+
+	@ShellMethod(key = LEGACY_ANNO + "option-type-string-collection", group = GROUP)
+	public String optionTypeStringCollectionAnnotation(
+		@ShellOption(help = "Desc arg1") Collection<String> arg1
+	) {
+		return "Hello " + arg1;
+	}
+
+	@Bean
+	public CommandRegistration optionTypeStringCollectionRegistration(CommandRegistration.BuilderSupplier builder) {
+		return builder.get()
+			.command(REG, "option-type-string-collection")
+			.group(GROUP)
+			.withOption()
+				.longNames("arg1")
+				.type(Collection.class)
+				.required()
+				.and()
+			.withTarget()
+				.function(ctx -> {
+					Collection<String> arg1 = ctx.getOptionValue("arg1");
+					return "Hello " + arg1;
+				})
+				.and()
+			.build();
+	}
+
 	//
 	// Void
 	//