Add unrecognised option support

- This commit modifies CommandParser to better track positional parameters
  which previously used to go there for non-recognised options. Now using
  relatively dump logic of just checking if first positional parameter starts
  with '-' which indicates it's a candidate for a new `UnrecognisedOptionException`
  which then would give user an error "Unrecognised option '--xxx'" for example.
- Backport #601
- Backport #602
- Fixes #603
- Fixes #604

Author: jvalkeal

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

@@ -227,7 +227,15 @@ public CommandParserResults parse(List<CommandOption> options, String[] args) {
 					requiredOptions.remove(pr.option);
 				}
 				else {
-					positional.addAll(pr.args);
+					for (String arg : pr.args) {
+						if (arg.startsWith("-")) {
+							errors.add(UnrecognisedOptionException.of(String.format("Unrecognised option '%s'", arg),
+									arg));
+						}
+						else {
+							positional.add(arg);
+						}
+					}
 				}
 				if (pr.error != null) {
 					errors.add(pr.error);
@@ -258,7 +266,9 @@ public CommandParserResults parse(List<CommandOption> options, String[] args) {
 							}
 						}
 					}
-					if (!oargs.isEmpty()) {
+					// don't do anything if first arg looks like an option as if we are here
+					// then we'd might remove wrong required option
+					if (!oargs.isEmpty() && !oargs.get(0).startsWith("-")) {
 						results.add(new DefaultCommandParserResult(o, oargs.stream().collect(Collectors.joining(" "))));
 						requiredOptions.remove(o);
 					}
@@ -516,4 +526,22 @@ public CommandOption getOption() {
 			return option;
 		}
 	}
+
+	public static class UnrecognisedOptionException extends CommandParserException {
+
+		private String option;
+
+		public UnrecognisedOptionException(String message, String option) {
+			super(message);
+			this.option = option;
+		}
+
+		public static UnrecognisedOptionException of(String message, String option) {
+			return new UnrecognisedOptionException(message, option);
+		}
+
+		public String getOption() {
+			return option;
+		}
+	}
 }

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

@@ -365,6 +365,65 @@ public void testIntegerWithGivenValue() {
 		assertThat(results.results().get(0).value()).isEqualTo(1);
 	}
 
+	@Test
+	public void testNotDefinedLongOptionWithoutOptions() {
+		// gh-602
+		List<CommandOption> options = Arrays.asList();
+		String[] args = new String[]{"--arg1", "foo"};
+		CommandParserResults results = parser.parse(options, args);
+		assertThat(results.results()).hasSize(0);
+		assertThat(results.errors()).hasSize(1);
+		assertThat(results.positional()).hasSize(1);
+	}
+
+	@Test
+	public void testNotDefinedLongOptionWithOptionalOption() {
+		// gh-602
+		CommandOption option1 = longOption("arg1");
+		List<CommandOption> options = Arrays.asList(option1);
+		String[] args = new String[]{"--arg1", "bar", "--arg2", "foo"};
+		CommandParserResults results = parser.parse(options, args);
+		assertThat(results.results()).hasSize(1);
+		assertThat(results.errors()).hasSize(1);
+		assertThat(results.positional()).hasSize(1);
+	}
+
+	@Test
+	public void testNotDefinedLongOptionWithRequiredOption() {
+		// gh-602
+		CommandOption option1 = longOption("arg1", true);
+		List<CommandOption> options = Arrays.asList(option1);
+		String[] args = new String[]{"--arg2", "foo"};
+		CommandParserResults results = parser.parse(options, args);
+		assertThat(results.results()).hasSize(0);
+		assertThat(results.errors()).hasSize(2);
+		assertThat(results.positional()).hasSize(1);
+	}
+
+	@Test
+	public void testPositionDoesNotAffectRequiredErrorWithOtherErrors() {
+		// gh-601
+		CommandOption o1 = CommandOption.of(
+				new String[] { "arg1" },
+				null,
+				null,
+				null,
+				true,
+				null,
+				0,
+				1,
+				1,
+				null,
+				null);
+
+		List<CommandOption> options = Arrays.asList(o1);
+		String[] args = new String[]{"--arg2"};
+		CommandParserResults results = parser.parse(options, args);
+		assertThat(results.results()).hasSize(0);
+		assertThat(results.errors()).hasSize(2);
+		assertThat(results.positional()).hasSize(0);
+	}
+
 	private static CommandOption longOption(String name) {
 		return longOption(name, null);
 	}

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

@@ -0,0 +1,73 @@
+/*
+ * Copyright 2023 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.shell.samples.e2e;
+
+import java.util.function.Supplier;
+
+import org.springframework.context.annotation.Bean;
+import org.springframework.shell.command.CommandRegistration;
+import org.springframework.shell.standard.ShellComponent;
+import org.springframework.shell.standard.ShellMethod;
+import org.springframework.shell.standard.ShellOption;
+
+@ShellComponent
+public class UnrecognisedOptionCommands extends BaseE2ECommands {
+
+	@ShellMethod(key = LEGACY_ANNO + "unrecognised-option-noother", group = GROUP)
+	public String testUnrecognisedOptionNoOtherAnnotation(
+	) {
+		return "Hi";
+	}
+
+	@Bean
+	public CommandRegistration testUnrecognisedOptionNoOtherRegistration(Supplier<CommandRegistration.Builder> builder) {
+		return builder.get()
+			.command(REG, "unrecognised-option-noother")
+			.group(GROUP)
+			.withTarget()
+				.function(ctx -> {
+					return "Hi";
+				})
+				.and()
+			.build();
+	}
+
+	@ShellMethod(key = LEGACY_ANNO + "unrecognised-option-withrequired", group = GROUP)
+	public String testUnrecognisedOptionWithRequiredAnnotation(
+		@ShellOption(help = "Desc arg1") String arg1
+	) {
+		return "Hello " + arg1;
+	}
+
+	@Bean
+	public CommandRegistration testUnrecognisedOptionWithRequiredRegistration(Supplier<CommandRegistration.Builder> builder) {
+		return builder.get()
+			.command(REG, "unrecognised-option-withrequired")
+			.group(GROUP)
+			.withOption()
+				.longNames("arg1")
+				.description("Desc arg1")
+				.required()
+				.and()
+			.withTarget()
+				.function(ctx -> {
+					String arg1 = ctx.getOptionValue("arg1");
+					return "Hello " + arg1;
+				})
+				.and()
+			.build();
+	}
+}