Skip to content

Commit af799ed

Browse files
authored
Fix parsing of boolean options without values
Resolves #1304 Signed-off-by: David Pilar <david@czpilar.net>
1 parent 84abfda commit af799ed

4 files changed

Lines changed: 72 additions & 10 deletions

File tree

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.shell.core.command;
1717

1818
import org.jspecify.annotations.Nullable;
19+
import org.springframework.util.StringUtils;
1920

2021
/**
2122
* Record representing the definition as well as the runtime information about a command
@@ -24,10 +25,16 @@
2425
* @author Janne Valkealahti
2526
* @author Piotr Olaszewski
2627
* @author Mahmoud Ben Hassine
28+
* @author David Pilar
2729
*/
2830
public record CommandOption(char shortName, @Nullable String longName, @Nullable String description,
2931
@Nullable Boolean required, @Nullable String defaultValue, @Nullable String value, Class<?> type) {
3032

33+
public boolean isOptionEqual(String optionName) {
34+
return StringUtils.hasLength(longName) && optionName.equals("--" + longName)
35+
|| shortName != ' ' && optionName.equals("-" + shortName);
36+
}
37+
3138
public static Builder with() {
3239
return new Builder();
3340
}

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import java.util.Collections;
1919
import java.util.List;
20+
import java.util.Optional;
2021

2122
import org.apache.commons.logging.Log;
2223
import org.apache.commons.logging.LogFactory;
@@ -102,11 +103,16 @@ public ParsedInput parse(String input) {
102103
}
103104
else { // use next word as option value
104105
if (nextWord == null || isOption(nextWord) || isArgumentSeparator(nextWord)) {
105-
throw new IllegalArgumentException("Option '" + currentWord + "' requires a value");
106+
if (!isBooleanOption(commandName, currentWord)) {
107+
throw new IllegalArgumentException("Option '" + currentWord + "' requires a value");
108+
}
109+
nextWord = "true";
110+
}
111+
else {
112+
i++; // skip next word as it was used as option value
106113
}
107114
CommandOption commandOption = parseOption(currentWord + "=" + nextWord);
108115
parsedInputBuilder.addOption(commandOption);
109-
i++; // skip next word as it was used as option value
110116
}
111117
}
112118
else {
@@ -171,4 +177,13 @@ private String unquoteAndUnescapeQuoted(String s) {
171177
return s;
172178
}
173179

180+
private boolean isBooleanOption(String commandName, String currentWord) {
181+
return Optional.ofNullable(commandRegistry.getCommandByName(commandName))
182+
.map(Command::getOptions)
183+
.orElse(List.of())
184+
.stream()
185+
.filter(o -> o.isOptionEqual(currentWord))
186+
.anyMatch(o -> o.type() == boolean.class || o.type() == Boolean.class);
187+
}
188+
174189
}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,51 @@ static Stream<Arguments> parseWithQuotedArgumentData() {
269269
Arguments.of("mycommand -- value", "value"), Arguments.of("mycommand -- \"value\"", "value"));
270270
}
271271

272+
@ParameterizedTest
273+
@MethodSource("parseWithBooleanOptionData")
274+
void testParseWithBooleanOption(String input, String longName, char shortName, Class<?> type,
275+
String expectedValue) {
276+
// given
277+
Command command = createCommand("mycommand", "My test command");
278+
command.getOptions().add(CommandOption.with().longName(longName).shortName(shortName).type(type).build());
279+
commandRegistry.registerCommand(command);
280+
// when
281+
ParsedInput parsedInput = parser.parse(input);
282+
283+
// then
284+
assertEquals("mycommand", parsedInput.commandName());
285+
assertEquals(1, parsedInput.options().size());
286+
assertEquals(longName, parsedInput.options().get(0).longName());
287+
assertEquals(shortName, parsedInput.options().get(0).shortName());
288+
assertEquals(expectedValue, parsedInput.options().get(0).value());
289+
}
290+
291+
static Stream<Arguments> parseWithBooleanOptionData() {
292+
return Stream.of(Arguments.of("mycommand --option=true", "option", ' ', boolean.class, "true"),
293+
Arguments.of("mycommand --option=false", "option", ' ', boolean.class, "false"),
294+
Arguments.of("mycommand --option true", "option", ' ', boolean.class, "true"),
295+
Arguments.of("mycommand --option false", "option", ' ', boolean.class, "false"),
296+
Arguments.of("mycommand --option", "option", ' ', boolean.class, "true"),
297+
298+
Arguments.of("mycommand -on=true", "", 'o', boolean.class, "true"),
299+
Arguments.of("mycommand -o=false", "", 'o', boolean.class, "false"),
300+
Arguments.of("mycommand -o true", "", 'o', boolean.class, "true"),
301+
Arguments.of("mycommand -o false", "", 'o', boolean.class, "false"),
302+
Arguments.of("mycommand -o", "", 'o', boolean.class, "true"),
303+
304+
Arguments.of("mycommand --option=true", "option", ' ', Boolean.class, "true"),
305+
Arguments.of("mycommand --option=false", "option", ' ', Boolean.class, "false"),
306+
Arguments.of("mycommand --option true", "option", ' ', Boolean.class, "true"),
307+
Arguments.of("mycommand --option false", "option", ' ', Boolean.class, "false"),
308+
Arguments.of("mycommand --option", "option", ' ', Boolean.class, "true"),
309+
310+
Arguments.of("mycommand -on=true", "", 'o', Boolean.class, "true"),
311+
Arguments.of("mycommand -o=false", "", 'o', Boolean.class, "false"),
312+
Arguments.of("mycommand -o true", "", 'o', Boolean.class, "true"),
313+
Arguments.of("mycommand -o false", "", 'o', Boolean.class, "false"),
314+
Arguments.of("mycommand -o", "", 'o', Boolean.class, "true"));
315+
}
316+
272317
private static Command createCommand(String name, String description) {
273318
return new AbstractCommand(name, description) {
274319
@Override

spring-shell-jline/src/main/java/org/springframework/shell/jline/CommandCompleter.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -116,15 +116,15 @@ private boolean isOptionPresent(ParsedLine line, CommandOption option) {
116116
CommandOption option;
117117
if (reversed.get(0).isEmpty()) {
118118
// the option name was completed, but no value provided ---> "--optionName "
119-
option = findOption(options, o -> isOptionEqual(reversed.get(1), o));
119+
option = findOption(options, o -> o.isOptionEqual(reversed.get(1)));
120120
}
121121
else {
122122
// the option uses key-value pair ---> "--optionName=someValue"
123123
option = findOption(options, o -> isOptionStartWith(reversed.get(0), o));
124124

125125
// the option uses completion on the value level ---> "--optionName someValue"
126126
if (option == null) {
127-
option = findOption(options, o -> isOptionEqual(reversed.get(1), o));
127+
option = findOption(options, o -> o.isOptionEqual(reversed.get(1)));
128128
}
129129
}
130130

@@ -135,13 +135,8 @@ private boolean isOptionPresent(ParsedLine line, CommandOption option) {
135135
return options.stream().filter(optionFilter).findFirst().orElse(null);
136136
}
137137

138-
private static boolean isOptionEqual(String optionName, CommandOption option) {
139-
return option.longName() != null && optionName.equals("--" + option.longName())
140-
|| option.shortName() != ' ' && optionName.equals("-" + option.shortName());
141-
}
142-
143138
private static boolean isOptionStartWith(String optionName, CommandOption option) {
144-
return option.longName() != null && optionName.startsWith("--" + option.longName() + "=")
139+
return StringUtils.hasLength(option.longName()) && optionName.startsWith("--" + option.longName() + "=")
145140
|| option.shortName() != ' ' && optionName.startsWith("-" + option.shortName() + "=");
146141
}
147142

0 commit comments

Comments
 (0)