Skip to content

Commit 6b1e701

Browse files
czpilarfmbenhassine
authored andcommitted
Fix longName defaulting to parameter name when not provided
Resolves #1261 Signed-off-by: David Pilar <[email protected]>
1 parent 10d33a8 commit 6b1e701

File tree

4 files changed

+86
-18
lines changed

4 files changed

+86
-18
lines changed

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -136,16 +136,11 @@ private List<Object> prepareArguments(CommandContext commandContext) {
136136
log.debug("Processing option for parameter: " + parameters[i].getName());
137137
char shortName = optionAnnotation.shortName();
138138
String longName = optionAnnotation.longName();
139-
if (shortName == ' ' && longName.isEmpty()) {
140-
throw new IllegalArgumentException(
141-
"Either shortName or longName (or both) must be provided for option on parameter '"
142-
+ parameters[i].getName() + "'");
139+
if (longName.isEmpty()) {
140+
longName = parameters[i].getName();
143141
}
144142
boolean required = optionAnnotation.required();
145-
CommandOption commandOption = null;
146-
if (!longName.isEmpty()) {
147-
commandOption = commandContext.getOptionByLongName(longName);
148-
}
143+
CommandOption commandOption = commandContext.getOptionByLongName(longName);
149144
if (commandOption == null && shortName != ' ') {
150145
commandOption = commandContext.getOptionByShortName(shortName);
151146
}
@@ -157,15 +152,13 @@ private List<Object> prepareArguments(CommandContext commandContext) {
157152
}
158153
else {
159154
if (required) {
160-
throw new IllegalArgumentException(
161-
"Required option '--" + (longName.isEmpty() ? shortName : longName) + "' is missing.");
155+
throw new IllegalArgumentException("Required option '--" + longName + "' is missing.");
162156
}
163157
else {
164158
// try to use default value
165159
String defaultValue = optionAnnotation.defaultValue();
166160
if (defaultValue.isEmpty()) {
167-
log.warn("No value provided for optional option '--"
168-
+ (longName.isEmpty() ? shortName : longName)
161+
log.warn("No value provided for optional option '--" + longName
169162
+ "' and no default value specified.");
170163
}
171164
Class<?> parameterType = parameterTypes[i];

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
* @author Janne Valkealahti
5454
* @author Piotr Olaszewski
5555
* @author Mahmoud Ben Hassine
56+
* @author David Pilar
5657
*/
5758
public class CommandFactoryBean implements ApplicationContextAware, FactoryBean<Command> {
5859

@@ -119,14 +120,12 @@ private List<CommandOption> getCommandOptions() {
119120
if (optionAnnotation != null) {
120121
char shortName = optionAnnotation.shortName();
121122
String longName = optionAnnotation.longName();
123+
if (longName.isEmpty()) {
124+
longName = parameter.getName();
125+
}
122126
String description = optionAnnotation.description();
123127
boolean required = optionAnnotation.required();
124128
String defaultValue = optionAnnotation.defaultValue();
125-
if (shortName == ' ' && longName.isEmpty()) {
126-
throw new IllegalArgumentException(
127-
"Either shortName or longName (or both) must be provided for option on parameter '"
128-
+ parameter.getName() + "'");
129-
}
130129
CommandOption commandOption = CommandOption.with()
131130
.shortName(shortName)
132131
.longName(longName)
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
* Copyright 2026-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.shell.core.command.annotation.support;
17+
18+
import java.util.List;
19+
20+
import org.junit.jupiter.api.Test;
21+
import org.springframework.context.ApplicationContext;
22+
import org.springframework.shell.core.command.CommandOption;
23+
import org.springframework.shell.core.command.annotation.Command;
24+
import org.springframework.shell.core.command.annotation.Option;
25+
26+
import static org.junit.jupiter.api.Assertions.assertEquals;
27+
import static org.mockito.Mockito.mock;
28+
import static org.mockito.Mockito.when;
29+
30+
/**
31+
* @author David Pilar
32+
*/
33+
class CommandFactoryBeanTests {
34+
35+
@Test
36+
void testOptionNames() {
37+
// given
38+
ApplicationContext context = mock(ApplicationContext.class);
39+
when(context.getBean(TestClass.class)).thenReturn(new TestClass());
40+
CommandFactoryBean commandFactoryBean = new CommandFactoryBean(TestClass.class.getDeclaredMethods()[0]);
41+
commandFactoryBean.setApplicationContext(context);
42+
43+
// when
44+
org.springframework.shell.core.command.Command result = commandFactoryBean.getObject();
45+
46+
// then
47+
assertEquals("hello", result.getName());
48+
List<CommandOption> options = result.getOptions();
49+
assertEquals(4, options.size());
50+
51+
assertEquals("myOption1", options.get(0).longName());
52+
assertEquals(' ', options.get(0).shortName());
53+
54+
assertEquals("myOption2", options.get(1).longName());
55+
assertEquals('m', options.get(1).shortName());
56+
57+
assertEquals("longNameOption3", options.get(2).longName());
58+
assertEquals('l', options.get(2).shortName());
59+
60+
assertEquals("longNameOption4", options.get(3).longName());
61+
assertEquals(' ', options.get(3).shortName());
62+
}
63+
64+
static class TestClass {
65+
66+
@Command(name = "hello")
67+
public void helloMethod(@Option String myOption1, @Option(shortName = 'm') String myOption2,
68+
@Option(longName = "longNameOption3", shortName = 'l') String myOption3,
69+
@Option(longName = "longNameOption4") String myOption4) {
70+
// no-op
71+
}
72+
73+
}
74+
75+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.springframework.shell.core.command.completion.CompletionProposal;
1313
import org.springframework.shell.core.command.completion.CompletionProvider;
1414
import org.springframework.shell.core.utils.Utils;
15+
import org.springframework.util.StringUtils;
1516

1617
import java.util.*;
1718
import java.util.function.Predicate;
@@ -46,7 +47,7 @@ public void complete(LineReader reader, ParsedLine line, List<Candidate> candida
4647
// add option completions for the command
4748
for (CommandOption option : options) {
4849
boolean present = isOptionPresent(line, option);
49-
if (option.longName() != null && !present) {
50+
if (StringUtils.hasLength(option.longName()) && !present) {
5051
candidates.add(new Candidate("--" + option.longName()));
5152
}
5253
if (option.shortName() != ' ' && !present) {

0 commit comments

Comments
 (0)