Handle arity errors

- Introduce new error TooManyArgumentsOptionException
  and NotEnoughArgumentsOptionException.
- Parser not tracks arity min/max and imposes
  if num of option arguments.
- Backport #614
- Fixes #615
- Original commit this sources had some additional changes
  which is not possible to merge.
This commit is contained in:
Janne Valkealahti
2023-01-13 17:25:24 +00:00
parent 6e3d43c549
commit 4381c2423e
3 changed files with 178 additions and 5 deletions

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,7 +534,35 @@ public interface CommandParser {
}
}
static class MissingOptionException extends CommandParserException {
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;

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);

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() {
public CommandRegistration testArityBooleanDefaultTrueRegistration() {
return CommandRegistration.builder()
.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() {
return CommandRegistration.builder()
.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() {
return CommandRegistration.builder()
.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() {
return CommandRegistration.builder()
.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(","));
}
}