From 3b9bbff2f20041a82e4a0f59b332a233befb221c Mon Sep 17 00:00:00 2001
From: Eric Bottard <eric.bottard+github@gmail.com>
Date: Thu, 18 May 2017 17:01:25 +0200
Subject: [PATCH] Fix completion with arity != 1

Fixes #24

Handle arrays and primitives
---
 .../standard/StandardParameterResolver.java   | 35 ++++++++----
 .../shell2/standard/Remote.java               | 16 +++++-
 .../StandardParameterResolverTest.java        | 54 +++++++++----------
 .../shell2/samples/standard/Commands.java     |  9 +++-
 4 files changed, 73 insertions(+), 41 deletions(-)

diff --git a/spring-shell2-core/src/main/java/org/springframework/shell2/standard/StandardParameterResolver.java b/spring-shell2-core/src/main/java/org/springframework/shell2/standard/StandardParameterResolver.java
index b849e062..a9aea569 100644
--- a/spring-shell2-core/src/main/java/org/springframework/shell2/standard/StandardParameterResolver.java
+++ b/spring-shell2-core/src/main/java/org/springframework/shell2/standard/StandardParameterResolver.java
@@ -16,8 +16,7 @@
 
 package org.springframework.shell2.standard;
 
-import static org.springframework.shell2.Utils.unCamelify;
-
+import java.lang.reflect.Array;
 import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
 import java.util.ArrayList;
@@ -48,6 +47,9 @@ import org.springframework.shell2.Utils;
 import org.springframework.stereotype.Component;
 import org.springframework.util.Assert;
 import org.springframework.util.ConcurrentReferenceHashMap;
+import org.springframework.util.ObjectUtils;
+
+import static org.springframework.shell2.Utils.unCamelify;
 
 /**
  * Default ParameterResolver implementation that supports the following features:<ul>
@@ -178,7 +180,12 @@ public class StandardParameterResolver implements ParameterResolver {
 		if (!resolved.containsKey(param)) {
 			throw new ParameterMissingResolutionException(describe(methodParameter));
 		}
-		String s = resolved.get(param).value;
+		ParameterRawValue parameterRawValue = resolved.get(param);
+		return convertRawValue(parameterRawValue, methodParameter);
+	}
+
+	private Object convertRawValue(ParameterRawValue parameterRawValue, MethodParameter methodParameter) {
+		String s = parameterRawValue.value;
 		if (ShellOption.NULL.equals(s)) {
 			return null;
 		}
@@ -261,10 +268,12 @@ public class StandardParameterResolver implements ParameterResolver {
 		Exception unfinished = null;
 		// First try to see if this parameter has been set, even to some unfinished value
 		ParameterRawValue parameterRawValue = null;
+		int arity = 1;
 		try {
 			resolve(methodParameter, context.getWords());
 			CacheKey cacheKey = new CacheKey(methodParameter.getMethod(), context.getWords());
 			Parameter parameter = methodParameter.getMethod().getParameters()[methodParameter.getParameterIndex()];
+			arity = getArity(parameter);
 			parameterRawValue = parameterCache.get(cacheKey).get(parameter);
 			set = parameterRawValue.explicit;
 		}
@@ -279,11 +288,12 @@ public class StandardParameterResolver implements ParameterResolver {
 			//return Collections.emptyList();
 		}
 
-		// There are 3 possible cases:
+		// There are 4 possible cases:
 		// 1) parameter not set at all
 		// 2) parameter set via its key, not enough input to consume a value
-		// 3) parameter set, and some value bound. But maybe that value is just a prefix to what the user actually wants
-		// 3.1) or maybe that value was resolved by position, but is a prefix of an actual valid key
+		// 3) parameter set with multiple values, enough to cover arity. We're done
+		// 4) parameter set, and some value bound. But maybe that value is just a prefix to what the user actually wants
+		// 4.1) or maybe that value was resolved by position, but is a prefix of an actual valid key
 
 		if (!set) {
 			if (unfinished == null) { // case 1 above
@@ -298,11 +308,18 @@ public class StandardParameterResolver implements ParameterResolver {
 
 			String prefix = context.currentWordUpToCursor() != null ? context.currentWordUpToCursor() : "";
 			// TODO: should not look at last word only, but everything after what was used for key
-			// Case 3
+
+			Object value = convertRawValue(parameterRawValue, methodParameter);
+			if (value instanceof Collection && ((Collection) value).size() == arity
+					|| (ObjectUtils.isArray(value) && Array.getLength(value) == arity)) {
+				// We're done already
+				return result;
+			}
+			// Case 4
 			result.addAll(valueCompletions(methodParameter, context));
 
 			if (parameterRawValue.positional()) {
-				// Case 3.1: There exists "--command foo" and user has typed "--comm" which (wrongly) got resolved as a positional param
+				// Case 4.1: There exists "--command foo" and user has typed "--comm" which (wrongly) got resolved as a positional param
 				result.addAll(commandsThatStartWithContextPrefix(methodParameter, context));
 			}
 			return result;
@@ -320,7 +337,7 @@ public class StandardParameterResolver implements ParameterResolver {
 		String prefix = context.currentWordUpToCursor() != null ? context.currentWordUpToCursor() : "";
 		return describe(methodParameter).keys().stream()
 				.filter(k -> k.startsWith(prefix))
-				.map(v -> new CompletionProposal(v))
+				.map(CompletionProposal::new)
 				.collect(Collectors.toList());
 	}
 
diff --git a/spring-shell2-core/src/test/java/org/springframework/shell2/standard/Remote.java b/spring-shell2-core/src/test/java/org/springframework/shell2/standard/Remote.java
index 0f7b3025..a1433e12 100644
--- a/spring-shell2-core/src/test/java/org/springframework/shell2/standard/Remote.java
+++ b/spring-shell2-core/src/test/java/org/springframework/shell2/standard/Remote.java
@@ -19,6 +19,7 @@ package org.springframework.shell2.standard;
 import java.util.Arrays;
 import java.util.List;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.springframework.core.MethodParameter;
 import org.springframework.shell2.CompletionContext;
@@ -57,6 +58,11 @@ public class Remote {
 
 	}
 
+	@ShellMethod(help = "add 3 numbers together (array)")
+	public void addAsArray(@ShellOption(arity = 3, valueProvider = NumberValueProvider.class) int[] numbers) {
+
+	}
+
 	public enum Delay {
 		small, medium, big;
 	}
@@ -64,12 +70,18 @@ public class Remote {
 
 	public static class NumberValueProvider extends ValueProviderSupport {
 
+		private final String[] values;
+
+		public NumberValueProvider(String... values) {
+			this.values = values;
+		}
+
 		@Override
 		public List<CompletionProposal> complete(MethodParameter parameter, CompletionContext completionContext, String[] hints) {
 			String prefix = completionContext.currentWord() != null ? completionContext.currentWord() : "";
-			return Arrays.asList("12", "42", "7").stream()
+			return Stream.of(values)
 					.filter(n -> n.startsWith(prefix))
-					.map(n -> new CompletionProposal(n))
+					.map(CompletionProposal::new)
 					.collect(Collectors.toList());
 		}
 	}
diff --git a/spring-shell2-core/src/test/java/org/springframework/shell2/standard/StandardParameterResolverTest.java b/spring-shell2-core/src/test/java/org/springframework/shell2/standard/StandardParameterResolverTest.java
index 3ccbeacc..56dd5010 100644
--- a/spring-shell2-core/src/test/java/org/springframework/shell2/standard/StandardParameterResolverTest.java
+++ b/spring-shell2-core/src/test/java/org/springframework/shell2/standard/StandardParameterResolverTest.java
@@ -16,18 +16,12 @@
 
 package org.springframework.shell2.standard;
 
-import static java.util.Arrays.asList;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.springframework.util.ReflectionUtils.findMethod;
-
 import java.lang.reflect.Method;
-import java.util.Arrays;
 import java.util.List;
 import java.util.stream.Collectors;
 
 import org.jline.reader.ParsedLine;
 import org.jline.reader.impl.DefaultParser;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -39,6 +33,11 @@ import org.springframework.shell2.ParameterMissingResolutionException;
 import org.springframework.shell2.UnfinishedParameterResolutionException;
 import org.springframework.shell2.Utils;
 
+import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.springframework.util.ReflectionUtils.findMethod;
+
 /**
  * Unit tests for DefaultParameterResolver.
  * @author Eric Bottard
@@ -208,35 +207,32 @@ public class StandardParameterResolverTest {
 	}
 
 	@Test
-	@Ignore("--numbers 42 34 66 fails")
 	public void testValueCompletionWithNonDefaultArity() {
 		
-		resolver.setValueProviders(Arrays.asList(new Remote.NumberValueProvider()));
+		resolver.setValueProviders(singletonList(new Remote.NumberValueProvider("12", "42", "7")));
 		
-		Method method = findMethod(Remote.class, "add", List.class);
-		List<String> completions = resolver.complete(
-				Utils.createMethodParameter(method, 0),
-				contextFor("--numbers ")
-		).stream().map(CompletionProposal::value).collect(Collectors.toList());
-		assertThat(completions).contains("12");
+		Method[] methods = {
+			findMethod(org.springframework.shell2.standard.Remote.class, "add", List.class),
+			findMethod(org.springframework.shell2.standard.Remote.class, "addAsArray", int[].class),
+		};
+		for (Method method : methods) {
+			List<String> completions = resolver
+					.complete(Utils.createMethodParameter(method, 0), contextFor("--numbers ")).stream()
+					.map(CompletionProposal::value).collect(Collectors.toList());
+			assertThat(completions).contains("12", "42", "7");
 
-		completions = resolver.complete(
-				Utils.createMethodParameter(method, 0),
-				contextFor("--numbers 42 ")
-		).stream().map(CompletionProposal::value).collect(Collectors.toList());
-		assertThat(completions).contains("12");
+			completions = resolver.complete(Utils.createMethodParameter(method, 0), contextFor("--numbers 42 "))
+					.stream().map(CompletionProposal::value).collect(Collectors.toList());
+			assertThat(completions).contains("12", "7");
 
-		completions = resolver.complete(
-				Utils.createMethodParameter(method, 0),
-				contextFor("--numbers 42 34 ")
-		).stream().map(CompletionProposal::value).collect(Collectors.toList());
-		assertThat(completions).contains("12");
+			completions = resolver.complete(Utils.createMethodParameter(method, 0), contextFor("--numbers 42 34 "))
+					.stream().map(CompletionProposal::value).collect(Collectors.toList());
+			assertThat(completions).contains("12", "7");
 
-		completions = resolver.complete(
-				Utils.createMethodParameter(method, 0),
-				contextFor("--numbers 42 34 66 ")
-		).stream().map(CompletionProposal::value).collect(Collectors.toList());
-		assertThat(completions).isEmpty();
+			completions = resolver.complete(Utils.createMethodParameter(method, 0), contextFor("--numbers 42 34 66 "))
+					.stream().map(CompletionProposal::value).collect(Collectors.toList());
+			assertThat(completions).isEmpty(); // All 3 have already been set
+		}
 	}
 
 
diff --git a/spring-shell2-samples/src/main/java/org/springframework/shell2/samples/standard/Commands.java b/spring-shell2-samples/src/main/java/org/springframework/shell2/samples/standard/Commands.java
index b9c50570..ee596fd3 100644
--- a/spring-shell2-samples/src/main/java/org/springframework/shell2/samples/standard/Commands.java
+++ b/spring-shell2-samples/src/main/java/org/springframework/shell2/samples/standard/Commands.java
@@ -17,10 +17,11 @@
 package org.springframework.shell2.samples.standard;
 
 import java.lang.annotation.ElementType;
-import java.lang.annotation.Target;
+import java.util.Arrays;
 
 import org.springframework.shell2.standard.ShellComponent;
 import org.springframework.shell2.standard.ShellMethod;
+import org.springframework.shell2.standard.ShellOption;
 
 /**
  * Example commands for the Shell 2 Standard resolver.
@@ -54,4 +55,10 @@ public class Commands {
 	public void fail(ElementType elementType) {
 		throw new IllegalArgumentException("You said " + elementType);
 	}
+
+	@ShellMethod(help = "add array numbers")
+	public double addDoubles(@ShellOption(arity = 3) double[] numbers) {
+		return Arrays.stream(numbers).sum();
+	}
 }
+