Spring/spring-shell
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Handle arity errors

Browse Source 
- Introduce new error TooManyArgumentsOptionException
  and NotEnoughArgumentsOptionException.
- Parser not tracks arity min/max and imposes
  if num of option arguments.
- CommandParserExceptionResolver contains better error
  message handling for these containing more context
  for a user.
- Fixes #614
This commit is contained in:
Janne Valkealahti
2023-01-12 11:26:20 +00:00
parent 6461a755d5
commit b10786814a
5 changed files with 277 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -332,6 +332,9 @@ public interface CommandParser {
return option.stream().flatMap(o -> {
List<String> subArgs = lr.subList(1, lr.size());
ConvertArgumentsHolder holder = convertArguments(o, subArgs);
if (holder.error != null) {
return Stream.of(ParserResult.of(o, subArgs, null, holder.error));
}
Object value = holder.value;
if (conversionService != null && o.getType() != null && value != null) {
if (conversionService.canConvert(value.getClass(), o.getType().getRawClass())) {
@@ -433,6 +436,22 @@ public interface CommandParser {
}
}
if (arityMax > 1 && arityMin > -1 && arityMax >= arityMin && (arguments.size() < arityMin || arguments.size() > arityMax)) {
String ln = option.getLongNames() != null
? Stream.of(option.getLongNames()).collect(Collectors.joining(","))
: "";
String sn = option.getShortNames() != null ? Stream.of(option.getShortNames())
.map(n -> Character.toString(n)).collect(Collectors.joining(",")) : "";
if (arguments.size() < arityMin) {
String msg = String.format("Not enough arguments, longnames='%s', shortnames='%s'", ln, sn);
return new ConvertArgumentsHolder(value, unmapped, new NotEnoughArgumentsOptionException(msg, option));
}
if (arguments.size() > arityMax) {
String msg = String.format("Too many arguments, longnames='%s', shortnames='%s'", ln, sn);
return new ConvertArgumentsHolder(value, unmapped, new TooManyArgumentsOptionException(msg, option));
}
}
if (type != null && type.isAssignableFrom(boolean.class)) {
if (arguments.size() == 0) {
value = true;
@@ -469,12 +488,18 @@ public interface CommandParser {
private class ConvertArgumentsHolder {
Object value;
final List<String> unmapped = new ArrayList<>();
CommandParserException error;
ConvertArgumentsHolder(Object value, List<String> unmapped) {
this(value, unmapped, null);
}
ConvertArgumentsHolder(Object value, List<String> unmapped, CommandParserException error) {
this.value = value;
if (unmapped != null) {
this.unmapped.addAll(unmapped);
}
this.error = error;
}
}
}
@@ -509,6 +534,34 @@ public interface CommandParser {
}
}
public static class OptionException extends CommandParserException {
private CommandOption option;
public OptionException(String message, CommandOption option) {
super(message);
this.option = option;
}
public CommandOption getOption() {
return option;
}
}
public static class TooManyArgumentsOptionException extends OptionException {
public TooManyArgumentsOptionException(String message, CommandOption option) {
super(message, option);
}
}
public static class NotEnoughArgumentsOptionException extends OptionException {
public NotEnoughArgumentsOptionException(String message, CommandOption option) {
super(message, option);
}
}
public static class MissingOptionException extends CommandParserException {
private CommandOption option;

40
spring-shell-core/src/main/java/org/springframework/shell/command/CommandParserExceptionResolver.java
View File

@@ -21,6 +21,8 @@ import org.jline.utils.AttributedStyle;
import org.springframework.shell.command.CommandExecution.CommandParserExceptionsException;
import org.springframework.shell.command.CommandParser.MissingOptionException;
import org.springframework.shell.command.CommandParser.NotEnoughArgumentsOptionException;
import org.springframework.shell.command.CommandParser.TooManyArgumentsOptionException;
import org.springframework.util.StringUtils;
/**
@@ -44,6 +46,18 @@ public class CommandParserExceptionResolver implements CommandExceptionResolver
handleShort(builder, option);
}
}
else if (e instanceof NotEnoughArgumentsOptionException neaoe) {
CommandOption option = neaoe.getOption();
if (option.getLongNames().length > 0) {
handleLongNotEnough(builder, option);
}
}
else if (e instanceof TooManyArgumentsOptionException tmaoe) {
CommandOption option = tmaoe.getOption();
if (option.getLongNames().length > 0) {
handleLongTooMany(builder, option);
}
}
else {
builder.append(new AttributedString(e.getMessage(), AttributedStyle.DEFAULT.foreground(AttributedStyle.RED)));
}
@@ -78,4 +92,30 @@ public class CommandParserExceptionResolver implements CommandExceptionResolver
buf.append(".");
builder.append(new AttributedString(buf.toString(), AttributedStyle.DEFAULT.foreground(AttributedStyle.RED)));
}
private static void handleLongNotEnough(AttributedStringBuilder builder, CommandOption option) {
StringBuilder buf = new StringBuilder();
buf.append("Not enough arguments --");
buf.append(option.getLongNames()[0]);
buf.append(" requires at least " + option.getArityMin());
if (StringUtils.hasText(option.getDescription())) {
buf.append(", ");
buf.append(option.getDescription());
}
buf.append(".");
builder.append(new AttributedString(buf.toString(), AttributedStyle.DEFAULT.foreground(AttributedStyle.RED)));
}
private static void handleLongTooMany(AttributedStringBuilder builder, CommandOption option) {
StringBuilder buf = new StringBuilder();
buf.append("Too many arguments --");
buf.append(option.getLongNames()[0]);
buf.append(" requires at most " + option.getArityMax());
if (StringUtils.hasText(option.getDescription())) {
buf.append(", ");
buf.append(option.getDescription());
}
buf.append(".");
builder.append(new AttributedString(buf.toString(), AttributedStyle.DEFAULT.foreground(AttributedStyle.RED)));
}
}

66
spring-shell-core/src/test/java/org/springframework/shell/command/CommandParserExceptionResolverTests.java
View File

@@ -23,6 +23,8 @@ import org.junit.jupiter.api.Test;
import org.springframework.shell.command.CommandExecution.CommandParserExceptionsException;
import org.springframework.shell.command.CommandParser.CommandParserException;
import org.springframework.shell.command.CommandParser.MissingOptionException;
import org.springframework.shell.command.CommandParser.NotEnoughArgumentsOptionException;
import org.springframework.shell.command.CommandParser.TooManyArgumentsOptionException;
import static org.assertj.core.api.Assertions.assertThat;
@@ -44,7 +46,7 @@ class CommandParserExceptionResolverTests {
.and()
.build();
CommandHandlingResult resolve = resolver.resolve(of(registration.getOptions().get(0)));
CommandHandlingResult resolve = resolver.resolve(missingOption(registration.getOptions().get(0)));
assertThat(resolve).isNotNull();
assertThat(resolve.message()).contains("--arg1", "Desc arg1");
}
@@ -64,7 +66,7 @@ class CommandParserExceptionResolverTests {
.and()
.build();
CommandHandlingResult resolve = resolver.resolve(of(registration.getOptions().get(0)));
CommandHandlingResult resolve = resolver.resolve(missingOption(registration.getOptions().get(0)));
assertThat(resolve).isNotNull();
assertThat(resolve.message()).contains("--arg1", "Desc arg1");
assertThat(resolve.message()).doesNotContain("-x", "Desc x");
@@ -84,14 +86,66 @@ class CommandParserExceptionResolverTests {
.and()
.build();
CommandHandlingResult resolve = resolver.resolve(of(registration.getOptions().get(0)));
CommandHandlingResult resolve = resolver.resolve(missingOption(registration.getOptions().get(0)));
assertThat(resolve).isNotNull();
assertThat(resolve.message()).contains("-x", "Desc x");
}
static CommandParserExceptionsException of(CommandOption option) {
MissingOptionException moe = new MissingOptionException("msg", option);
List<CommandParserException> parserExceptions = Arrays.asList(moe);
@Test
void resolvesTooManyLongOption() {
CommandRegistration registration = CommandRegistration.builder()
.command("required-value")
.withOption()
.longNames("arg1")
.description("Desc arg1")
.arity(2, 3)
.required()
.and()
.withTarget()
.consumer(ctx -> {})
.and()
.build();
CommandHandlingResult resolve = resolver.resolve(tooManyArguments(registration.getOptions().get(0)));
assertThat(resolve).isNotNull();
assertThat(resolve.message()).contains("--arg1 requires at most", "Desc arg1");
}
@Test
void resolvesNotEnoughLongOption() {
CommandRegistration registration = CommandRegistration.builder()
.command("required-value")
.withOption()
.longNames("arg1")
.description("Desc arg1")
.arity(2, 3)
.required()
.and()
.withTarget()
.consumer(ctx -> {})
.and()
.build();
CommandHandlingResult resolve = resolver.resolve(notEnoughArguments(registration.getOptions().get(0)));
assertThat(resolve).isNotNull();
assertThat(resolve.message()).contains("--arg1 requires at least", "Desc arg1");
}
static CommandParserExceptionsException missingOption(CommandOption option) {
MissingOptionException e = new MissingOptionException("msg", option);
List<CommandParserException> parserExceptions = Arrays.asList(e);
return new CommandParserExceptionsException("msg", parserExceptions);
}
static CommandParserExceptionsException tooManyArguments(CommandOption option) {
TooManyArgumentsOptionException e = new TooManyArgumentsOptionException("msg", option);
List<CommandParserException> parserExceptions = Arrays.asList(e);
return new CommandParserExceptionsException("msg", parserExceptions);
}
static CommandParserExceptionsException notEnoughArguments(CommandOption option) {
NotEnoughArgumentsOptionException e = new NotEnoughArgumentsOptionException("msg", option);
List<CommandParserException> parserExceptions = Arrays.asList(e);
return new CommandParserExceptionsException("msg", parserExceptions);
}
}

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

@@ -26,6 +26,8 @@ import org.springframework.core.ResolvableType;
import org.springframework.core.convert.ConversionService;
import org.springframework.core.convert.support.DefaultConversionService;
import org.springframework.shell.command.CommandParser.CommandParserResults;
import org.springframework.shell.command.CommandParser.NotEnoughArgumentsOptionException;
import org.springframework.shell.command.CommandParser.TooManyArgumentsOptionException;
import static org.assertj.core.api.Assertions.assertThat;
@@ -295,6 +297,40 @@ public class CommandParserTests extends AbstractCommandTests {
assertThat(results.results().get(0).value()).isEqualTo(new int[] { 1, 2 });
}
@Test
public void testArityErrors() {
CommandOption option1 = CommandOption.of(
new String[] { "arg1" },
null,
null,
ResolvableType.forType(int[].class),
true,
null,
null,
2,
3,
null,
null);
List<CommandOption> options = Arrays.asList(option1);
String[] args1 = new String[]{"--arg1", "1", "2", "3", "4"};
CommandParserResults results1 = parser.parse(options, args1);
assertThat(results1.errors()).hasSize(1);
assertThat(results1.errors().get(0)).isInstanceOf(TooManyArgumentsOptionException.class);
assertThat(results1.results()).hasSize(1);
assertThat(results1.results().get(0).option()).isSameAs(option1);
assertThat(results1.results().get(0).value()).isNull();
String[] args2 = new String[]{"--arg1", "1"};
CommandParserResults results2 = parser.parse(options, args2);
assertThat(results2.errors()).hasSize(1);
assertThat(results2.errors().get(0)).isInstanceOf(NotEnoughArgumentsOptionException.class);
assertThat(results2.results()).hasSize(1);
assertThat(results2.results().get(0).option()).isSameAs(option1);
assertThat(results2.results().get(0).value()).isNull();
}
@Test
public void testMapPositionalArgs1() {
CommandOption option1 = longOption("arg1", 0, 1, 1);

92
spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/ArityCommands.java
View File

@@ -15,6 +15,10 @@
*/
package org.springframework.shell.samples.e2e;
import java.util.Arrays;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import org.springframework.context.annotation.Bean;
import org.springframework.shell.command.CommandRegistration;
import org.springframework.shell.command.CommandRegistration.OptionArity;
@@ -30,17 +34,17 @@ import org.springframework.shell.standard.ShellOption;
@ShellComponent
public class ArityCommands extends BaseE2ECommands {
@ShellMethod(key = LEGACY_ANNO + "boolean-arity1-default-true", group = GROUP)
public String testBooleanArity1DefaultTrue(
@ShellMethod(key = LEGACY_ANNO + "arity-boolean-default-true", group = GROUP)
public String testArityBooleanDefaultTrueLegacyAnnotation(
@ShellOption(value = "--overwrite", arity = 1, defaultValue = "true") Boolean overwrite
) {
return "Hello " + overwrite;
}
@Bean
public CommandRegistration testBooleanArity1DefaultTrueRegistration(CommandRegistration.BuilderSupplier builder) {
public CommandRegistration testArityBooleanDefaultTrueRegistration(CommandRegistration.BuilderSupplier builder) {
return builder.get()
.command(REG, "boolean-arity1-default-true")
.command(REG, "arity-boolean-default-true")
.group(GROUP)
.withOption()
.longNames("overwrite")
@@ -56,4 +60,84 @@ public class ArityCommands extends BaseE2ECommands {
.and()
.build();
}
@ShellMethod(key = LEGACY_ANNO + "arity-string-array", group = GROUP)
public String testArityStringArrayLegacyAnnotation(
@ShellOption(value = "--arg1", arity = 3) String[] arg1
) {
return "Hello " + Arrays.asList(arg1);
}
@Bean
public CommandRegistration testArityStringArrayRegistration(CommandRegistration.BuilderSupplier builder) {
return builder.get()
.command(REG, "arity-string-array")
.group(GROUP)
.withOption()
.longNames("arg1")
.type(String[].class)
.arity(0, 3)
.and()
.withTarget()
.function(ctx -> {
String[] arg1 = ctx.getOptionValue("arg1");
return "Hello " + Arrays.asList(arg1);
})
.and()
.build();
}
@ShellMethod(key = LEGACY_ANNO + "arity-float-array", group = GROUP)
public String testArityFloatArrayLegacyAnnotation(
@ShellOption(value = "--arg1", arity = 3) float[] arg1
) {
return "Hello " + floatsToString(arg1);
}
@Bean
public CommandRegistration testArityFloatArrayRegistration(CommandRegistration.BuilderSupplier builder) {
return builder.get()
.command(REG, "arity-float-array")
.group(GROUP)
.withOption()
.longNames("arg1")
.type(float[].class)
.arity(0, 3)
.and()
.withTarget()
.function(ctx -> {
float[] arg1 = ctx.getOptionValue("arg1");
return "Hello " + floatsToString(arg1);
})
.and()
.build();
}
@Bean
public CommandRegistration testArityErrorsRegistration(CommandRegistration.BuilderSupplier builder) {
return builder.get()
.command(REG, "arity-errors")
.group(GROUP)
.withOption()
.longNames("arg1")
.type(String[].class)
.required()
.arity(1, 2)
.and()
.withTarget()
.function(ctx -> {
String[] arg1 = ctx.getOptionValue("arg1");
return "Hello " + Arrays.asList(arg1);
})
.and()
.build();
}
private static String floatsToString(float[] arg1) {
return IntStream.range(0, arg1.length)
.mapToDouble(i -> arg1[i])
.boxed()
.map(d -> d.toString())
.collect(Collectors.joining(","));
}
}