Fix positional args with new parser

- Better alignment for use of positional args without using
  an option.
- Should work similarly with old parsing framework.
- Change position where missing options are checked.
- Fixes #701

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

@@ -17,6 +17,7 @@ package org.springframework.shell.command.parser;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashSet;
 import java.util.List;
@@ -162,7 +163,6 @@ public interface Parser {
 			List<MessageResult> messageResults = new ArrayList<>();
 			if (registration != null) {
 				messageResults.addAll(commonMessageResults);
-				messageResults.addAll(validateOptionNotMissing(registration));
 				messageResults.addAll(validateOptionIsValid(registration));
 
 
@@ -182,7 +182,6 @@ public interface Parser {
 				List<CommandOption> optionsForArguments = registration.getOptions().stream()
 					.filter(o -> !resolvedOptions1.contains(o))
 					.filter(o -> o.getPosition() > -1)
-					.filter(o -> o.getArityMin() > -1)
 					.sorted(Comparator.comparingInt(o -> o.getPosition()))
 					.collect(Collectors.toList());
 
@@ -193,7 +192,11 @@ public interface Parser {
 
 				int i = 0;
 				for (CommandOption o : optionsForArguments) {
-					int j = i + o.getArityMax();
+					int aMax = o.getArityMax();
+					if (aMax < 0) {
+						aMax = optionsForArguments.size() == 1 ? Integer.MAX_VALUE : 1;
+					}
+					int j = i + aMax;
 					j = Math.min(argumentValues.size(), j);
 
 					List<String> asdf = argumentValues.subList(i, j);
@@ -211,6 +214,8 @@ public interface Parser {
 					i = j;
 				}
 
+				// can only validate after optionResults has been populated
+				messageResults.addAll(validateOptionNotMissing(registration));
 			}
 
 			return new ParseResult(registration, optionResults, argumentResults, messageResults, directiveResults);
@@ -369,11 +374,24 @@ public interface Parser {
 				.filter(o -> o.isRequired())
 				.collect(Collectors.toCollection(() -> new HashSet<>()));
 
-			optionResults.stream().map(or -> or.option()).forEach(o -> {
-				requiredOptions.remove(o);
-			});
+			List<String> argumentResultValues = argumentResults.stream().map(ar -> ar.value).collect(Collectors.toList());
+			optionResults.stream()
+				.filter(or -> or.value() != null)
+				.map(or -> or.option())
+				.forEach(o -> {
+					requiredOptions.remove(o);
+				});
+			Set<CommandOption> requiredOptions2 = requiredOptions.stream()
+				.filter(o -> {
+					if (argumentResultValues.isEmpty()) {
+						return true;
+					}
+					List<String> longNames = Arrays.asList(o.getLongNames());
+					return !Collections.disjoint(argumentResultValues, longNames);
+				})
+				.collect(Collectors.toSet());
 
-			return requiredOptions.stream()
+			return requiredOptions2.stream()
 				.map(o -> {
 					String ln = o.getLongNames() != null
 							? Stream.of(o.getLongNames()).collect(Collectors.joining(","))

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

@@ -181,6 +181,35 @@ abstract class AbstractParsingTests {
 			.and()
 		.build();
 
+	static final CommandRegistration ROOT7_POSITIONAL_ONE_ARG_STRING = CommandRegistration.builder()
+		.command("root7")
+		.withOption()
+			.longNames("arg1")
+			.type(String.class)
+			.position(0)
+			.and()
+		.withTarget()
+			.consumer(ctx -> {})
+			.and()
+		.build();
+
+	static final CommandRegistration ROOT7_POSITIONAL_TWO_ARG_STRING = CommandRegistration.builder()
+		.command("root7")
+		.withOption()
+			.longNames("arg1")
+			.type(String.class)
+			.position(0)
+			.and()
+		.withOption()
+			.longNames("arg2")
+			.type(String.class)
+			.position(1)
+			.and()
+		.withTarget()
+			.consumer(ctx -> {})
+			.and()
+		.build();
+
 	Map<String, CommandRegistration> registrations = new HashMap<>();
 
 	@BeforeEach

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

@@ -136,14 +136,17 @@ class ParserTests extends AbstractParsingTests {
 		@Test
 		void shouldHaveErrorResult2() {
 			register(ROOT4);
-			// ParseResult result = parse("root4", "--arg1", "value1", "--arg2", "value2");
 			ParseResult result = parse("root4", "--arg1", "--arg2");
 			assertThat(result).isNotNull();
-			assertThat(result.messageResults()).satisfiesExactly(message -> {
-				ParserAssertions.assertThat(message.parserMessage()).hasCode(2001).hasType(ParserMessage.Type.ERROR);
-			});
-			// "101E:(pos 0): Unrecognised option '--arg2'"
-			// assertThat(result.messageResults().get(0).getMessage()).contains("xxx");
+
+			assertThat(result.messageResults()).satisfiesExactlyInAnyOrder(
+				message -> {
+					ParserAssertions.assertThat(message.parserMessage()).hasCode(2000).hasType(ParserMessage.Type.ERROR);
+				},
+				message -> {
+					ParserAssertions.assertThat(message.parserMessage()).hasCode(2001).hasType(ParserMessage.Type.ERROR);
+				}
+			);
 		}
 
 		@Test
@@ -339,4 +342,37 @@ class ParserTests extends AbstractParsingTests {
 			);
 		}
 	}
+
+	@Nested
+	class Positional {
+
+		@Test
+		void shouldGetPositionalArgWhenOneAsString() {
+			register(ROOT7_POSITIONAL_ONE_ARG_STRING);
+			ParseResult result = parse("root7", "a");
+			assertThat(result.argumentResults()).satisfiesExactly(
+				r -> {
+					assertThat(r.value()).isEqualTo("a");
+					assertThat(r.position()).isEqualTo(0);
+				}
+			);
+		}
+
+		@Test
+		void shouldGetPositionalArgWhenTwoAsString() {
+			register(ROOT7_POSITIONAL_TWO_ARG_STRING);
+			ParseResult result = parse("root7", "a", "b");
+			assertThat(result.argumentResults()).satisfiesExactly(
+				r -> {
+					assertThat(r.value()).isEqualTo("a");
+					assertThat(r.position()).isEqualTo(0);
+				},
+				r -> {
+					assertThat(r.value()).isEqualTo("b");
+					assertThat(r.position()).isEqualTo(1);
+				}
+			);
+		}
+	}
+
 }

spring-shell-samples/src/main/java/org/springframework/shell/samples/e2e/OptionTypeCommands.java

@@ -216,6 +216,7 @@ public class OptionTypeCommands {
 				.withOption()
 					.longNames("arg1")
 					.type(String.class)
+					.position(0)
 					.required()
 					.and()
 				.withTarget()

spring-shell-samples/src/test/java/org/springframework/shell/samples/e2e/OptionTypeCommandsTests.java

@@ -36,6 +36,13 @@ class OptionTypeCommandsTests extends AbstractSampleTests {
 		assertScreenContainsText(session, "Hello hi");
 	}
 
+	@ParameterizedTest
+	@E2ESource(command = "option-type-string hi")
+	void optionTypeStringPositional(String command, boolean interactive) {
+		BaseShellSession<?> session = createSession(command, interactive);
+		assertScreenContainsText(session, "Hello hi");
+	}
+
 	@ParameterizedTest
 	@E2ESource(command = "option-type-boolean", reg = false)
 	void optionTypeBooleanWithAnno(String command, boolean interactive) {