Skip to content

Commit f93d8c3

Browse files
committed
Fix inconsistent unknown option parsing
- Fix an issue where unknown option is wrongly lexed as an argument if it's later than an known valid option. - Essentially if `arg1` is only valid option `--arg1 value1 --arg2 value2` would not complain about `arg2`. - Backport #1120 - Fixes #1122
1 parent 1dbb35a commit f93d8c3

File tree

4 files changed

+76
-2
lines changed

4 files changed

+76
-2
lines changed

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

+7-1
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,13 @@ else if (isLastTokenOfType(tokenList, TokenType.COMMAND)) {
222222
}
223223
}
224224
else if (isLastTokenOfType(tokenList, TokenType.ARGUMENT)) {
225-
tokenList.add(Token.of(argument, TokenType.ARGUMENT, i2));
225+
int decuceArgumentStyle = decuceArgumentStyle(argument);
226+
if (decuceArgumentStyle < 0) {
227+
tokenList.add(Token.of(argument, TokenType.ARGUMENT, i2));
228+
}
229+
else {
230+
tokenList.add(Token.of(argument, TokenType.OPTION, i2));
231+
}
226232
}
227233

228234
}

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

+8-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023 the original author or authors.
2+
* Copyright 2023-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -416,4 +416,11 @@ ParseResult parse(ParserConfig config, String... arguments) {
416416
Parser parser = new Parser.DefaultParser(commandModel, lexer(config), ast, config);
417417
return parser.parse(Arrays.asList(arguments));
418418
}
419+
420+
record RegAndArgs(CommandRegistration reg, String... args) {
421+
static RegAndArgs of(CommandRegistration reg, String... args) {
422+
return new RegAndArgs(reg, args);
423+
}
424+
}
425+
419426
}

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

+32
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,38 @@ void notDefinedOptionShouldBeOptionAfterDefinedOption() {
274274
});
275275
}
276276

277+
@Test
278+
void notDefinedOptionShouldBeOptionAfterDefinedOptionHavingArgument() {
279+
register(ROOT3);
280+
List<Token> tokens = tokenize("root3", "--arg1", "value1", "--arg2");
281+
282+
assertThat(tokens).satisfiesExactly(
283+
token -> {
284+
ParserAssertions.assertThat(token)
285+
.isType(TokenType.COMMAND)
286+
.hasPosition(0)
287+
.hasValue("root3");
288+
},
289+
token -> {
290+
ParserAssertions.assertThat(token)
291+
.isType(TokenType.OPTION)
292+
.hasPosition(1)
293+
.hasValue("--arg1");
294+
},
295+
token -> {
296+
ParserAssertions.assertThat(token)
297+
.isType(TokenType.ARGUMENT)
298+
.hasPosition(2)
299+
.hasValue("value1");
300+
},
301+
token -> {
302+
ParserAssertions.assertThat(token)
303+
.isType(TokenType.OPTION)
304+
.hasPosition(3)
305+
.hasValue("--arg2");
306+
});
307+
}
308+
277309
@Test
278310
void optionsWithoutValuesFromRoot() {
279311
register(ROOT5);

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

+29
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,12 @@
1515
*/
1616
package org.springframework.shell.command.parser;
1717

18+
import java.util.stream.Stream;
19+
1820
import org.junit.jupiter.api.Nested;
1921
import org.junit.jupiter.api.Test;
22+
import org.junit.jupiter.params.ParameterizedTest;
23+
import org.junit.jupiter.params.provider.MethodSource;
2024

2125
import org.springframework.shell.command.parser.Parser.ParseResult;
2226
import org.springframework.shell.command.parser.ParserConfig.Feature;
@@ -161,6 +165,31 @@ void shouldHaveErrorResult2() {
161165
);
162166
}
163167

168+
static Stream<RegAndArgs> shouldHaveErrorForUnrecognisedOption() {
169+
return Stream.of(
170+
RegAndArgs.of(ROOT1, "root1", "--arg1"),
171+
RegAndArgs.of(ROOT3, "root3", "--arg2", "--arg1"),
172+
RegAndArgs.of(ROOT3, "root3", "--arg1", "--arg2"),
173+
RegAndArgs.of(ROOT3, "root3", "--arg2", "fake1", "--arg1"),
174+
RegAndArgs.of(ROOT3, "root3", "--arg1", "--arg2", "fake1"),
175+
RegAndArgs.of(ROOT3, "root3", "--arg2", "fake1", "--arg1", "fake2"),
176+
RegAndArgs.of(ROOT3, "root3", "--arg1", "fake2", "--arg2", "fake1")
177+
);
178+
}
179+
180+
@ParameterizedTest
181+
@MethodSource
182+
void shouldHaveErrorForUnrecognisedOption(RegAndArgs regAndArgs) {
183+
register(regAndArgs.reg());
184+
ParseResult result = parse(regAndArgs.args());
185+
assertThat(result.messageResults()).satisfiesExactlyInAnyOrder(
186+
message -> {
187+
ParserAssertions.assertThat(message.parserMessage()).hasCode(2001).hasType(ParserMessage.Type.ERROR);
188+
}
189+
);
190+
191+
}
192+
164193
@Test
165194
void lexerMessageShouldGetPropagated() {
166195
ParseResult parse = parse("--");

0 commit comments

Comments
 (0)