Implement ParameterDescription for the LegacyParameterResolver

Fixes #8

updated Help to support parameters with empty keys (e.g. default key in Shell 1)
updated LegacyParameterResolver to avoid including a default value if none is provided

included some additional sample commands

spring-shell2-core/src/main/java/org/springframework/shell2/commands/Help.java

@@ -107,12 +107,14 @@ public class Help {
 			if (description.defaultValue().isPresent()) {
 				result.append("[");
 			}
-			if (!description.mandatoryKey()) {
-				result.append("[");
-			}
-			result.append(description.keys().iterator().next(), AttributedStyle.BOLD);
-			if (!description.mandatoryKey()) {
-				result.append("]");
+			if(!description.keys().isEmpty()) {
+				if (!description.mandatoryKey()) {
+					result.append("[");
+				}
+				result.append(description.keys().iterator().next(), AttributedStyle.BOLD);
+				if (!description.mandatoryKey()) {
+					result.append("]");
+				}
 				if (!description.formal().isEmpty()) {
 					result.append(" ");
 				}
@@ -132,7 +134,9 @@ public class Help {
 		for (ParameterDescription description : parameterDescriptions) {
 			result.append("\t").append(description.keys().stream().collect(Collectors.joining(" or ")), AttributedStyle.BOLD);
 			if (description.formal().length() > 0) {
-				result.append("  ");
+				if (!description.keys().isEmpty()) {
+					result.append("  ");
+				}
 				appendUnderlinedFormal(result, description);
 				result.append("\n\t");
 			}

spring-shell2-core/src/main/java/org/springframework/shell2/standard/StandardParameterResolver.java

@@ -100,7 +100,7 @@ public class StandardParameterResolver implements ParameterResolver {
 
 	@Override
 	public boolean supports(MethodParameter parameter) {
-		return true;
+		return parameter.getMethodAnnotation(ShellMethod.class) != null;
 	}
 
 	@Override

spring-shell2-samples/src/main/java/org/springframework/shell2/samples/legacy/LegacyCommands.java

@@ -43,7 +43,7 @@ public class LegacyCommands implements CommandMarker {
 					key = {"type"},
 					help = "the type for the registered module")
 			ArtifactType type,
-			@CliOption(mandatory = true,
+			@CliOption(mandatory = false,
 					key = {"coordinates", "coords"},
 					help = "coordinates to the module archive")
 			String coordinates,
@@ -56,8 +56,28 @@ public class LegacyCommands implements CommandMarker {
 	}
 
 	@CliCommand(value = "sum", help = "adds two numbers")
-	public int sum(@CliOption(key = "v1", unspecifiedDefaultValue = "38") int a, @CliOption(key = "v2", specifiedDefaultValue = "42") int b) {
+	public int sum(
+		@CliOption(key = "v1", unspecifiedDefaultValue = "38") int a,
+		@CliOption(key = "v2", specifiedDefaultValue = "42") int b) {
 		return a + b;
 	}
 
+	@CliCommand(value = "sum2", help = "adds two numbers")
+	public int sum2(
+			@CliOption(key = "v1", unspecifiedDefaultValue = "38") int a,
+			@CliOption(key = "v2", specifiedDefaultValue = "42", mandatory = true) int b,
+			@CliOption(key = "v3", mandatory = false) int c) {
+		return a + b + c;
+	}
+	
+	@CliCommand(value = "legacy-echo", help = "Echoes a message")
+	public String legacyEcho(@CliOption(key = "", mandatory = true) String message) {
+		return message;
+	}
+	
+	@CliCommand(value = "optional-echo", help = "Echoes an optional message")
+	public String optionalEcho(@CliOption(key = "", mandatory = false) String message) {
+		return message;
+	}
+
 }

spring-shell2-shell1-adapter/src/main/java/org/springframework/shell2/legacy/LegacyParameterResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 the original author or authors.
+ * Copyright 2015-2017 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.
@@ -16,6 +16,7 @@
 
 package org.springframework.shell2.legacy;
 
+import java.lang.reflect.Parameter;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -41,10 +42,18 @@ import org.springframework.util.Assert;
  * Resolves parameters by looking at the {@link CliOption} annotation and acting accordingly.
  *
  * @author Eric Bottard
+ * @author Camilo Gonzalez
  */
 @Component
 public class LegacyParameterResolver implements ParameterResolver {
 
+	private static final String CLI_OPTION_NULL = "__NULL__";
+	
+	/**
+	 * Prefix used by Spring Shell 1 for the argument keys (<em>e.g.</em> command --key value).
+	 */
+	private static final String CLI_PREFIX = "--";
+	
 	@Autowired(required = false)
 	private Collection<Converter<?>> converters = new ArrayList<>();
 
@@ -82,7 +91,58 @@ public class LegacyParameterResolver implements ParameterResolver {
 
 	@Override
 	public ParameterDescription describe(MethodParameter parameter) {
-		throw new UnsupportedOperationException();
+		Parameter jlrParameter = parameter.getMethod().getParameters()[parameter.getParameterIndex()];
+		CliOption option = jlrParameter.getAnnotation(CliOption.class);
+		ParameterDescription result = ParameterDescription.outOf(parameter);
+		result.help(option.help());
+		List<String> keys = Arrays.asList(option.key());
+		result.keys(keys.stream()
+				.filter(key -> !key.isEmpty())
+				.map(key -> CLI_PREFIX + key)
+				.collect(Collectors.toList()));
+		Optional<String> defaultValue = defaultValueFor(option, result.keys());
+		if (defaultValue.isPresent()) {
+			result.defaultValue(defaultValue.get());
+		}
+		boolean containsEmptyKey = keys.contains("");
+		result.mandatoryKey(!containsEmptyKey);
+		return result;
+	}
+
+	private Optional<String> defaultValueFor(CliOption option, List<String> keys) {
+		// CliOption annotations have two default values, one for when the key is specified without a value,
+		// and one when the key isn't specified (e.g. "command --key" vs "command")
+
+		final boolean unspecifiedDefaultDeclared = !CLI_OPTION_NULL.equals(option.unspecifiedDefaultValue());
+		final boolean specifiedDefaultDeclared = !CLI_OPTION_NULL.equals(option.specifiedDefaultValue());
+
+		if (!unspecifiedDefaultDeclared && !specifiedDefaultDeclared) {
+			if (option.mandatory()) {
+				return Optional.empty();
+			} else {
+				// according to CliOption, is no default is declared, then null will be presented to non-primitive
+				// arguments
+				return Optional.of("null");
+			}
+		}
+
+		final StringBuilder defaultValue = new StringBuilder();
+
+		if (unspecifiedDefaultDeclared) {
+			defaultValue.append(option.unspecifiedDefaultValue());
+		}
+		
+		if (specifiedDefaultDeclared) {
+			if (unspecifiedDefaultDeclared) {
+				defaultValue.append(", or ");
+			}
+			
+			defaultValue.append(option.specifiedDefaultValue());
+			defaultValue.append(" if used as ");
+			defaultValue.append(keys.stream().collect(Collectors.joining(" or ")));
+		}
+
+		return Optional.of(defaultValue.toString());
 	}
 
 	@Override
@@ -137,5 +197,5 @@ public class LegacyParameterResolver implements ParameterResolver {
 	private Supplier<IllegalStateException> noConverterFound(String key, String value, Class<?> parameterType) {
 		return () -> new IllegalStateException("No converter found for --" + key + " from '" + value + "' to type " + parameterType);
 	}
-
+	
 }

spring-shell2-shell1-adapter/src/test/java/org/springframework/shell2/legacy/LegacyCommands.java

@@ -30,6 +30,8 @@ public class LegacyCommands implements CommandMarker {
 
 	public static final Method REGISTER_METHOD = ReflectionUtils.findMethod(LegacyCommands.class, "register", String.class, ArtifactType.class, String.class, boolean.class);
 	public static final Method SUM_METHOD = ReflectionUtils.findMethod(LegacyCommands.class, "sum", int.class, int.class);
+	public static final Method LEGACY_ECHO_METHOD = ReflectionUtils.findMethod(LegacyCommands.class, "legacyEcho", String.class);	
+	public static final Method SOME_METHOD = ReflectionUtils.findMethod(LegacyCommands.class, "someMethod", String.class, boolean.class);
 
 	@CliCommand(value = "register module", help = "Register a new module")
 	public String register(
@@ -57,5 +59,17 @@ public class LegacyCommands implements CommandMarker {
 	public int sum(@CliOption(key = "v1", unspecifiedDefaultValue = "38") int a, @CliOption(key = "v2", specifiedDefaultValue = "42") int b) {
 		return a + b;
 	}
+	
+	@CliCommand(value = "legacy-echo", help = "Echoes a message")
+	public String legacyEcho(@CliOption(key = "", mandatory = true) String message) {
+		return message;
+	}
 
+	@CliCommand(value = "someMethod", help = "Method used for testing purposes")
+	public String someMethod(
+			@CliOption(key = "key", mandatory = false, help = "The optional parameter") String parameter,
+			@CliOption(key = "option", help = "an option", specifiedDefaultValue = "true", unspecifiedDefaultValue = "false", mandatory = true) boolean option) {
+		return parameter + ", " + option;
+	}
+	
 }

spring-shell2-shell1-adapter/src/test/java/org/springframework/shell2/legacy/LegacyParameterResolverTest.java

@@ -33,6 +33,8 @@ import org.springframework.shell.converters.BooleanConverter;
 import org.springframework.shell.converters.EnumConverter;
 import org.springframework.shell.converters.StringConverter;
 import org.springframework.shell.core.Converter;
+import org.springframework.shell.core.annotation.CliOption;
+import org.springframework.shell2.ParameterDescription;
 import org.springframework.shell2.ParameterResolver;
 import org.springframework.shell2.Utils;
 import org.springframework.test.context.ContextConfiguration;
@@ -153,6 +155,96 @@ public class LegacyParameterResolverTest {
 		thrown.expectMessage("No converter found for --v2 from '42' to type int");
 		resolve(methodParameter, "--v1 1 --v2");
 	}
+	
+	@Test
+	public void testDescribeBothDefaultsNotDeclared() {
+		MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.REGISTER_METHOD, 1);
+		
+		ParameterDescription description = parameterResolver.describe(methodParameter);
+		
+		assertThat(description.keys()).containsExactly("--type");
+		assertThat(description.formal()).isEqualTo(Utils.unCamelify(ArtifactType.class.getSimpleName()));
+		assertThat(description.defaultValue().isPresent()).isFalse();
+		assertThat(description.mandatoryKey()).isTrue();
+		
+		String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
+		assertThat(description.help()).isEqualTo(expectedHelp);
+	}
+	
+	@Test
+	public void testDescribeBothDefaultsDeclared() {
+		MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SOME_METHOD, 1);
+		
+		ParameterDescription description = parameterResolver.describe(methodParameter);
+		
+		assertThat(description.keys()).containsExactly("--option");
+		assertThat(description.formal()).isEqualTo(boolean.class.getName());
+		assertThat(description.defaultValue().get()).isEqualTo("false, or true if used as --option");
+		assertThat(description.mandatoryKey()).isTrue();
+		
+		String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
+		assertThat(description.help()).isEqualTo(expectedHelp);
+	}
+	
+	@Test
+	public void testDescribeOnlySpecifiedDefaultDeclared() {
+		MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SUM_METHOD, 1);
+		
+		ParameterDescription description = parameterResolver.describe(methodParameter);
+		
+		assertThat(description.keys()).containsExactly("--v2");
+		assertThat(description.formal()).isEqualTo(int.class.getName());
+		assertThat(description.defaultValue().get()).isEqualTo("42 if used as --v2");
+		assertThat(description.mandatoryKey()).isTrue();
+		
+		String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
+		assertThat(description.help()).isEqualTo(expectedHelp);
+	}
+	
+	@Test
+	public void testDescribeOnlyUnspecifiedDefaultDeclared() {
+		MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SUM_METHOD, 0);
+		
+		ParameterDescription description = parameterResolver.describe(methodParameter);
+		
+		assertThat(description.keys()).containsExactly("--v1");
+		assertThat(description.formal()).isEqualTo(int.class.getName());
+		assertThat(description.defaultValue().get()).isEqualTo("38");
+		assertThat(description.mandatoryKey()).isTrue();
+		
+		String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
+		assertThat(description.help()).isEqualTo(expectedHelp);
+	}
+	
+	@Test
+	public void testDescribeDefaultKey() {
+		MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.LEGACY_ECHO_METHOD, 0);
+		
+		ParameterDescription description = parameterResolver.describe(methodParameter);
+		
+		assertThat(description.keys()).isEmpty();
+		assertThat(description.formal()).isEqualTo(Utils.unCamelify(String.class.getSimpleName()));
+		assertThat(description.defaultValue().isPresent()).isFalse();
+		assertThat(description.mandatoryKey()).isFalse();
+		
+		String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
+		assertThat(description.help()).isEqualTo(expectedHelp);
+	}
+	
+	@Test
+	public void testDescribeNonMandatoryNoDefaults() {
+		MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SOME_METHOD, 0);
+		
+		ParameterDescription description = parameterResolver.describe(methodParameter);
+		
+		assertThat(description.keys()).containsExactly("--key");
+		assertThat(description.formal()).isEqualTo(Utils.unCamelify(String.class.getSimpleName()));
+		assertThat(description.defaultValue().get()).isEqualTo("null");
+		assertThat(description.mandatoryKey()).isTrue();
+		
+		String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help();
+		assertThat(description.help()).isEqualTo(expectedHelp);
+	}
 
 	private Object resolve(MethodParameter methodParameter, String command) {
 		return parameterResolver.resolve(methodParameter, asList(command.split(" ")));