Skip to content

Commit c125c46

Browse files
jvalkealjvalkeal
committed
Add short options into valid tokens
- Add short options as valid tokens in a `CommandModel` so that Lexer create tokens with accurate info and doesn't then cascade this issue in Ast and Parser. - Previously with a command `command -a aaa -b bbb` tokenisation resulted `COMMAND OPTION ARGUMENT ARGUMENT ARGUMENT` with multiple short options while it should have been `COMMAND OPTION ARGUMENT OPTION ARGUMENT`. - Relates spring-projects#757
1 parent 8cba636 commit c125c46

File tree

5 files changed

+276
-0
lines changed

5 files changed

+276
-0
lines changed

‎spring-shell-core/src/main/java/org/springframework/shell/command/parser/CommandModel.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,9 @@ Map<String, Token> getValidTokens() {
196196
for (String longName : commandOption.getLongNames()) {
197197
tokens.put("--" + longName, new Token(longName, TokenType.OPTION));
198198
}
199+
for (Character shortName : commandOption.getShortNames()) {
200+
tokens.put("-" + shortName, new Token(shortName.toString(), TokenType.OPTION));
201+
}
199202
});
200203
}
201204
return tokens;

‎spring-shell-core/src/test/java/org/springframework/shell/command/parser/AbstractParsingTests.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,19 @@ abstract class AbstractParsingTests {
110110
.and()
111111
.build();
112112

113+
static final CommandRegistration ROOT3_OPTION_ARG1_ARG2 = CommandRegistration.builder()
114+
.command("root3")
115+
.withOption()
116+
.longNames("arg1")
117+
.and()
118+
.withOption()
119+
.longNames("arg2")
120+
.and()
121+
.withTarget()
122+
.consumer(ctx -> {})
123+
.and()
124+
.build();
125+
113126
static final CommandRegistration ROOT3_SHORT_OPTION_A = CommandRegistration.builder()
114127
.command("root3")
115128
.withOption()
@@ -120,6 +133,34 @@ abstract class AbstractParsingTests {
120133
.and()
121134
.build();
122135

136+
static final CommandRegistration ROOT3_SHORT_OPTION_A_B = CommandRegistration.builder()
137+
.command("root3")
138+
.withOption()
139+
.shortNames('a')
140+
.and()
141+
.withOption()
142+
.shortNames('b')
143+
.and()
144+
.withTarget()
145+
.consumer(ctx -> {})
146+
.and()
147+
.build();
148+
149+
static final CommandRegistration ROOT3_SHORT_OPTION_A_B_REQUIRED = CommandRegistration.builder()
150+
.command("root3")
151+
.withOption()
152+
.shortNames('a')
153+
.required()
154+
.and()
155+
.withOption()
156+
.required()
157+
.shortNames('b')
158+
.and()
159+
.withTarget()
160+
.consumer(ctx -> {})
161+
.and()
162+
.build();
163+
123164
static final CommandRegistration ROOT4 = CommandRegistration.builder()
124165
.command("root4")
125166
.withOption()

‎spring-shell-core/src/test/java/org/springframework/shell/command/parser/AstTests.java

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,102 @@ void createsOptionNodeWithOptionArg() {
137137
});
138138
}
139139

140+
@Test
141+
void createsOptionNodesWithTwoOptionArg() {
142+
register(ROOT3);
143+
Token root3 = token("root3", TokenType.COMMAND, 0);
144+
Token arg1 = token("--arg1", TokenType.OPTION, 1);
145+
Token value1 = token("value1", TokenType.ARGUMENT, 2);
146+
Token arg2 = token("--arg2", TokenType.OPTION, 3);
147+
Token value2 = token("value2", TokenType.ARGUMENT, 4);
148+
AstResult result = ast(root3, arg1, value1, arg2, value2);
149+
150+
assertThat(result).isNotNull();
151+
assertThat(result.nonterminalNodes()).hasSize(1);
152+
assertThat(result.nonterminalNodes().get(0)).isInstanceOf(CommandNode.class);
153+
assertThat(result.nonterminalNodes().get(0)).satisfies(n -> {
154+
CommandNode cn = (CommandNode)n;
155+
assertThat(cn.getCommand()).isEqualTo("root3");
156+
assertThat(cn.getChildren()).hasSize(2);
157+
assertThat(cn.getChildren())
158+
.filteredOn(on -> on instanceof OptionNode)
159+
.extracting(on -> {
160+
return ((OptionNode)on).getName();
161+
})
162+
.containsExactly("--arg1", "--arg2");
163+
OptionNode on1 = (OptionNode) cn.getChildren().get(0);
164+
assertThat(on1.getChildren()).hasSize(1);
165+
OptionArgumentNode oan1 = (OptionArgumentNode) on1.getChildren().get(0);
166+
assertThat(oan1.getValue()).isEqualTo("value1");
167+
OptionNode on2 = (OptionNode) cn.getChildren().get(1);
168+
assertThat(on2.getChildren()).hasSize(1);
169+
OptionArgumentNode oan2 = (OptionArgumentNode) on2.getChildren().get(0);
170+
assertThat(oan2.getValue()).isEqualTo("value2");
171+
});
172+
}
173+
174+
@Test
175+
void createsOptionNodeWithShortOptionArg() {
176+
register(ROOT3_SHORT_OPTION_A);
177+
Token root3 = token("root3", TokenType.COMMAND, 0);
178+
Token arg1 = token("-a", TokenType.OPTION, 1);
179+
Token value1 = token("value1", TokenType.ARGUMENT, 2);
180+
AstResult result = ast(root3, arg1, value1);
181+
182+
assertThat(result).isNotNull();
183+
assertThat(result.nonterminalNodes()).hasSize(1);
184+
assertThat(result.nonterminalNodes().get(0)).isInstanceOf(CommandNode.class);
185+
assertThat(result.nonterminalNodes().get(0)).satisfies(n -> {
186+
CommandNode cn = (CommandNode)n;
187+
assertThat(cn.getCommand()).isEqualTo("root3");
188+
assertThat(cn.getChildren()).hasSize(1);
189+
assertThat(cn.getChildren())
190+
.filteredOn(on -> on instanceof OptionNode)
191+
.extracting(on -> {
192+
return ((OptionNode)on).getName();
193+
})
194+
.containsExactly("-a");
195+
OptionNode on = (OptionNode) cn.getChildren().get(0);
196+
assertThat(on.getChildren()).hasSize(1);
197+
OptionArgumentNode oan = (OptionArgumentNode) on.getChildren().get(0);
198+
assertThat(oan.getValue()).isEqualTo("value1");
199+
});
200+
}
201+
202+
@Test
203+
void createsOptionNodesWithTwoShortOptionArg() {
204+
register(ROOT3_SHORT_OPTION_A_B);
205+
Token root3 = token("root3", TokenType.COMMAND, 0);
206+
Token arg1 = token("-a", TokenType.OPTION, 1);
207+
Token value1 = token("value1", TokenType.ARGUMENT, 2);
208+
Token arg2 = token("-b", TokenType.OPTION, 3);
209+
Token value2 = token("value2", TokenType.ARGUMENT, 4);
210+
AstResult result = ast(root3, arg1, value1, arg2, value2);
211+
212+
assertThat(result).isNotNull();
213+
assertThat(result.nonterminalNodes()).hasSize(1);
214+
assertThat(result.nonterminalNodes().get(0)).isInstanceOf(CommandNode.class);
215+
assertThat(result.nonterminalNodes().get(0)).satisfies(n -> {
216+
CommandNode cn = (CommandNode)n;
217+
assertThat(cn.getCommand()).isEqualTo("root3");
218+
assertThat(cn.getChildren()).hasSize(2);
219+
assertThat(cn.getChildren())
220+
.filteredOn(on -> on instanceof OptionNode)
221+
.extracting(on -> {
222+
return ((OptionNode)on).getName();
223+
})
224+
.containsExactly("-a", "-b");
225+
OptionNode on1 = (OptionNode) cn.getChildren().get(0);
226+
assertThat(on1.getChildren()).hasSize(1);
227+
OptionArgumentNode oan1 = (OptionArgumentNode) on1.getChildren().get(0);
228+
assertThat(oan1.getValue()).isEqualTo("value1");
229+
OptionNode on2 = (OptionNode) cn.getChildren().get(1);
230+
assertThat(on2.getChildren()).hasSize(1);
231+
OptionArgumentNode oan2 = (OptionArgumentNode) on2.getChildren().get(0);
232+
assertThat(oan2.getValue()).isEqualTo("value2");
233+
});
234+
}
235+
140236
@Test
141237
void createOptionNodesWhenNoOptionArguments() {
142238
register(ROOT3);

‎spring-shell-core/src/test/java/org/springframework/shell/command/parser/LexerTests.java

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,70 @@ void optionsWithValuesFromRoot() {
336336
});
337337
}
338338

339+
@Test
340+
void shortOptionWithValuesFromRoot() {
341+
register(ROOT3_SHORT_OPTION_A);
342+
List<Token> tokens = tokenize("root3", "-a", "value1");
343+
344+
assertThat(tokens).satisfiesExactly(
345+
token -> {
346+
ParserAssertions.assertThat(token)
347+
.isType(TokenType.COMMAND)
348+
.hasPosition(0)
349+
.hasValue("root3");
350+
},
351+
token -> {
352+
ParserAssertions.assertThat(token)
353+
.isType(TokenType.OPTION)
354+
.hasPosition(1)
355+
.hasValue("-a");
356+
},
357+
token -> {
358+
ParserAssertions.assertThat(token)
359+
.isType(TokenType.ARGUMENT)
360+
.hasPosition(2)
361+
.hasValue("value1");
362+
});
363+
}
364+
365+
@Test
366+
void shortOptionsWithValuesFromRoot() {
367+
register(ROOT3_SHORT_OPTION_A_B);
368+
List<Token> tokens = tokenize("root3", "-a", "value1", "-b", "value2");
369+
370+
assertThat(tokens).satisfiesExactly(
371+
token -> {
372+
ParserAssertions.assertThat(token)
373+
.isType(TokenType.COMMAND)
374+
.hasPosition(0)
375+
.hasValue("root3");
376+
},
377+
token -> {
378+
ParserAssertions.assertThat(token)
379+
.isType(TokenType.OPTION)
380+
.hasPosition(1)
381+
.hasValue("-a");
382+
},
383+
token -> {
384+
ParserAssertions.assertThat(token)
385+
.isType(TokenType.ARGUMENT)
386+
.hasPosition(2)
387+
.hasValue("value1");
388+
},
389+
token -> {
390+
ParserAssertions.assertThat(token)
391+
.isType(TokenType.OPTION)
392+
.hasPosition(3)
393+
.hasValue("-b");
394+
},
395+
token -> {
396+
ParserAssertions.assertThat(token)
397+
.isType(TokenType.ARGUMENT)
398+
.hasPosition(4)
399+
.hasValue("value2");
400+
});
401+
}
402+
339403
@Test
340404
void optionValueFromRoot() {
341405
register(ROOT3);

‎spring-shell-core/src/test/java/org/springframework/shell/command/parser/ParserTests.java

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,26 @@ void shouldFindLongOptionArgument() {
251251
);
252252
assertThat(result.messageResults()).isEmpty();
253253
}
254+
255+
@Test
256+
void shouldFindTwoLongOptionArgument() {
257+
register(ROOT3_OPTION_ARG1_ARG2);
258+
ParseResult result = parse("root3", "--arg1", "value1", "--arg2", "value2");
259+
assertThat(result).isNotNull();
260+
assertThat(result.commandRegistration()).isNotNull();
261+
assertThat(result.optionResults()).isNotEmpty();
262+
assertThat(result.optionResults()).satisfiesExactly(
263+
r -> {
264+
assertThat(r.option().getLongNames()).isEqualTo(new String[] { "arg1" });
265+
assertThat(r.value()).isEqualTo("value1");
266+
},
267+
r -> {
268+
assertThat(r.option().getLongNames()).isEqualTo(new String[] { "arg2" });
269+
assertThat(r.value()).isEqualTo("value2");
270+
}
271+
);
272+
assertThat(result.messageResults()).isEmpty();
273+
}
254274
}
255275

256276
@Nested
@@ -263,6 +283,58 @@ void shouldFindShortOption() {
263283
assertThat(result).isNotNull();
264284
assertThat(result.commandRegistration()).isNotNull();
265285
assertThat(result.optionResults()).isNotEmpty();
286+
assertThat(result.optionResults()).satisfiesExactly(
287+
r -> {
288+
assertThat(r.option().getShortNames()).isEqualTo(new Character[] { 'a' });
289+
assertThat(r.value()).isNull();
290+
}
291+
);
292+
assertThat(result.messageResults()).isEmpty();
293+
}
294+
295+
@Test
296+
void shouldFindShortOptionWithArg() {
297+
register(ROOT3_SHORT_OPTION_A);
298+
ParseResult result = parse("root3", "-a", "aaa");
299+
assertThat(result).isNotNull();
300+
assertThat(result.commandRegistration()).isNotNull();
301+
assertThat(result.optionResults()).isNotEmpty();
302+
assertThat(result.optionResults()).satisfiesExactly(
303+
r -> {
304+
assertThat(r.option().getShortNames()).isEqualTo(new Character[] { 'a' });
305+
assertThat(r.value()).isEqualTo("aaa");
306+
}
307+
);
308+
assertThat(result.messageResults()).isEmpty();
309+
}
310+
311+
@Test
312+
void shouldFindShortOptions() {
313+
register(ROOT3_SHORT_OPTION_A_B);
314+
ParseResult result = parse("root3", "-a", "aaa", "-b", "bbb");
315+
assertThat(result).isNotNull();
316+
assertThat(result.commandRegistration()).isNotNull();
317+
assertThat(result.optionResults()).isNotEmpty();
318+
assertThat(result.optionResults()).satisfiesExactly(
319+
r -> {
320+
assertThat(r.option().getShortNames()).isEqualTo(new Character[] { 'a' });
321+
assertThat(r.value()).isEqualTo("aaa");
322+
},
323+
r -> {
324+
assertThat(r.option().getShortNames()).isEqualTo(new Character[] { 'b' });
325+
assertThat(r.value()).isEqualTo("bbb");
326+
}
327+
);
328+
assertThat(result.messageResults()).isEmpty();
329+
}
330+
331+
@Test
332+
void shouldFindShortOptionsRequired() {
333+
register(ROOT3_SHORT_OPTION_A_B_REQUIRED);
334+
ParseResult result = parse("root3", "-a", "aaa", "-b", "bbb");
335+
assertThat(result).isNotNull();
336+
assertThat(result.commandRegistration()).isNotNull();
337+
assertThat(result.optionResults()).isNotEmpty();
266338
assertThat(result.messageResults()).isEmpty();
267339
}
268340

0 commit comments

Comments
 (0)