Skip to content

Commit 4381c24

Browse files
jvalkealjvalkeal
committed
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.
1 parent 6e3d43c commit 4381c24

File tree

3 files changed

+178
-5
lines changed

3 files changed

+178
-5
lines changed

‎spring-shell-core/src/main/java/org/springframework/shell/command/CommandParser.java

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,9 @@ ParserResults visit(List<List<String>> lexerResults, List<CommandOption> options
332332
return option.stream().flatMap(o -> {
333333
List<String> subArgs = lr.subList(1, lr.size());
334334
ConvertArgumentsHolder holder = convertArguments(o, subArgs);
335+
if (holder.error != null) {
336+
return Stream.of(ParserResult.of(o, subArgs, null, holder.error));
337+
}
335338
Object value = holder.value;
336339
if (conversionService != null && o.getType() != null && value != null) {
337340
if (conversionService.canConvert(value.getClass(), o.getType().getRawClass())) {
@@ -433,6 +436,22 @@ private ConvertArgumentsHolder convertArguments(CommandOption option, List<Strin
433436
}
434437
}
435438

439+
if (arityMax > 1 && arityMin > -1 && arityMax >= arityMin && (arguments.size() < arityMin || arguments.size() > arityMax)) {
440+
String ln = option.getLongNames() != null
441+
? Stream.of(option.getLongNames()).collect(Collectors.joining(","))
442+
: "";
443+
String sn = option.getShortNames() != null ? Stream.of(option.getShortNames())
444+
.map(n -> Character.toString(n)).collect(Collectors.joining(",")) : "";
445+
if (arguments.size() < arityMin) {
446+
String msg = String.format("Not enough arguments, longnames='%s', shortnames='%s'", ln, sn);
447+
return new ConvertArgumentsHolder(value, unmapped, new NotEnoughArgumentsOptionException(msg, option));
448+
}
449+
if (arguments.size() > arityMax) {
450+
String msg = String.format("Too many arguments, longnames='%s', shortnames='%s'", ln, sn);
451+
return new ConvertArgumentsHolder(value, unmapped, new TooManyArgumentsOptionException(msg, option));
452+
}
453+
}
454+
436455
if (type != null && type.isAssignableFrom(boolean.class)) {
437456
if (arguments.size() == 0) {
438457
value = true;
@@ -469,12 +488,18 @@ else if (type != null && type.isArray()) {
469488
private class ConvertArgumentsHolder {
470489
Object value;
471490
final List<String> unmapped = new ArrayList<>();
491+
CommandParserException error;
472492

473493
ConvertArgumentsHolder(Object value, List<String> unmapped) {
494+
this(value, unmapped, null);
495+
}
496+
497+
ConvertArgumentsHolder(Object value, List<String> unmapped, CommandParserException error) {
474498
this.value = value;
475499
if (unmapped != null) {
476500
this.unmapped.addAll(unmapped);
477501
}
502+
this.error = error;
478503
}
479504
}
480505
}
@@ -509,7 +534,35 @@ public static CommandParserException of(String message) {
509534
}
510535
}
511536

512-
static class MissingOptionException extends CommandParserException {
537+
public static class OptionException extends CommandParserException {
538+
539+
private CommandOption option;
540+
541+
public OptionException(String message, CommandOption option) {
542+
super(message);
543+
this.option = option;
544+
}
545+
546+
public CommandOption getOption() {
547+
return option;
548+
}
549+
}
550+
551+
public static class TooManyArgumentsOptionException extends OptionException {
552+
553+
public TooManyArgumentsOptionException(String message, CommandOption option) {
554+
super(message, option);
555+
}
556+
}
557+
558+
public static class NotEnoughArgumentsOptionException extends OptionException {
559+
560+
public NotEnoughArgumentsOptionException(String message, CommandOption option) {
561+
super(message, option);
562+
}
563+
}
564+
565+
public static class MissingOptionException extends CommandParserException {
513566

514567
private CommandOption option;
515568

‎spring-shell-core/src/test/java/org/springframework/shell/command/CommandParserTests.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import org.springframework.core.convert.ConversionService;
2727
import org.springframework.core.convert.support.DefaultConversionService;
2828
import org.springframework.shell.command.CommandParser.CommandParserResults;
29+
import org.springframework.shell.command.CommandParser.NotEnoughArgumentsOptionException;
30+
import org.springframework.shell.command.CommandParser.TooManyArgumentsOptionException;
2931

3032
import static org.assertj.core.api.Assertions.assertThat;
3133

@@ -295,6 +297,40 @@ public void testLongOptionsWithArray() {
295297
assertThat(results.results().get(0).value()).isEqualTo(new int[] { 1, 2 });
296298
}
297299

300+
@Test
301+
public void testArityErrors() {
302+
CommandOption option1 = CommandOption.of(
303+
new String[] { "arg1" },
304+
null,
305+
null,
306+
ResolvableType.forType(int[].class),
307+
true,
308+
null,
309+
null,
310+
2,
311+
3,
312+
null,
313+
null);
314+
315+
List<CommandOption> options = Arrays.asList(option1);
316+
317+
String[] args1 = new String[]{"--arg1", "1", "2", "3", "4"};
318+
CommandParserResults results1 = parser.parse(options, args1);
319+
assertThat(results1.errors()).hasSize(1);
320+
assertThat(results1.errors().get(0)).isInstanceOf(TooManyArgumentsOptionException.class);
321+
assertThat(results1.results()).hasSize(1);
322+
assertThat(results1.results().get(0).option()).isSameAs(option1);
323+
assertThat(results1.results().get(0).value()).isNull();
324+
325+
String[] args2 = new String[]{"--arg1", "1"};
326+
CommandParserResults results2 = parser.parse(options, args2);
327+
assertThat(results2.errors()).hasSize(1);
328+
assertThat(results2.errors().get(0)).isInstanceOf(NotEnoughArgumentsOptionException.class);
329+
assertThat(results2.results()).hasSize(1);
330+
assertThat(results2.results().get(0).option()).isSameAs(option1);
331+
assertThat(results2.results().get(0).value()).isNull();
332+
}
333+
298334
@Test
299335
public void testMapPositionalArgs1() {
300336
CommandOption option1 = longOption("arg1", 0, 1, 1);

‎spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/ArityCommands.java

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
*/
1616
package org.springframework.shell.samples.e2e;
1717

18+
import java.util.Arrays;
19+
import java.util.stream.Collectors;
20+
import java.util.stream.IntStream;
21+
1822
import org.springframework.context.annotation.Bean;
1923
import org.springframework.shell.command.CommandRegistration;
2024
import org.springframework.shell.command.CommandRegistration.OptionArity;
@@ -30,17 +34,17 @@
3034
@ShellComponent
3135
public class ArityCommands extends BaseE2ECommands {
3236

33-
@ShellMethod(key = LEGACY_ANNO + "boolean-arity1-default-true", group = GROUP)
34-
public String testBooleanArity1DefaultTrue(
37+
@ShellMethod(key = LEGACY_ANNO + "arity-boolean-default-true", group = GROUP)
38+
public String testArityBooleanDefaultTrueLegacyAnnotation(
3539
@ShellOption(value = "--overwrite", arity = 1, defaultValue = "true") Boolean overwrite
3640
) {
3741
return "Hello " + overwrite;
3842
}
3943

4044
@Bean
41-
public CommandRegistration testBooleanArity1DefaultTrueRegistration() {
45+
public CommandRegistration testArityBooleanDefaultTrueRegistration() {
4246
return CommandRegistration.builder()
43-
.command(REG, "boolean-arity1-default-true")
47+
.command(REG, "arity-boolean-default-true")
4448
.group(GROUP)
4549
.withOption()
4650
.longNames("overwrite")
@@ -56,4 +60,84 @@ public CommandRegistration testBooleanArity1DefaultTrueRegistration() {
5660
.and()
5761
.build();
5862
}
63+
64+
@ShellMethod(key = LEGACY_ANNO + "arity-string-array", group = GROUP)
65+
public String testArityStringArrayLegacyAnnotation(
66+
@ShellOption(value = "--arg1", arity = 3) String[] arg1
67+
) {
68+
return "Hello " + Arrays.asList(arg1);
69+
}
70+
71+
@Bean
72+
public CommandRegistration testArityStringArrayRegistration() {
73+
return CommandRegistration.builder()
74+
.command(REG, "arity-string-array")
75+
.group(GROUP)
76+
.withOption()
77+
.longNames("arg1")
78+
.type(String[].class)
79+
.arity(0, 3)
80+
.and()
81+
.withTarget()
82+
.function(ctx -> {
83+
String[] arg1 = ctx.getOptionValue("arg1");
84+
return "Hello " + Arrays.asList(arg1);
85+
})
86+
.and()
87+
.build();
88+
}
89+
90+
@ShellMethod(key = LEGACY_ANNO + "arity-float-array", group = GROUP)
91+
public String testArityFloatArrayLegacyAnnotation(
92+
@ShellOption(value = "--arg1", arity = 3) float[] arg1
93+
) {
94+
return "Hello " + floatsToString(arg1);
95+
}
96+
97+
@Bean
98+
public CommandRegistration testArityFloatArrayRegistration() {
99+
return CommandRegistration.builder()
100+
.command(REG, "arity-float-array")
101+
.group(GROUP)
102+
.withOption()
103+
.longNames("arg1")
104+
.type(float[].class)
105+
.arity(0, 3)
106+
.and()
107+
.withTarget()
108+
.function(ctx -> {
109+
float[] arg1 = ctx.getOptionValue("arg1");
110+
return "Hello " + floatsToString(arg1);
111+
})
112+
.and()
113+
.build();
114+
}
115+
116+
@Bean
117+
public CommandRegistration testArityErrorsRegistration() {
118+
return CommandRegistration.builder()
119+
.command(REG, "arity-errors")
120+
.group(GROUP)
121+
.withOption()
122+
.longNames("arg1")
123+
.type(String[].class)
124+
.required()
125+
.arity(1, 2)
126+
.and()
127+
.withTarget()
128+
.function(ctx -> {
129+
String[] arg1 = ctx.getOptionValue("arg1");
130+
return "Hello " + Arrays.asList(arg1);
131+
})
132+
.and()
133+
.build();
134+
}
135+
136+
private static String floatsToString(float[] arg1) {
137+
return IntStream.range(0, arg1.length)
138+
.mapToDouble(i -> arg1[i])
139+
.boxed()
140+
.map(d -> d.toString())
141+
.collect(Collectors.joining(","));
142+
}
59143
}

0 commit comments

Comments
 (0)