Skip to content

Commit

Permalink
Fix inconsistent unknown option parsing
Browse files Browse the repository at this point in the history
- 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`.
- Fixes #1120
  • Loading branch information
jvalkeal committed Aug 19, 2024
1 parent b099398 commit 7165e2d
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,13 @@ else if (isLastTokenOfType(tokenList, TokenType.COMMAND)) {
}
}
else if (isLastTokenOfType(tokenList, TokenType.ARGUMENT)) {
tokenList.add(Token.of(argument, TokenType.ARGUMENT, i2));
int decuceArgumentStyle = decuceArgumentStyle(argument);
if (decuceArgumentStyle < 0) {
tokenList.add(Token.of(argument, TokenType.ARGUMENT, i2));
}
else {
tokenList.add(Token.of(argument, TokenType.OPTION, i2));
}
}

}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 the original author or authors.
* Copyright 2023-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -416,4 +416,11 @@ ParseResult parse(ParserConfig config, String... arguments) {
Parser parser = new Parser.DefaultParser(commandModel, lexer(config), ast, config);
return parser.parse(Arrays.asList(arguments));
}

record RegAndArgs(CommandRegistration reg, String... args) {
static RegAndArgs of(CommandRegistration reg, String... args) {
return new RegAndArgs(reg, args);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,38 @@ void notDefinedOptionShouldBeOptionAfterDefinedOption() {
});
}

@Test
void notDefinedOptionShouldBeOptionAfterDefinedOptionHavingArgument() {
register(ROOT3);
List<Token> tokens = tokenize("root3", "--arg1", "value1", "--arg2");

assertThat(tokens).satisfiesExactly(
token -> {
ParserAssertions.assertThat(token)
.isType(TokenType.COMMAND)
.hasPosition(0)
.hasValue("root3");
},
token -> {
ParserAssertions.assertThat(token)
.isType(TokenType.OPTION)
.hasPosition(1)
.hasValue("--arg1");
},
token -> {
ParserAssertions.assertThat(token)
.isType(TokenType.ARGUMENT)
.hasPosition(2)
.hasValue("value1");
},
token -> {
ParserAssertions.assertThat(token)
.isType(TokenType.OPTION)
.hasPosition(3)
.hasValue("--arg2");
});
}

@Test
void optionsWithoutValuesFromRoot() {
register(ROOT5);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@
*/
package org.springframework.shell.command.parser;

import java.util.stream.Stream;

import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import org.springframework.shell.command.parser.Parser.ParseResult;
import org.springframework.shell.command.parser.ParserConfig.Feature;
Expand Down Expand Up @@ -161,6 +165,31 @@ void shouldHaveErrorResult2() {
);
}

static Stream<RegAndArgs> shouldHaveErrorForUnrecognisedOption() {
return Stream.of(
RegAndArgs.of(ROOT1, "root1", "--arg1"),
RegAndArgs.of(ROOT3, "root3", "--arg2", "--arg1"),
RegAndArgs.of(ROOT3, "root3", "--arg1", "--arg2"),
RegAndArgs.of(ROOT3, "root3", "--arg2", "fake1", "--arg1"),
RegAndArgs.of(ROOT3, "root3", "--arg1", "--arg2", "fake1"),
RegAndArgs.of(ROOT3, "root3", "--arg2", "fake1", "--arg1", "fake2"),
RegAndArgs.of(ROOT3, "root3", "--arg1", "fake2", "--arg2", "fake1")
);
}

@ParameterizedTest
@MethodSource
void shouldHaveErrorForUnrecognisedOption(RegAndArgs regAndArgs) {
register(regAndArgs.reg());
ParseResult result = parse(regAndArgs.args());
assertThat(result.messageResults()).satisfiesExactlyInAnyOrder(
message -> {
ParserAssertions.assertThat(message.parserMessage()).hasCode(2001).hasType(ParserMessage.Type.ERROR);
}
);

}

@Test
void lexerMessageShouldGetPropagated() {
ParseResult parse = parse("--");
Expand Down

0 comments on commit 7165e2d

Please sign in to comment.