diff --git a/spring-shell-samples/src/main/java/org/springframework/shell/samples/legacy/LegacyCommands.java b/spring-shell-samples/src/main/java/org/springframework/shell/samples/legacy/LegacyCommands.java index cf2925bf..6df68438 100644 --- a/spring-shell-samples/src/main/java/org/springframework/shell/samples/legacy/LegacyCommands.java +++ b/spring-shell-samples/src/main/java/org/springframework/shell/samples/legacy/LegacyCommands.java @@ -16,6 +16,7 @@ package org.springframework.shell.samples.legacy; +import java.io.File; import java.lang.reflect.Method; import org.springframework.shell.core.CommandMarker; @@ -91,4 +92,8 @@ public class LegacyCommands implements CommandMarker { return message; } + @CliCommand(value = "legacy-file", help = "Uses a legacy converter for completion") + public String legacyFile(@CliOption(key = "file") File file, @CliOption(key = "flag") boolean flag) { + return file + " exists? : " + file.exists(); + } } diff --git a/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyAdapterAutoConfiguration.java b/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyAdapterAutoConfiguration.java index 849ea4e4..e38b31c1 100644 --- a/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyAdapterAutoConfiguration.java +++ b/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyAdapterAutoConfiguration.java @@ -22,9 +22,12 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.FilterType; import org.springframework.shell.converters.ArrayConverter; import org.springframework.shell.converters.AvailableCommandsConverter; +import org.springframework.shell.converters.FileConverter; import org.springframework.shell.converters.SimpleFileConverter; import org.springframework.shell.core.annotation.CliCommand; +import java.io.File; + /** * Main configuration class for the Shell 2 - Shell 1 adapter. * @@ -47,4 +50,13 @@ public class LegacyAdapterAutoConfiguration { return new LegacyParameterResolver(); } + @Bean + public FileConverter fileConverter() { + return new FileConverter() { + @Override + protected File getWorkingDirectory() { + return new File("."); + } + }; + } } diff --git a/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyParameterResolver.java b/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyParameterResolver.java index 34c3e145..6a4ddbb0 100644 --- a/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyParameterResolver.java +++ b/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyParameterResolver.java @@ -17,21 +17,16 @@ package org.springframework.shell.legacy; import java.lang.reflect.Parameter; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.BitSet; -import java.util.Collection; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; +import java.util.*; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.MethodParameter; +import org.springframework.shell.core.Completion; import org.springframework.shell.core.Converter; +import org.springframework.shell.core.MethodTarget; import org.springframework.shell.core.annotation.CliOption; import org.springframework.shell.CompletionContext; import org.springframework.shell.CompletionProposal; @@ -40,9 +35,11 @@ import org.springframework.shell.ParameterResolver; import org.springframework.shell.ValueResult; import org.springframework.stereotype.Component; import org.springframework.util.Assert; +import org.springframework.util.ReflectionUtils; /** - * Resolves parameters by looking at the {@link CliOption} annotation and acting accordingly. + * Resolves parameters by looking at the {@link CliOption} annotation and acting + * accordingly. * * @author Eric Bottard * @author Camilo Gonzalez @@ -51,12 +48,13 @@ import org.springframework.util.Assert; public class LegacyParameterResolver implements ParameterResolver { private static final String CLI_OPTION_NULL = "__NULL__"; - + /** - * Prefix used by Spring Shell 1 for the argument keys (e.g. command --key value). + * Prefix used by Spring Shell 1 for the argument keys (e.g. command --key + * value). */ private static final String CLI_PREFIX = "--"; - + @Autowired(required = false) private Collection> converters = new ArrayList<>(); @@ -67,33 +65,42 @@ public class LegacyParameterResolver implements ParameterResolver { @Override public ValueResult resolve(MethodParameter methodParameter, List words) { + Optional> converter = findOptionalConverter(methodParameter); CliOption cliOption = methodParameter.getParameterAnnotation(CliOption.class); - Optional> converter = converters.stream() - .filter(c -> c.supports(methodParameter.getParameterType(), cliOption.optionContext())) - .findFirst(); Map values = parseOptions(words); Map seenValues = convertValues(values, methodParameter, converter); switch (seenValues.size()) { - case 0: - if (!cliOption.mandatory()) { - String value = cliOption.unspecifiedDefaultValue(); - Object resolvedValue = converter - .orElseThrow(noConverterFound(cliOption.key()[0], value, methodParameter.getParameterType())) - .convertFromText(value, methodParameter.getParameterType(), cliOption.optionContext()); - - return new ValueResult(methodParameter, resolvedValue); - } - else { - throw new IllegalArgumentException("Could not find parameter values for " + prettifyKeys(Arrays.asList(cliOption.key())) + " in " + words); - } - case 1: - return seenValues.values().iterator().next(); - default: - throw new RuntimeException("Option has been set multiple times via " + prettifyKeys(seenValues.keySet())); + case 0: + if (!cliOption.mandatory()) { + String value = cliOption.unspecifiedDefaultValue(); + Object resolvedValue = converter + .orElseThrow(noConverterFound(cliOption.key()[0], value, methodParameter.getParameterType())) + .convertFromText(value, methodParameter.getParameterType(), cliOption.optionContext()); + + return new ValueResult(methodParameter, resolvedValue); + } + else { + throw new IllegalArgumentException("Could not find parameter values for " + + prettifyKeys(Arrays.asList(cliOption.key())) + " in " + words); + } + case 1: + return seenValues.values().iterator().next(); + default: + throw new RuntimeException("Option has been set multiple times via " + prettifyKeys(seenValues.keySet())); } } + /** + * Maybe find a Shell 1 Converter that applies to the given {@literal methodParameter}. + */ + private Optional> findOptionalConverter(MethodParameter methodParameter) { + CliOption cliOption = methodParameter.getParameterAnnotation(CliOption.class); + return converters.stream() + .filter(c -> c.supports(methodParameter.getParameterType(), cliOption.optionContext())) + .findFirst(); + } + @Override public Stream describe(MethodParameter parameter) { Parameter jlrParameter = parameter.getMethod().getParameters()[parameter.getParameterIndex()]; @@ -101,14 +108,12 @@ public class LegacyParameterResolver implements ParameterResolver { ParameterDescription result = ParameterDescription.outOf(parameter); result.help(option.help()); List keys = Arrays.asList(option.key()); - result.keys(keys.stream() - .filter(key -> !key.isEmpty()) - .map(key -> CLI_PREFIX + key) - .collect(Collectors.toList())); + result.keys(notDefaultCommandKeys(parameter)); if (!option.mandatory()) { - result.defaultValue(CLI_OPTION_NULL.equals(option.unspecifiedDefaultValue()) ? "null" : option.unspecifiedDefaultValue()); + result.defaultValue(CLI_OPTION_NULL.equals(option.unspecifiedDefaultValue()) ? "null" + : option.unspecifiedDefaultValue()); } - if(!CLI_OPTION_NULL.equals(option.specifiedDefaultValue())) { + if (!CLI_OPTION_NULL.equals(option.specifiedDefaultValue())) { result.whenFlag(option.specifiedDefaultValue()); } boolean containsEmptyKey = keys.contains(""); @@ -116,9 +121,82 @@ public class LegacyParameterResolver implements ParameterResolver { return Stream.of(result); } + /** + * Return the list of keys (with their "--" prefix) that can be used to set the given + * parameter. If the parameter supports the empty key, this is not part of the result. + */ + private List notDefaultCommandKeys(MethodParameter parameter) { + Parameter jlrParameter = parameter.getMethod().getParameters()[parameter.getParameterIndex()]; + CliOption option = jlrParameter.getAnnotation(CliOption.class); + return Arrays.stream(option.key()) + .filter(key -> !key.isEmpty()) + .map(key -> CLI_PREFIX + key) + .collect(Collectors.toList()); + } + @Override public List complete(MethodParameter parameter, CompletionContext context) { - return null; + String nextToLast = null; + String last; + if (context.getWords().size() >= 2) { + nextToLast = context.getWords().get(context.getWords().size() - 2); + } + if (context.getWords().size() >= 1) { + last = context.getWords().get(context.getWords().size() - 1); + } + else { + last = null; + } + List commandKeys = notDefaultCommandKeys(parameter); + if (nextToLast != null) { + if (commandKeys.contains(nextToLast)) { + // nextToLast is our key, last is our (possibly unfinished) value + if (findOptionalConverter(parameter).isPresent()) { + ArrayList legacyProposals = new ArrayList<>(); + findOptionalConverter(parameter).get().getAllPossibleValues( + legacyProposals, + parameter.getParameterType(), + last, + parameter.getParameterAnnotation(CliOption.class).optionContext(), + craftMethodTarget() + ); + return legacyProposals.stream() + .filter(lp -> lp.getValue().startsWith(last)) + .map(this::toCompletionProposal) + .collect(Collectors.toList()); + } else { + return Collections.emptyList(); + } + } // nextToLast looks like a key, but not for this parameter + else if (nextToLast.startsWith(CLI_PREFIX)) { + // Not for this parameter + return Collections.emptyList(); + } + } + // Fallthrough: nextToLast is the value to another parameter + // and last (possibly the empty string) could be our key + if (last != null) { + return commandKeys.stream() + .filter(k -> k.startsWith(last)) + .map(CompletionProposal::new) + .collect(Collectors.toList()); + } + // Invoked completion just after the command (without a space): my-command + return Collections.emptyList(); + } + + /** + * Turn a Shell 1 Completion into a CompletionProposal. + */ + private CompletionProposal toCompletionProposal(Completion c) { + return new CompletionProposal(c.getValue()) + .displayText(c.getFormattedValue()) + .category(c.getHeading()); + } + + // TODO pass invokable method in the completion context. Rarely used by converters, so ok for now + private MethodTarget craftMethodTarget() { + return new MethodTarget(ReflectionUtils.findMethod(Object.class, "toString"), "foo"); } private Map parseOptions(List words) { @@ -130,7 +208,8 @@ public class LegacyParameterResolver implements ParameterResolver { String key = word.substring(CLI_PREFIX.length()); // If next word doesn't exist or starts with '--', this is an unary option. Store null String value = i < words.size() - 1 && !words.get(i + 1).startsWith(CLI_PREFIX) ? words.get(++i) : null; - Assert.isTrue(!values.containsKey(key), String.format("Option %s%s has already been set", CLI_PREFIX, key)); + Assert.isTrue(!values.containsKey(key), + String.format("Option %s%s has already been set", CLI_PREFIX, key)); values.put(key, new ParseResult(value, from)); } // Must be the 'anonymous' option else { @@ -141,7 +220,8 @@ public class LegacyParameterResolver implements ParameterResolver { return values; } - private Map convertValues(Map values, MethodParameter methodParameter, Optional> converter) { + private Map convertValues(Map values, MethodParameter methodParameter, + Optional> converter) { Map seenValues = new HashMap<>(); CliOption option = methodParameter.getParameterAnnotation(CliOption.class); for (String key : option.key()) { @@ -170,26 +250,29 @@ public class LegacyParameterResolver implements ParameterResolver { } /** - * Return the list of possible keys for an option, suitable for displaying in an error message. + * Return the list of possible keys for an option, suitable for displaying in an error + * message. */ private String prettifyKeys(Collection keys) { - return keys.stream().map(s -> "".equals(s) ? "" : CLI_PREFIX + s).collect(Collectors.joining(", ", "[", "]")); + return keys.stream().map(s -> "".equals(s) ? "" : CLI_PREFIX + s) + .collect(Collectors.joining(", ", "[", "]")); } private Supplier noConverterFound(String key, String value, Class parameterType) { - return () -> new IllegalStateException("No converter found for " + CLI_PREFIX + key + " from '" + value + "' to type " + parameterType); + return () -> new IllegalStateException( + "No converter found for " + CLI_PREFIX + key + " from '" + value + "' to type " + parameterType); } - + private static class ParseResult { private final String value; - + private final Integer from; public ParseResult(String value, Integer from) { this.value = value; this.from = from; } - + } - + } diff --git a/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyCommands.java b/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyCommands.java index e8dea14f..c04cb276 100644 --- a/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyCommands.java +++ b/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyCommands.java @@ -47,6 +47,7 @@ public class LegacyCommands implements CommandMarker { ArtifactType type, @CliOption(mandatory = true, key = {"coordinates", "coords"}, + optionContext = "disable-string-converter", help = "coordinates to the module archive") String coordinates, @CliOption(key = "force", diff --git a/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyParameterResolverTest.java b/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyParameterResolverTest.java index cb63aea0..122e3dee 100644 --- a/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyParameterResolverTest.java +++ b/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyParameterResolverTest.java @@ -29,18 +29,20 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.MethodParameter; +import org.springframework.shell.*; 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.shell.ParameterDescription; -import org.springframework.shell.ParameterResolver; -import org.springframework.shell.Utils; -import org.springframework.shell.ValueResult; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + /** * Tests for {@link LegacyParameterResolver}. * @@ -259,6 +261,74 @@ public class LegacyParameterResolverTest { return result; } + // ======================== Completion Tests ========================== + + @Test + public void testNoCompletionJustAfterCommandWithNoSpace() { + MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, NAME_OR_ANONYMOUS); + + List proposals = parameterResolver.complete(methodParameter, new CompletionContext(Collections.emptyList(), -1, 0)); + assertThat(proposals).isEmpty(); + } + + @Test + public void testAllCommandKeysJustAfterCommand() { + MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, COORDINATES); + + List proposals = parameterResolver.complete(methodParameter, new CompletionContext(Collections.singletonList(""), 0, 0)); + assertThat(valuesOf(proposals)).contains("--coords", "--coordinates"); + } + + @Test + public void testAllCommandKeysWhenStarted() { + MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, COORDINATES); + + List proposals = parameterResolver.complete(methodParameter, new CompletionContext(Collections.singletonList("--co"), 0, 4)); + assertThat(valuesOf(proposals)).contains("--coords", "--coordinates"); + + proposals = parameterResolver.complete(methodParameter, new CompletionContext(Arrays.asList("--name", "foo", "--co"), 2, 4)); + assertThat(valuesOf(proposals)).contains("--coords", "--coordinates"); + } + + @Test + public void testNoCompletionsWhenAnotherParameterDetected() { + MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, COORDINATES); + + List proposals = parameterResolver.complete(methodParameter, new CompletionContext(Arrays.asList("--name", ""), 1, 0)); + assertThat(valuesOf(proposals)).isEmpty(); + + proposals = parameterResolver.complete(methodParameter, new CompletionContext(Arrays.asList("--name", "foo"), 1, 3)); + assertThat(valuesOf(proposals)).isEmpty(); + } + + @Test + public void testValueCompletionsWhenConverterAvailable() { + MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, FORCE); + + List proposals = parameterResolver.complete(methodParameter, new CompletionContext(Arrays.asList("--force", ""), 1, 0)); + assertThat(valuesOf(proposals)).contains("true", "false"); + + proposals = parameterResolver.complete(methodParameter, new CompletionContext(Arrays.asList("--force", "fa"), 1, 2)); + assertThat(valuesOf(proposals)).contains("false").doesNotContain("true"); + + } + + @Test + public void testNoValueCompletionsWhenNoConverterAvailable() { + MethodParameter methodParameter = Utils.createMethodParameter(REGISTER_METHOD, COORDINATES); + + List proposals = parameterResolver.complete(methodParameter, new CompletionContext(Arrays.asList("--coords", ""), 1, 0)); + assertThat(valuesOf(proposals)).isEmpty(); + + proposals = parameterResolver.complete(methodParameter, new CompletionContext(Arrays.asList("--coords", "foo"), 1, 3)); + assertThat(valuesOf(proposals)).isEmpty(); + + } + + private List valuesOf(List proposals) { + return proposals.stream().map(CompletionProposal::value).collect(Collectors.toList()); + } + @Configuration static class Config {