From a9c7840409d6c1afdf61101ebed63e192b527ff9 Mon Sep 17 00:00:00 2001 From: David Pilar Date: Mon, 2 Feb 2026 00:55:38 +0100 Subject: [PATCH] Fix Boolean option without value does not properly work with argument #1309 Signed-off-by: David Pilar --- .../ShellRunnerAutoConfiguration.java | 8 ++- .../StandardCommandsAutoConfiguration.java | 8 ++- .../core/command/DefaultCommandParser.java | 21 ++++++- .../shell/core/command/Script.java | 6 +- .../command/DefaultCommandParserTests.java | 55 ++++++++++++++++++- .../jline/DefaultJLineShellConfiguration.java | 8 ++- .../ShellTestClientAutoConfiguration.java | 8 ++- .../shell/test/ShellTestClientTests.java | 8 ++- 8 files changed, 108 insertions(+), 14 deletions(-) diff --git a/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/ShellRunnerAutoConfiguration.java b/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/ShellRunnerAutoConfiguration.java index 4b9fc6d89..b3dfcdb03 100644 --- a/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/ShellRunnerAutoConfiguration.java +++ b/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/ShellRunnerAutoConfiguration.java @@ -21,6 +21,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Bean; +import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.core.env.Environment; import org.springframework.shell.core.ConsoleInputProvider; import org.springframework.shell.core.NonInteractiveShellRunner; @@ -30,6 +31,8 @@ import org.springframework.shell.core.command.CommandRegistry; import org.springframework.shell.core.command.DefaultCommandParser; +import java.util.Optional; + @AutoConfiguration public class ShellRunnerAutoConfiguration { @@ -66,8 +69,9 @@ public ConsoleInputProvider consoleInputProvider() { @Bean @ConditionalOnMissingBean - public CommandParser commandParser(CommandRegistry commandRegistry) { - return new DefaultCommandParser(commandRegistry); + public CommandParser commandParser(CommandRegistry commandRegistry, + Optional conversionService) { + return new DefaultCommandParser(commandRegistry, conversionService); } } diff --git a/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/StandardCommandsAutoConfiguration.java b/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/StandardCommandsAutoConfiguration.java index c7348db2a..1c34a1bed 100644 --- a/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/StandardCommandsAutoConfiguration.java +++ b/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/StandardCommandsAutoConfiguration.java @@ -23,8 +23,11 @@ import org.springframework.boot.info.BuildProperties; import org.springframework.boot.info.GitProperties; import org.springframework.context.annotation.Bean; +import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.shell.core.command.*; +import java.util.Optional; + /** * Creates beans for standard commands. * @@ -73,8 +76,9 @@ public Command versionCommand(SpringShellProperties shellProperties, @Bean @ConditionalOnProperty(value = "spring.shell.command.script.enabled", havingValue = "true", matchIfMissing = true) - public Command scriptCommand(CommandRegistry commandRegistry) { - return new Script(commandRegistry); + public Command scriptCommand(CommandRegistry commandRegistry, + Optional conversionService) { + return new Script(commandRegistry, conversionService); } } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/core/command/DefaultCommandParser.java b/spring-shell-core/src/main/java/org/springframework/shell/core/command/DefaultCommandParser.java index f368ddf6f..828a841bf 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/core/command/DefaultCommandParser.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/core/command/DefaultCommandParser.java @@ -21,6 +21,9 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.convert.ConversionFailedException; +import org.springframework.core.convert.support.ConfigurableConversionService; +import org.springframework.core.convert.support.DefaultConversionService; /** * Default implementation of {@link CommandParser}. Supports options in the long form of @@ -48,8 +51,12 @@ public class DefaultCommandParser implements CommandParser { private final CommandRegistry commandRegistry; - public DefaultCommandParser(CommandRegistry commandRegistry) { + private final ConfigurableConversionService conversionService; + + public DefaultCommandParser(CommandRegistry commandRegistry, + Optional conversionService) { this.commandRegistry = commandRegistry; + this.conversionService = conversionService.orElse(new DefaultConversionService()); } @Override @@ -121,6 +128,9 @@ public ParsedInput parse(String input) { } nextWord = "true"; } + else if (isBooleanOption(commandName, currentWord) && !isBooleanValue(nextWord)) { + nextWord = "true"; + } else { i++; // skip next word as it was used as option value } @@ -199,4 +209,13 @@ private boolean isBooleanOption(String commandName, String currentWord) { .anyMatch(o -> o.type() == boolean.class || o.type() == Boolean.class); } + private boolean isBooleanValue(String rawValue) { + try { + return conversionService.convert(rawValue, boolean.class) != null; + } + catch (ConversionFailedException e) { + return false; + } + } + } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/core/command/Script.java b/spring-shell-core/src/main/java/org/springframework/shell/core/command/Script.java index 5cb8284ed..4ad30c18b 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/core/command/Script.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/core/command/Script.java @@ -18,7 +18,9 @@ import java.io.File; import java.util.List; import java.util.Objects; +import java.util.Optional; +import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.shell.core.FileInputProvider; import org.springframework.shell.core.NonInteractiveShellRunner; @@ -34,8 +36,8 @@ public class Script implements Command { private CommandParser commandParser; - public Script(CommandRegistry commandRegistry) { - this.commandParser = new DefaultCommandParser(commandRegistry); + public Script(CommandRegistry commandRegistry, Optional conversionService) { + this.commandParser = new DefaultCommandParser(commandRegistry, conversionService); } @Override diff --git a/spring-shell-core/src/test/java/org/springframework/shell/core/command/DefaultCommandParserTests.java b/spring-shell-core/src/test/java/org/springframework/shell/core/command/DefaultCommandParserTests.java index b59fe2161..d44cee2fb 100644 --- a/spring-shell-core/src/test/java/org/springframework/shell/core/command/DefaultCommandParserTests.java +++ b/spring-shell-core/src/test/java/org/springframework/shell/core/command/DefaultCommandParserTests.java @@ -23,6 +23,7 @@ import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; +import java.util.Optional; import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -36,7 +37,7 @@ class DefaultCommandParserTests { @BeforeEach void setUp() { this.commandRegistry = new CommandRegistry(); - this.parser = new DefaultCommandParser(this.commandRegistry); + this.parser = new DefaultCommandParser(this.commandRegistry, Optional.empty()); } @Test @@ -315,6 +316,58 @@ static Stream parseWithBooleanOptionData() { Arguments.of("mycommand -o", "", 'o', Boolean.class, "true")); } + @ParameterizedTest + @MethodSource("parseWithBooleanOptionAndArgumentData") + void testParseWithBooleanOptionAndArgument(String input, String expectedValue, int expectedArguments, + String expectedArgumentValue) { + // given + Command command = createCommand("mycommand", "My test command"); + command.getOptions().add(CommandOption.with().longName("option").type(boolean.class).build()); + commandRegistry.registerCommand(command); + // when + ParsedInput parsedInput = parser.parse(input); + + // then + assertEquals("mycommand", parsedInput.commandName()); + assertEquals(1, parsedInput.options().size()); + assertEquals(expectedValue, parsedInput.options().get(0).value()); + assertEquals(expectedArguments, parsedInput.arguments().size()); + if (expectedArguments > 0) { + assertEquals(expectedArgumentValue, parsedInput.arguments().get(0).value()); + } + } + + static Stream parseWithBooleanOptionAndArgumentData() { + return Stream.of(Arguments.of("mycommand --option=false", "false", 0, null), + Arguments.of("mycommand --option=true", "true", 0, null), + Arguments.of("mycommand --option false", "false", 0, null), + Arguments.of("mycommand --option true", "true", 0, null), + Arguments.of("mycommand --option", "true", 0, null), + + Arguments.of("mycommand --option=false argument", "false", 1, "argument"), + Arguments.of("mycommand --option=true argument", "true", 1, "argument"), + Arguments.of("mycommand --option false argument", "false", 1, "argument"), + Arguments.of("mycommand --option true argument", "true", 1, "argument"), + Arguments.of("mycommand --option argument", "true", 1, "argument"), + + Arguments.of("mycommand argument --option=false", "false", 1, "argument"), + Arguments.of("mycommand argument --option=true", "true", 1, "argument"), + Arguments.of("mycommand argument --option false", "false", 1, "argument"), + Arguments.of("mycommand argument --option true", "true", 1, "argument"), + Arguments.of("mycommand argument --option", "true", 1, "argument"), + + Arguments.of("mycommand --option=false true", "false", 1, "true"), + Arguments.of("mycommand --option=true true", "true", 1, "true"), + Arguments.of("mycommand --option false false", "false", 1, "false"), + Arguments.of("mycommand --option true false", "true", 1, "false"), + + Arguments.of("mycommand false --option=false", "false", 1, "false"), + Arguments.of("mycommand false --option=true", "true", 1, "false"), + Arguments.of("mycommand true --option false", "false", 1, "true"), + Arguments.of("mycommand true --option true", "true", 1, "true"), + Arguments.of("mycommand true --option", "true", 1, "true")); + } + @ParameterizedTest @ValueSource(strings = { "mycommand --help", "mycommand -h" }) void testParseWithHelpOption(String input) { diff --git a/spring-shell-jline/src/main/java/org/springframework/shell/jline/DefaultJLineShellConfiguration.java b/spring-shell-jline/src/main/java/org/springframework/shell/jline/DefaultJLineShellConfiguration.java index 8159a471d..720b0c233 100644 --- a/spring-shell-jline/src/main/java/org/springframework/shell/jline/DefaultJLineShellConfiguration.java +++ b/spring-shell-jline/src/main/java/org/springframework/shell/jline/DefaultJLineShellConfiguration.java @@ -1,6 +1,7 @@ package org.springframework.shell.jline; import java.io.IOException; +import java.util.Optional; import org.jline.reader.LineReader; import org.jline.reader.LineReaderBuilder; @@ -10,6 +11,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.shell.core.ShellConfigurationException; import org.springframework.shell.core.ShellRunner; import org.springframework.shell.core.command.CommandRegistry; @@ -19,8 +21,10 @@ public class DefaultJLineShellConfiguration { @Bean - public ShellRunner shellRunner(JLineInputProvider inputProvider, CommandRegistry commandRegistry) { - return new JLineShellRunner(inputProvider, new DefaultCommandParser(commandRegistry), commandRegistry); + public ShellRunner shellRunner(JLineInputProvider inputProvider, CommandRegistry commandRegistry, + Optional conversionService) { + return new JLineShellRunner(inputProvider, new DefaultCommandParser(commandRegistry, conversionService), + commandRegistry); } @Bean diff --git a/spring-shell-test-autoconfigure/src/main/java/org/springframework/shell/test/autoconfigure/ShellTestClientAutoConfiguration.java b/spring-shell-test-autoconfigure/src/main/java/org/springframework/shell/test/autoconfigure/ShellTestClientAutoConfiguration.java index d658aabcc..418fa7217 100644 --- a/spring-shell-test-autoconfigure/src/main/java/org/springframework/shell/test/autoconfigure/ShellTestClientAutoConfiguration.java +++ b/spring-shell-test-autoconfigure/src/main/java/org/springframework/shell/test/autoconfigure/ShellTestClientAutoConfiguration.java @@ -18,11 +18,14 @@ import org.springframework.boot.autoconfigure.AutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.context.annotation.Bean; +import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.shell.core.command.CommandParser; import org.springframework.shell.core.command.CommandRegistry; import org.springframework.shell.core.command.DefaultCommandParser; import org.springframework.shell.test.ShellTestClient; +import java.util.Optional; + /** * @author Janne Valkealahti * @author Mahmoud Ben Hassine @@ -44,8 +47,9 @@ public CommandRegistry commandRegistry() { @Bean @ConditionalOnMissingBean - public CommandParser commandParser(CommandRegistry commandRegistry) { - return new DefaultCommandParser(commandRegistry); + public CommandParser commandParser(CommandRegistry commandRegistry, + Optional conversionService) { + return new DefaultCommandParser(commandRegistry, conversionService); } } diff --git a/spring-shell-test/src/test/java/org/springframework/shell/test/ShellTestClientTests.java b/spring-shell-test/src/test/java/org/springframework/shell/test/ShellTestClientTests.java index 3f1ebbbef..f12354b47 100644 --- a/spring-shell-test/src/test/java/org/springframework/shell/test/ShellTestClientTests.java +++ b/spring-shell-test/src/test/java/org/springframework/shell/test/ShellTestClientTests.java @@ -7,6 +7,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.shell.core.command.AbstractCommand; import org.springframework.shell.core.command.Command; import org.springframework.shell.core.command.CommandContext; @@ -17,6 +18,8 @@ import org.springframework.shell.core.command.ExitStatus; import org.springframework.test.context.junit.jupiter.SpringExtension; +import java.util.Optional; + @ExtendWith(SpringExtension.class) class ShellTestClientTests { @@ -56,8 +59,9 @@ public CommandRegistry commandRegistry() { } @Bean - public CommandParser commandParser(CommandRegistry commandRegistry) { - return new DefaultCommandParser(commandRegistry); + public CommandParser commandParser(CommandRegistry commandRegistry, + Optional conversionService) { + return new DefaultCommandParser(commandRegistry, conversionService); } @Bean