Skip to content

Commit b1084a6

Browse files
committed
Fix wrong parser message
- Parser error message gave wrong lower level arity value if option didn't have enough arguments - Fixes #930
1 parent 8d6223d commit b1084a6

File tree

6 files changed

+107
-1
lines changed

6 files changed

+107
-1
lines changed

spring-shell-core/src/main/java/org/springframework/shell/command/parser/Parser.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ else if (currentOption.getArityMax() > -1 && currentOptionArgument.size() > curr
351351
if (currentOption.getArityMin() > -1 && toUse.size() < currentOption.getArityMin()) {
352352
String arg = currentOption.getLongNames()[0];
353353
commonMessageResults.add(MessageResult.of(ParserMessage.NOT_ENOUGH_OPTION_ARGUMENTS, 0, arg,
354-
toUse.size()));
354+
currentOption.getArityMin()));
355355
}
356356
else if (currentOption.getArityMax() > -1 && toUse.size() > currentOption.getArityMax()) {
357357
String arg = currentOption.getLongNames()[0];

spring-shell-core/src/test/java/org/springframework/shell/command/parser/AbstractParsingTests.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,19 @@ abstract class AbstractParsingTests {
313313
.and()
314314
.build();
315315

316+
static final CommandRegistration ROOT8_ONE_ARG_ARITYEONE_STRING = CommandRegistration.builder()
317+
.command("root8")
318+
.withOption()
319+
.longNames("arg1")
320+
.type(String.class)
321+
.arity(OptionArity.EXACTLY_ONE)
322+
.position(0)
323+
.and()
324+
.withTarget()
325+
.consumer(ctx -> {})
326+
.and()
327+
.build();
328+
316329
Map<String, CommandRegistration> registrations = new HashMap<>();
317330

318331
@BeforeEach
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Copyright 2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.shell.command.parser;
17+
18+
import org.assertj.core.api.AbstractAssert;
19+
20+
import static org.assertj.core.api.Assertions.assertThat;
21+
22+
public class MessageResultAssert extends AbstractAssert<MessageResultAssert, MessageResult> {
23+
24+
public MessageResultAssert(MessageResult actual) {
25+
super(actual, MessageResultAssert.class);
26+
}
27+
28+
public MessageResultAssert hasPosition(int position) {
29+
isNotNull();
30+
assertThat(actual.position()).isEqualTo(position);
31+
return this;
32+
}
33+
34+
public MessageResultAssert hasInserts(Object... inserts) {
35+
isNotNull();
36+
assertThat(actual.inserts()).containsExactly(inserts);
37+
return this;
38+
}
39+
40+
}

spring-shell-core/src/test/java/org/springframework/shell/command/parser/ParserAssertions.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,8 @@ public static TokenAssert assertThat(Token actual) {
2424
public static ParserMessageAssert assertThat(ParserMessage message) {
2525
return new ParserMessageAssert(message);
2626
}
27+
28+
public static MessageResultAssert assertThat(MessageResult result) {
29+
return new MessageResultAssert(result);
30+
}
2731
}

spring-shell-core/src/test/java/org/springframework/shell/command/parser/ParserTests.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,5 +635,18 @@ void tooManyArgsInMiddleShouldCreateError() {
635635
}
636636
);
637637
}
638+
639+
@Test
640+
void notEnoughOptionArgumentsShouldHaveCorrectCount() {
641+
register(ROOT8_ONE_ARG_ARITYEONE_STRING);
642+
ParseResult result = parse("root8", "--arg1");
643+
assertThat(result.messageResults()).satisfiesExactlyInAnyOrder(
644+
message -> {
645+
ParserAssertions.assertThat(message.parserMessage()).hasCode(2003).hasType(ParserMessage.Type.ERROR);
646+
ParserAssertions.assertThat(message).hasPosition(0).hasInserts("arg1", 1);
647+
}
648+
);
649+
}
650+
638651
}
639652
}

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,13 @@ public String testArityFloatArrayLegacyAnnotation(
5858
return "Hello " + stringOfFloats(arg1);
5959
}
6060

61+
@ShellMethod(key = LEGACY_ANNO + "string-arityeone-required", group = GROUP)
62+
public String testStringArityeoneRequiredLegacyAnnotation(
63+
@ShellOption(value = "--arg1", arity = 1) String arg1
64+
) {
65+
return "Hello " + arg1;
66+
}
67+
6168
}
6269

6370
@Command(command = BaseE2ECommands.ANNO, group = BaseE2ECommands.GROUP)
@@ -86,6 +93,15 @@ public String testArityFloatArrayAnnotation(
8693
) {
8794
return "Hello " + stringOfFloats(arg1);
8895
}
96+
97+
@Command(command = "string-arityeone-required")
98+
public String testStringArityeoneRequiredAnnotation(
99+
@Option(longNames = {"arg1"}, arity = OptionArity.EXACTLY_ONE, required = true)
100+
String arg1
101+
) {
102+
return "Hello " + arg1;
103+
}
104+
89105
}
90106

91107
@Component
@@ -151,6 +167,26 @@ public CommandRegistration testArityFloatArrayRegistration() {
151167
.build();
152168
}
153169

170+
@Bean
171+
public CommandRegistration testStringArityeoneRequiredRegistration() {
172+
return getBuilder()
173+
.command(REG, "string-arityeone-required")
174+
.group(GROUP)
175+
.withOption()
176+
.longNames("arg1")
177+
.type(String.class)
178+
.required()
179+
.arity(OptionArity.EXACTLY_ONE)
180+
.and()
181+
.withTarget()
182+
.function(ctx -> {
183+
String arg1 = ctx.getOptionValue("arg1");
184+
return "Hello " + arg1;
185+
})
186+
.and()
187+
.build();
188+
}
189+
154190
@Bean
155191
public CommandRegistration testArityErrorsRegistration() {
156192
return getBuilder()

0 commit comments

Comments
 (0)