Fix longName defaulting to parameter name when not provided #1261

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

@@ -136,16 +136,11 @@ private List<Object> prepareArguments(CommandContext commandContext) {
 				log.debug("Processing option for parameter: " + parameters[i].getName());
 				char shortName = optionAnnotation.shortName();
 				String longName = optionAnnotation.longName();
-				if (shortName == ' ' && longName.isEmpty()) {
-					throw new IllegalArgumentException(
-							"Either shortName or longName (or both) must be provided for option on parameter '"
-									+ parameters[i].getName() + "'");
+				if (longName.isEmpty()) {
+					longName = parameters[i].getName();
 				}
 				boolean required = optionAnnotation.required();
-				CommandOption commandOption = null;
-				if (!longName.isEmpty()) {
-					commandOption = commandContext.getOptionByLongName(longName);
-				}
+				CommandOption commandOption = commandContext.getOptionByLongName(longName);
 				if (commandOption == null && shortName != ' ') {
 					commandOption = commandContext.getOptionByShortName(shortName);
 				}
@@ -157,15 +152,13 @@ private List<Object> prepareArguments(CommandContext commandContext) {
 				}
 				else {
 					if (required) {
-						throw new IllegalArgumentException(
-								"Required option '--" + (longName.isEmpty() ? shortName : longName) + "' is missing.");
+						throw new IllegalArgumentException("Required option '--" + longName + "' is missing.");
 					}
 					else {
 						// try to use default value
 						String defaultValue = optionAnnotation.defaultValue();
 						if (defaultValue.isEmpty()) {
-							log.warn("No value provided for optional option '--"
-									+ (longName.isEmpty() ? shortName : longName)
+							log.warn("No value provided for optional option '--" + longName
 									+ "' and no default value specified.");
 						}
 						Class<?> parameterType = parameterTypes[i];

spring-shell-core/src/main/java/org/springframework/shell/core/command/annotation/support/CommandFactoryBean.java

@@ -53,6 +53,7 @@
  * @author Janne Valkealahti
  * @author Piotr Olaszewski
  * @author Mahmoud Ben Hassine
+ * @author David Pilar
  */
 public class CommandFactoryBean implements ApplicationContextAware, FactoryBean<Command> {
 
@@ -119,14 +120,12 @@ private List<CommandOption> getCommandOptions() {
 			if (optionAnnotation != null) {
 				char shortName = optionAnnotation.shortName();
 				String longName = optionAnnotation.longName();
+				if (longName.isEmpty()) {
+					longName = parameter.getName();
+				}
 				String description = optionAnnotation.description();
 				boolean required = optionAnnotation.required();
 				String defaultValue = optionAnnotation.defaultValue();
-				if (shortName == ' ' && longName.isEmpty()) {
-					throw new IllegalArgumentException(
-							"Either shortName or longName (or both) must be provided for option on parameter '"
-									+ parameter.getName() + "'");
-				}
 				CommandOption commandOption = CommandOption.with()
 					.shortName(shortName)
 					.longName(longName)

spring-shell-core/src/test/java/org/springframework/shell/core/command/annotation/support/CommandFactoryBeanTests.java

@@ -0,0 +1,56 @@
+package org.springframework.shell.core.command.annotation.support;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.context.ApplicationContext;
+import org.springframework.shell.core.command.annotation.Command;
+import org.springframework.shell.core.command.annotation.Option;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * @author David Pilar
+ */
+class CommandFactoryBeanTests {
+
+	@Test
+	void testOptionNames() {
+		// given
+		ApplicationContext context = mock(ApplicationContext.class);
+		when(context.getBean(TestClass.class)).thenReturn(new TestClass());
+		CommandFactoryBean commandFactoryBean = new CommandFactoryBean(TestClass.class.getDeclaredMethods()[0]);
+		commandFactoryBean.setApplicationContext(context);
+
+		// when
+		org.springframework.shell.core.command.Command result = commandFactoryBean.getObject();
+
+		// then
+		assertEquals("hello", result.getName());
+		assertEquals(4, result.getOptions().size());
+
+		assertEquals("myOption1", result.getOptions().get(0).longName());
+		assertEquals(' ', result.getOptions().get(0).shortName());
+
+		assertEquals("myOption2", result.getOptions().get(1).longName());
+		assertEquals('m', result.getOptions().get(1).shortName());
+
+		assertEquals("longNameOption3", result.getOptions().get(2).longName());
+		assertEquals('l', result.getOptions().get(2).shortName());
+
+		assertEquals("longNameOption4", result.getOptions().get(3).longName());
+		assertEquals(' ', result.getOptions().get(3).shortName());
+	}
+
+	static class TestClass {
+
+		@Command(name = "hello")
+		public void helloMethod(@Option String myOption1, @Option(shortName = 'm') String myOption2,
+				@Option(longName = "longNameOption3", shortName = 'l') String myOption3,
+				@Option(longName = "longNameOption4") String myOption4) {
+			// no-op
+		}
+
+	}
+
+}

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

@@ -12,6 +12,7 @@
 import org.springframework.shell.core.command.completion.CompletionProposal;
 import org.springframework.shell.core.command.completion.CompletionProvider;
 import org.springframework.shell.core.utils.Utils;
+import org.springframework.util.StringUtils;
 
 import java.util.*;
 import java.util.function.Predicate;
@@ -46,7 +47,7 @@ public void complete(LineReader reader, ParsedLine line, List<Candidate> candida
 				// add option completions for the command
 				for (CommandOption option : options) {
 					boolean present = isOptionPresent(line, option);
-					if (option.longName() != null && !present) {
+					if (StringUtils.hasLength(option.longName()) && !present) {
 						candidates.add(new Candidate("--" + option.longName()));
 					}
 					if (option.shortName() != ' ' && !present) {