Fix Mixup of Option short and long name #1284

Signed-off-by: czpilar <david@czpilar.net>

Author: czpilar

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

@@ -15,16 +15,17 @@
  */
 package org.springframework.shell.core.command;
 
-import java.io.PrintWriter;
-
 import org.jspecify.annotations.Nullable;
-
 import org.springframework.shell.core.InputReader;
 
+import java.io.PrintWriter;
+import java.util.function.Predicate;
+
 /**
  * Interface containing runtime information about the current command invocation.
  *
  * @author Mahmoud Ben Hassine
+ * @author David Pilar
  * @since 4.0.0
  */
 public record CommandContext(ParsedInput parsedInput, CommandRegistry commandRegistry, PrintWriter outputWriter,
@@ -36,11 +37,33 @@ public record CommandContext(ParsedInput parsedInput, CommandRegistry commandReg
 	 * @return the matching {@link CommandOption} or null if not found
 	 */
 	@Nullable public CommandOption getOptionByName(String optionName) {
-		return this.parsedInput.options()
-			.stream()
-			.filter(option -> option.longName().equals(optionName) || option.shortName() == optionName.charAt(0))
-			.findFirst()
-			.orElse(null);
+		CommandOption option = getOptionByLongName(optionName);
+		if (option == null && optionName.length() == 1) {
+			option = getOptionByShortName(optionName.charAt(0));
+		}
+		return option;
+	}
+
+	/**
+	 * Retrieve a command option by its long name.
+	 * @param longName the long name of the option to retrieve
+	 * @return the matching {@link CommandOption} or null if not found
+	 */
+	@Nullable public CommandOption getOptionByLongName(String longName) {
+		return getOptionByFilter(option -> longName.equals(option.longName()));
+	}
+
+	/**
+	 * Retrieve a command option by its short name.
+	 * @param shortName the short name of the option to retrieve
+	 * @return the matching {@link CommandOption} or null if not found
+	 */
+	@Nullable public CommandOption getOptionByShortName(char shortName) {
+		return getOptionByFilter(option -> option.shortName() == shortName);
+	}
+
+	@Nullable private CommandOption getOptionByFilter(Predicate<CommandOption> filter) {
+		return this.parsedInput.options().stream().filter(filter).findFirst().orElse(null);
 	}
 
 	/**

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

@@ -43,6 +43,7 @@
  * An adapter to adapt a method as a command.
  *
  * @author Mahmoud Ben Hassine
+ * @author David Pilar
  * @since 4.0.0
  */
 public class MethodInvokerCommandAdapter extends AbstractCommand {
@@ -141,8 +142,13 @@ private List<Object> prepareArguments(CommandContext commandContext) {
 									+ parameters[i].getName() + "'");
 				}
 				boolean required = optionAnnotation.required();
-				CommandOption commandOption = commandContext
-					.getOptionByName(longName.isEmpty() ? String.valueOf(shortName) : longName);
+				CommandOption commandOption = null;
+				if (!longName.isEmpty()) {
+					commandOption = commandContext.getOptionByLongName(longName);
+				}
+				if (commandOption == null && shortName != ' ') {
+					commandOption = commandContext.getOptionByShortName(shortName);
+				}
 				if (commandOption != null) {
 					String rawValue = commandOption.value();
 					Class<?> parameterType = parameterTypes[i];

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

@@ -0,0 +1,66 @@
+package org.springframework.shell.core.command;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.springframework.shell.core.InputReader;
+
+import java.io.PrintWriter;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.mock;
+
+class CommandContextTests {
+
+	private CommandContext context;
+
+	@BeforeEach
+	void setUp() {
+		ParsedInput parsedInput = ParsedInput.builder()
+			.addOption(CommandOption.with().longName("aWrong").description("command1").build())
+			.addOption(CommandOption.with().longName("intendedA").shortName('a').description("command2").build())
+			.addOption(CommandOption.with().shortName('b').description("command3").build())
+			.addOption(CommandOption.with().longName("x").description("command4").build())
+			.addOption(CommandOption.with().shortName('x').description("command5").build())
+			.addOption(CommandOption.with().shortName('y').description("command6").build())
+			.addOption(CommandOption.with().longName("y").description("command7").build())
+			.build();
+		context = new CommandContext(parsedInput, mock(CommandRegistry.class), mock(PrintWriter.class),
+				mock(InputReader.class));
+	}
+
+	@ParameterizedTest
+	@CsvSource({ "aWrong, command1", "intendedA, command2", "noSuchOption, null", "x, command4", "y, command7" })
+	void testGetOptionByLongName(String longName, String description) {
+		// when
+		CommandOption commandOption = context.getOptionByLongName(longName);
+
+		// then
+		String result = commandOption == null ? "null" : commandOption.description();
+		assertEquals(description, result);
+	}
+
+	@ParameterizedTest
+	@CsvSource({ "a, command2", "b, command3", "n, null", "x, command5", "y, command6" })
+	void testGetOptionByShortName(char shortName, String description) {
+		// when
+		CommandOption commandOption = context.getOptionByShortName(shortName);
+
+		// then
+		String result = commandOption == null ? "null" : commandOption.description();
+		assertEquals(description, result);
+	}
+
+	@ParameterizedTest
+	@CsvSource({ "aWrong, command1", "intendedA, command2", "noSuchOption, null", "a, command2", "b, command3",
+			"n, null", "x, command4", "y, command7" })
+	void testGetOptionByName(String optionName, String description) {
+		// when
+		CommandOption commandOption = context.getOptionByName(optionName);
+
+		// then
+		String result = commandOption == null ? "null" : commandOption.description();
+		assertEquals(description, result);
+	}
+
+}