diff --git a/src/main/java/org/springframework/shell2/DefaultMethodTargetResolver.java b/src/main/java/org/springframework/shell2/DefaultMethodTargetResolver.java
index ddf9bb74..15d3e062 100644
--- a/src/main/java/org/springframework/shell2/DefaultMethodTargetResolver.java
+++ b/src/main/java/org/springframework/shell2/DefaultMethodTargetResolver.java
@@ -20,7 +20,7 @@ public class DefaultMethodTargetResolver implements MethodTargetResolver {
for (Object bean : commandBeans.values()) {
Class> clazz = bean.getClass();
ReflectionUtils.doWithMethods(clazz, method -> {
- ShellMehtod shellMapping = method.getAnnotation(ShellMehtod.class);
+ ShellMethod shellMapping = method.getAnnotation(ShellMethod.class);
String[] keys = shellMapping.value();
if (keys.length == 1 && "".equals(keys[0])) {
keys[0] = method.getName();
@@ -28,7 +28,7 @@ public class DefaultMethodTargetResolver implements MethodTargetResolver {
for (String key : keys) {
methodTargets.put(key, new MethodTarget(method, bean, shellMapping.help()));
}
- }, method -> method.getAnnotation(ShellMehtod.class) != null);
+ }, method -> method.getAnnotation(ShellMethod.class) != null);
}
return methodTargets;
}
diff --git a/src/main/java/org/springframework/shell2/DefaultParameterResolver.java b/src/main/java/org/springframework/shell2/DefaultParameterResolver.java
index 18fd42dd..ce6d767f 100644
--- a/src/main/java/org/springframework/shell2/DefaultParameterResolver.java
+++ b/src/main/java/org/springframework/shell2/DefaultParameterResolver.java
@@ -23,7 +23,7 @@ import org.springframework.util.ConcurrentReferenceHashMap;
/**
* Default ParameterResolver implementation that supports the following features:
- * - named parameters (recognized because they start with some {@link ShellMehtod#prefix()}
+ * - named parameters (recognized because they start with some {@link ShellMethod#prefix()}
* - implicit named parameters (from the actual method parameter name)
* - positional parameters (in order, for all parameter values that were not resolved via named parameters)
* - default values (for all remaining parameters)
@@ -63,7 +63,7 @@ class DefaultParameterResolver implements ParameterResolver {
@Override
public Object resolve(MethodParameter methodParameter, List words) {
- String prefix = methodParameter.getMethod().getAnnotation(ShellMehtod.class).prefix();
+ String prefix = methodParameter.getMethod().getAnnotation(ShellMethod.class).prefix();
CacheKey cacheKey = new CacheKey(methodParameter.getMethod(), words);
Map resolved = parameterCache.computeIfAbsent(cacheKey, (k) -> {
@@ -77,11 +77,11 @@ class DefaultParameterResolver implements ParameterResolver {
String word = words.get(i);
if (word.startsWith(prefix)) {
String key = word.substring(prefix.length());
- Parameter parameter = lookupParameterForKey(methodParameter.getMethod(), key);
+ Parameter parameter = lookupParameterForKey(methodParameter.getMethod(), key, prefix);
int arity = getArity(parameter);
String raw = words.subList(i + 1, i + 1 + arity).stream().collect(Collectors.joining(","));
- Assert.isTrue(!namedParameters.containsKey(key), String.format("Option '%s' has already been specified", word));
+ Assert.isTrue(!namedParameters.containsKey(key), String.format("Parameter for '%s' has already been specified", word));
namedParameters.put(key, raw);
result.put(parameter, raw);
i += arity;
@@ -123,9 +123,12 @@ class DefaultParameterResolver implements ParameterResolver {
}
}
else if (copy.size() > 1) {
- throw new RuntimeException("Named parameter has been specified multiple times via: " + copy);
+ throw new IllegalArgumentException("Named parameter has been specified multiple times via " + prefix(copy, prefix));
}
}
+
+ Assert.isTrue(offset == positionalValues.size(), "Too many arguments: the following could not be mapped to parameters: "
+ + positionalValues.subList(offset, positionalValues.size()).stream().collect(Collectors.joining(" ", "'", "'")));
return result;
});
@@ -133,6 +136,13 @@ class DefaultParameterResolver implements ParameterResolver {
return conversionService.convert(s, TypeDescriptor.valueOf(String.class), new TypeDescriptor(methodParameter));
}
+ /**
+ * Add the command prefix back to the list of keys that was used to invoke the method.
+ */
+ private String prefix(Collection keys, String prefix) {
+ return keys.stream().map(k -> prefix + k).collect(Collectors.joining(", ", "'", "'"));
+ }
+
private boolean booleanDefaultValue(Parameter parameter) {
ShellOption option = parameter.getAnnotation(ShellOption.class);
if (option != null && !ShellOption.NULL.equals(option.defaultValue())) {
@@ -177,7 +187,7 @@ class DefaultParameterResolver implements ParameterResolver {
/**
* Return the method parameter that should be bound to the given key.
*/
- private Parameter lookupParameterForKey(Method method, String key) {
+ private Parameter lookupParameterForKey(Method method, String key, String prefix) {
Parameter[] parameters = method.getParameters();
for (int i = 0, parametersLength = parameters.length; i < parametersLength; i++) {
Parameter p = parameters[i];
@@ -185,7 +195,7 @@ class DefaultParameterResolver implements ParameterResolver {
return p;
}
}
- throw new IllegalStateException(String.format("Could not lookup parameter for '%s' in %s", key, method));
+ throw new IllegalArgumentException(String.format("Could not look up parameter for '%s%s' in %s", prefix, key, method));
}
private static class CacheKey {
diff --git a/src/main/java/org/springframework/shell2/ShellMehtod.java b/src/main/java/org/springframework/shell2/ShellMehtod.java
deleted file mode 100644
index 1a5ac72e..00000000
--- a/src/main/java/org/springframework/shell2/ShellMehtod.java
+++ /dev/null
@@ -1,21 +0,0 @@
-package org.springframework.shell2;
-
-import java.lang.annotation.ElementType;
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-import java.lang.annotation.Target;
-
-/**
- * Created by ericbottard on 27/11/15.
- */
-@Retention(RetentionPolicy.RUNTIME)
-@Target({ElementType.METHOD})
-public @interface ShellMehtod {
-
- String[] value() default "";
-
- String help() default "";
-
- String prefix() default "--";
-
-}
diff --git a/src/main/java/org/springframework/shell2/ShellMethod.java b/src/main/java/org/springframework/shell2/ShellMethod.java
new file mode 100644
index 00000000..c7f2003f
--- /dev/null
+++ b/src/main/java/org/springframework/shell2/ShellMethod.java
@@ -0,0 +1,37 @@
+package org.springframework.shell2;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Used to mark a method as invokable via Spring Shell.
+ *
+ * @author Eric Bottard
+ * @author Florent Biville
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ElementType.METHOD})
+@Documented
+public @interface ShellMethod {
+
+ /**
+ * The name(s) by which this method can be invoked via Spring Shell. If not specified, the actual method name
+ * will be used (turning camelCase humps into "-").
+ */
+ String[] value() default {};
+
+ /**
+ * A description for the command. Should not contain any formatting (e.g. html) characters and would typically
+ * start with a capital letter and end with a dot.
+ */
+ String help() default "";
+
+ /**
+ * The prefix to use for assigning parameters by name.
+ */
+ String prefix() default "--";
+
+}
diff --git a/src/main/java/org/springframework/shell2/ShellOption.java b/src/main/java/org/springframework/shell2/ShellOption.java
index 377b97c7..598b0b17 100644
--- a/src/main/java/org/springframework/shell2/ShellOption.java
+++ b/src/main/java/org/springframework/shell2/ShellOption.java
@@ -7,7 +7,7 @@ import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
/**
- * Created by ericbottard on 27/11/15.
+ * Used to customize handling of a {@link ShellMethod} parameters.
*/
@Documented
@Retention(RetentionPolicy.RUNTIME)
@@ -16,9 +16,19 @@ public @interface ShellOption {
String NULL = "__NULL__";
+ /**
+ * The key(s) (without the {@link ShellMethod#prefix()}) by which this parameter can be referenced
+ * when using named parameters. If none is specified, the actual method parameter name will be used.
+ */
String[] value() default {};
+ /**
+ * Return the number of input "words" this parameter consumes.
+ */
int arity() default 1;
+ /**
+ * The textual (pre-conversion) value to assign to this parameter if no value is provided by the user.
+ */
String defaultValue() default NULL;
}
diff --git a/src/test/java/org/springframework/shell2/DefaultParameterResolverTest.java b/src/test/java/org/springframework/shell2/DefaultParameterResolverTest.java
index cb9401a3..4f89315b 100644
--- a/src/test/java/org/springframework/shell2/DefaultParameterResolverTest.java
+++ b/src/test/java/org/springframework/shell2/DefaultParameterResolverTest.java
@@ -1,6 +1,8 @@
package org.springframework.shell2;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.ExpectedException;
import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.MethodParameter;
import org.springframework.core.convert.support.DefaultConversionService;
@@ -19,6 +21,8 @@ import static org.springframework.util.ReflectionUtils.findMethod;
*/
public class DefaultParameterResolverTest {
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
private DefaultParameterResolver resolver = new DefaultParameterResolver(new DefaultConversionService());
@@ -45,6 +49,58 @@ public class DefaultParameterResolverTest {
}
+ @Test
+ public void testParameterSpecifiedTwiceViaDifferentAliases() throws Exception {
+ Method method = findMethod(Remote.class, "zap", boolean.class, String.class, String.class, String.class);
+
+ thrown.expect(IllegalArgumentException.class);
+ thrown.expectMessage("Named parameter has been specified multiple times via '--bar, --baz'");
+
+ resolver.resolve(
+ makeMethodParameter(method, 0),
+ asList("--force --name --foo y --bar x --baz z".split(" "))
+ );
+ }
+
+ @Test
+ public void testParameterSpecifiedTwiceViaSameKey() throws Exception {
+ Method method = findMethod(Remote.class, "zap", boolean.class, String.class, String.class, String.class);
+
+ thrown.expect(IllegalArgumentException.class);
+ thrown.expectMessage("Parameter for '--baz' has already been specified");
+
+ resolver.resolve(
+ makeMethodParameter(method, 0),
+ asList("--force --name --foo y --baz x --baz z".split(" "))
+ );
+ }
+
+ @Test
+ public void testUnknownParameter() throws Exception {
+ Method method = findMethod(Remote.class, "zap", boolean.class, String.class, String.class, String.class);
+
+ thrown.expect(IllegalArgumentException.class);
+ thrown.expectMessage("Could not look up parameter for '--unknown' in " + method);
+
+ resolver.resolve(
+ makeMethodParameter(method, 0),
+ asList("--unknown --foo bar".split(" "))
+ );
+ }
+
+ @Test
+ public void testTooMuchInput() throws Exception {
+ Method method = findMethod(Remote.class, "zap", boolean.class, String.class, String.class, String.class);
+
+ thrown.expect(IllegalArgumentException.class);
+ thrown.expectMessage("the following could not be mapped to parameters: 'leftover'");
+
+ resolver.resolve(
+ makeMethodParameter(method, 0),
+ asList("--foo hello --name bar --force --bar well leftover".split(" "))
+ );
+ }
+
private MethodParameter makeMethodParameter(Method method, int parameterIndex) {
MethodParameter methodParameter = new MethodParameter(method, parameterIndex);
methodParameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer());
diff --git a/src/test/java/org/springframework/shell2/Remote.java b/src/test/java/org/springframework/shell2/Remote.java
index f7af0ead..f41a5e94 100644
--- a/src/test/java/org/springframework/shell2/Remote.java
+++ b/src/test/java/org/springframework/shell2/Remote.java
@@ -15,11 +15,11 @@ public class Remote {
* - default value supplying (foo and bar)
*
*/
- @ShellMehtod
+ @ShellMethod
public void zap(boolean force,
String name,
@ShellOption(defaultValue="defoolt") String foo,
- @ShellOption(defaultValue = "last") String bar) {
+ @ShellOption(value = {"bar", "baz"}, defaultValue = "last") String bar) {
}
}