Fix wrong parser message
- Parser error message gave wrong lower level arity value if option didn't have enough arguments - Backport #930 - Fixes #939
This commit is contained in:
@@ -351,7 +351,7 @@ public interface Parser {
|
|||||||
if (currentOption.getArityMin() > -1 && toUse.size() < currentOption.getArityMin()) {
|
if (currentOption.getArityMin() > -1 && toUse.size() < currentOption.getArityMin()) {
|
||||||
String arg = currentOption.getLongNames()[0];
|
String arg = currentOption.getLongNames()[0];
|
||||||
commonMessageResults.add(MessageResult.of(ParserMessage.NOT_ENOUGH_OPTION_ARGUMENTS, 0, arg,
|
commonMessageResults.add(MessageResult.of(ParserMessage.NOT_ENOUGH_OPTION_ARGUMENTS, 0, arg,
|
||||||
toUse.size()));
|
currentOption.getArityMin()));
|
||||||
}
|
}
|
||||||
else if (currentOption.getArityMax() > -1 && toUse.size() > currentOption.getArityMax()) {
|
else if (currentOption.getArityMax() > -1 && toUse.size() > currentOption.getArityMax()) {
|
||||||
String arg = currentOption.getLongNames()[0];
|
String arg = currentOption.getLongNames()[0];
|
||||||
|
|||||||
@@ -313,6 +313,19 @@ abstract class AbstractParsingTests {
|
|||||||
.and()
|
.and()
|
||||||
.build();
|
.build();
|
||||||
|
|
||||||
|
static final CommandRegistration ROOT8_ONE_ARG_ARITYEONE_STRING = CommandRegistration.builder()
|
||||||
|
.command("root8")
|
||||||
|
.withOption()
|
||||||
|
.longNames("arg1")
|
||||||
|
.type(String.class)
|
||||||
|
.arity(OptionArity.EXACTLY_ONE)
|
||||||
|
.position(0)
|
||||||
|
.and()
|
||||||
|
.withTarget()
|
||||||
|
.consumer(ctx -> {})
|
||||||
|
.and()
|
||||||
|
.build();
|
||||||
|
|
||||||
Map<String, CommandRegistration> registrations = new HashMap<>();
|
Map<String, CommandRegistration> registrations = new HashMap<>();
|
||||||
|
|
||||||
@BeforeEach
|
@BeforeEach
|
||||||
|
|||||||
@@ -0,0 +1,40 @@
|
|||||||
|
/*
|
||||||
|
* Copyright 2023 the original author or authors.
|
||||||
|
*
|
||||||
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
* you may not use this file except in compliance with the License.
|
||||||
|
* You may obtain a copy of the License at
|
||||||
|
*
|
||||||
|
* https://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
*
|
||||||
|
* Unless required by applicable law or agreed to in writing, software
|
||||||
|
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
* See the License for the specific language governing permissions and
|
||||||
|
* limitations under the License.
|
||||||
|
*/
|
||||||
|
package org.springframework.shell.command.parser;
|
||||||
|
|
||||||
|
import org.assertj.core.api.AbstractAssert;
|
||||||
|
|
||||||
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
|
|
||||||
|
public class MessageResultAssert extends AbstractAssert<MessageResultAssert, MessageResult> {
|
||||||
|
|
||||||
|
public MessageResultAssert(MessageResult actual) {
|
||||||
|
super(actual, MessageResultAssert.class);
|
||||||
|
}
|
||||||
|
|
||||||
|
public MessageResultAssert hasPosition(int position) {
|
||||||
|
isNotNull();
|
||||||
|
assertThat(actual.position()).isEqualTo(position);
|
||||||
|
return this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public MessageResultAssert hasInserts(Object... inserts) {
|
||||||
|
isNotNull();
|
||||||
|
assertThat(actual.inserts()).containsExactly(inserts);
|
||||||
|
return this;
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
||||||
@@ -24,4 +24,8 @@ public class ParserAssertions {
|
|||||||
public static ParserMessageAssert assertThat(ParserMessage message) {
|
public static ParserMessageAssert assertThat(ParserMessage message) {
|
||||||
return new ParserMessageAssert(message);
|
return new ParserMessageAssert(message);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public static MessageResultAssert assertThat(MessageResult result) {
|
||||||
|
return new MessageResultAssert(result);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -635,5 +635,18 @@ class ParserTests extends AbstractParsingTests {
|
|||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void notEnoughOptionArgumentsShouldHaveCorrectCount() {
|
||||||
|
register(ROOT8_ONE_ARG_ARITYEONE_STRING);
|
||||||
|
ParseResult result = parse("root8", "--arg1");
|
||||||
|
assertThat(result.messageResults()).satisfiesExactlyInAnyOrder(
|
||||||
|
message -> {
|
||||||
|
ParserAssertions.assertThat(message.parserMessage()).hasCode(2003).hasType(ParserMessage.Type.ERROR);
|
||||||
|
ParserAssertions.assertThat(message).hasPosition(0).hasInserts("arg1", 1);
|
||||||
|
}
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -58,6 +58,13 @@ public class ArityCommands {
|
|||||||
return "Hello " + stringOfFloats(arg1);
|
return "Hello " + stringOfFloats(arg1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ShellMethod(key = LEGACY_ANNO + "string-arityeone-required", group = GROUP)
|
||||||
|
public String testStringArityeoneRequiredLegacyAnnotation(
|
||||||
|
@ShellOption(value = "--arg1", arity = 1) String arg1
|
||||||
|
) {
|
||||||
|
return "Hello " + arg1;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Command(command = BaseE2ECommands.ANNO, group = BaseE2ECommands.GROUP)
|
@Command(command = BaseE2ECommands.ANNO, group = BaseE2ECommands.GROUP)
|
||||||
@@ -86,6 +93,15 @@ public class ArityCommands {
|
|||||||
) {
|
) {
|
||||||
return "Hello " + stringOfFloats(arg1);
|
return "Hello " + stringOfFloats(arg1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Command(command = "string-arityeone-required")
|
||||||
|
public String testStringArityeoneRequiredAnnotation(
|
||||||
|
@Option(longNames = {"arg1"}, arity = OptionArity.EXACTLY_ONE, required = true)
|
||||||
|
String arg1
|
||||||
|
) {
|
||||||
|
return "Hello " + arg1;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Component
|
@Component
|
||||||
@@ -151,6 +167,26 @@ public class ArityCommands {
|
|||||||
.build();
|
.build();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Bean
|
||||||
|
public CommandRegistration testStringArityeoneRequiredRegistration() {
|
||||||
|
return getBuilder()
|
||||||
|
.command(REG, "string-arityeone-required")
|
||||||
|
.group(GROUP)
|
||||||
|
.withOption()
|
||||||
|
.longNames("arg1")
|
||||||
|
.type(String.class)
|
||||||
|
.required()
|
||||||
|
.arity(OptionArity.EXACTLY_ONE)
|
||||||
|
.and()
|
||||||
|
.withTarget()
|
||||||
|
.function(ctx -> {
|
||||||
|
String arg1 = ctx.getOptionValue("arg1");
|
||||||
|
return "Hello " + arg1;
|
||||||
|
})
|
||||||
|
.and()
|
||||||
|
.build();
|
||||||
|
}
|
||||||
|
|
||||||
@Bean
|
@Bean
|
||||||
public CommandRegistration testArityErrorsRegistration() {
|
public CommandRegistration testArityErrorsRegistration() {
|
||||||
return getBuilder()
|
return getBuilder()
|
||||||
|
|||||||
Reference in New Issue
Block a user