Add better support for modified option names

- Longnames in a command option if modified via
  name modifier didn't provide enough backmapping
  info for command execution experience being accurate.
- Add new getLongNamesModified() into CommandOption
  which is populated if name modifier is used.
- Add more hints in CommandExecution for modified
  option names.
- This should bring annotation
  and programmatic commands up to date.
- Backport #777
- Fixes #782

Author: jvalkeal

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2022 the original author or authors.
+ * Copyright 2022-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.
@@ -148,8 +148,9 @@ private Optional<CommandParserResult> find(String name) {
 			return results.results().stream()
 				.filter(r -> {
 					Stream<String> l = Arrays.asList(r.option().getLongNames()).stream();
+					Stream<String> lm = Arrays.asList(r.option().getLongNamesModified()).stream();
 					Stream<String> s = Arrays.asList(r.option().getShortNames()).stream().map(n -> Character.toString(n));
-					return Stream.concat(l, s)
+					return Stream.concat(Stream.concat(l, lm), s)
 						.filter(o -> ObjectUtils.nullSafeEquals(o, name))
 						.findFirst()
 						.isPresent();

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

@@ -198,6 +198,11 @@ else if (targetInfo.getTargetType() == TargetType.METHOD) {
 								messageBuilder.setHeader(ArgumentHeaderMethodArgumentResolver.ARGUMENT_PREFIX + n, r.value());
 								paramValues.put(n, r.value());
 							}
+							// need to provide backmapping for orinal names which were modified
+							for (String n : r.option().getLongNamesModified()) {
+								messageBuilder.setHeader(ArgumentHeaderMethodArgumentResolver.ARGUMENT_PREFIX + n, r.value());
+								paramValues.put(n, r.value());
+							}
 						}
 						if (r.option().getShortNames() != null) {
 							for (Character n : r.option().getShortNames()) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2022 the original author or authors.
+ * Copyright 2022-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.
@@ -32,6 +32,14 @@ public interface CommandOption {
 	 */
 	String[] getLongNames();
 
+	/**
+	 * Gets a modified long names of an option. Set within a command registration if
+	 * option name modifier were used to have an info about original names.
+	 *
+	 * @return modified long names of an option
+	 */
+	String[] getLongNamesModified();
+
 	/**
 	 * Gets a short names of an option.
 	 *
@@ -111,7 +119,7 @@ public interface CommandOption {
 	 * @return default command option
 	 */
 	public static CommandOption of(String[] longNames, Character[] shortNames, String description) {
-		return of(longNames, shortNames, description, null, false, null, null, null, null, null, null);
+		return of(longNames, null, shortNames, description, null, false, null, null, null, null, null, null);
 	}
 
 	/**
@@ -125,13 +133,14 @@ public static CommandOption of(String[] longNames, Character[] shortNames, Strin
 	 */
 	public static CommandOption of(String[] longNames, Character[] shortNames, String description,
 			ResolvableType type) {
-		return of(longNames, shortNames, description, type, false, null, null, null, null, null, null);
+		return of(longNames, null, shortNames, description, type, false, null, null, null, null, null, null);
 	}
 
 	/**
 	 * Gets an instance of a default {@link CommandOption}.
 	 *
 	 * @param longNames the long names
+	 * @param longNamesModified the modified long names
 	 * @param shortNames the short names
 	 * @param description the description
 	 * @param type the type
@@ -144,10 +153,10 @@ public static CommandOption of(String[] longNames, Character[] shortNames, Strin
 	 * @param completion the completion
 	 * @return default command option
 	 */
-	public static CommandOption of(String[] longNames, Character[] shortNames, String description,
+	public static CommandOption of(String[] longNames, String[] longNamesModified, Character[] shortNames, String description,
 			ResolvableType type, boolean required, String defaultValue, Integer position, Integer arityMin,
 			Integer arityMax, String label, CompletionResolver completion) {
-		return new DefaultCommandOption(longNames, shortNames, description, type, required, defaultValue, position,
+		return new DefaultCommandOption(longNames, longNamesModified, shortNames, description, type, required, defaultValue, position,
 				arityMin, arityMax, label, completion);
 	}
 
@@ -157,6 +166,7 @@ public static CommandOption of(String[] longNames, Character[] shortNames, Strin
 	public static class DefaultCommandOption implements CommandOption {
 
 		private String[] longNames;
+		private String[] longNamesModified;
 		private Character[] shortNames;
 		private String description;
 		private ResolvableType type;
@@ -168,11 +178,12 @@ public static class DefaultCommandOption implements CommandOption {
 		private String label;
 		private CompletionResolver completion;
 
-		public DefaultCommandOption(String[] longNames, Character[] shortNames, String description,
+		public DefaultCommandOption(String[] longNames, String[] longNamesModified, Character[] shortNames, String description,
 				ResolvableType type, boolean required, String defaultValue, Integer position,
 				Integer arityMin, Integer arityMax, String label,
 				CompletionResolver completion) {
 			this.longNames = longNames != null ? longNames : new String[0];
+			this.longNamesModified = longNamesModified != null ? longNamesModified : new String[0];
 			this.shortNames = shortNames != null ? shortNames : new Character[0];
 			this.description = description;
 			this.type = type;
@@ -190,6 +201,11 @@ public String[] getLongNames() {
 			return longNames;
 		}
 
+		@Override
+		public String[] getLongNamesModified() {
+			return longNamesModified;
+		}
+
 		@Override
 		public Character[] getShortNames() {
 			return shortNames;

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

@@ -1245,11 +1245,13 @@ public List<CommandOption> getOptions() {
 			options = optionSpecs.stream()
 				.map(o -> {
 					String[] longNames = o.getLongNames();
+					String[] longNamesModified = null;
 					Function<String, String> modifier = o.getOptionNameModifier();
 					if (modifier != null) {
+						longNamesModified = Arrays.copyOf(longNames, longNames.length);
 						longNames = Arrays.stream(longNames).map(modifier).toArray(String[]::new);
 					}
-					return CommandOption.of(longNames, o.getShortNames(), o.getDescription(), o.getType(),
+					return CommandOption.of(longNames, longNamesModified, o.getShortNames(), o.getDescription(), o.getType(),
 							o.isRequired(), o.getDefaultValue(), o.getPosition(), o.getArityMin(), o.getArityMax(),
 							o.getLabel(), o.getCompletion());
 					})

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

@@ -539,4 +539,21 @@ public void testCommandNotAvailable() {
 		Object result = execution.evaluate(r1, new String[]{"--arg1", "myarg1value"});
 		assertThat(result).isInstanceOf(CommandNotCurrentlyAvailable.class);
 	}
+
+	@Test
+	public void testExecutionWithModifiedLongOption() {
+		CommandRegistration r1 = CommandRegistration.builder()
+			.command("command1")
+			.withOption()
+				.longNames("arg1")
+				.nameModifier(orig -> "x" + orig)
+				.and()
+			.withTarget()
+				.function(function1)
+				.and()
+			.build();
+		Object result = execution.evaluate(r1, new String[] { "command1", "--xarg1", "myarg1value" });
+		assertThat(result).isEqualTo("himyarg1value");
+	}
+
 }

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

@@ -351,6 +351,7 @@ public void testArityErrors() {
 				new String[] { "arg1" },
 				null,
 				null,
+				null,
 				ResolvableType.forType(int[].class),
 				true,
 				null,
@@ -385,6 +386,7 @@ public void testMapToIntArray() {
 				new String[] { "arg1" },
 				null,
 				null,
+				null,
 				ResolvableType.forType(int[].class),
 				false,
 				null,
@@ -439,6 +441,7 @@ public void testMapPositionalArgsWhenTypeList() {
 				new String[] { "arg1" },
 				null,
 				null,
+				null,
 				ResolvableType.forType(List.class),
 				true,
 				null,
@@ -474,7 +477,7 @@ public void testMapPositionalArgs2() {
 	@Test
 	public void testBooleanWithDefault() {
 		ResolvableType type = ResolvableType.forType(boolean.class);
-		CommandOption option1 = CommandOption.of(new String[] { "arg1" }, new Character[0], "description", type, false,
+		CommandOption option1 = CommandOption.of(new String[] { "arg1" }, null, new Character[0], "description", type, false,
 				"true", null, null, null, null, null);
 
 		List<CommandOption> options = Arrays.asList(option1);
@@ -488,7 +491,7 @@ public void testBooleanWithDefault() {
 	@Test
 	public void testIntegerWithDefault() {
 		ResolvableType type = ResolvableType.forType(Integer.class);
-		CommandOption option1 = CommandOption.of(new String[] { "arg1" }, new Character[0], "description", type, false,
+		CommandOption option1 = CommandOption.of(new String[] { "arg1" }, null, new Character[0], "description", type, false,
 				"1", null, null, null, null, null);
 
 		List<CommandOption> options = Arrays.asList(option1);
@@ -502,7 +505,7 @@ public void testIntegerWithDefault() {
 	@Test
 	public void testIntegerWithGivenValue() {
 		ResolvableType type = ResolvableType.forType(Integer.class);
-		CommandOption option1 = CommandOption.of(new String[] { "arg1" }, new Character[0], "description", type, false,
+		CommandOption option1 = CommandOption.of(new String[] { "arg1" }, null, new Character[0], "description", type, false,
 				null, null, null, null, null, null);
 
 		List<CommandOption> options = Arrays.asList(option1);
@@ -586,6 +589,7 @@ public void testPositionDoesNotAffectRequiredErrorWithOtherErrors() {
 				null,
 				null,
 				null,
+				null,
 				true,
 				null,
 				0,
@@ -623,7 +627,7 @@ private static CommandOption longOption(String name, ResolvableType type, boolea
 	}
 
 	private static CommandOption longOption(String name, ResolvableType type, boolean required, Integer position, Integer arityMin, Integer arityMax) {
-		return CommandOption.of(new String[] { name }, new Character[0], "desc", type, required, null, position,
+		return CommandOption.of(new String[] { name }, null, new Character[0], "desc", type, required, null, position,
 				arityMin, arityMax, null, null);
 	}
 

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

@@ -28,12 +28,13 @@ public class OptionNamingCommands {
 	public static class LegacyAnnotation extends BaseE2ECommands {
 
 		@ShellMethod(key = LEGACY_ANNO + "option-naming-1", group = GROUP)
-		public void testOptionNaming1Annotation(
+		public String testOptionNaming1Annotation(
 			@ShellOption("from_snake") String snake,
 			@ShellOption("fromCamel") String camel,
 			@ShellOption("from-kebab") String kebab,
 			@ShellOption("FromPascal") String pascal
 		) {
+			return String.format("snake='%s' camel='%s' kebab='%s' pascal='%s' ", snake, camel, kebab, pascal);
 		}
 
 	}
@@ -65,10 +66,16 @@ public CommandRegistration testOptionNaming1Registration() {
 				.withOption()
 					.longNames("arg1")
 					.nameModifier(name -> "x" + name)
-					.required()
+					// .required()
 					.and()
 				.withTarget()
-					.consumer(ctx -> {})
+					.function(ctx -> {
+						String snake = ctx.getOptionValue("from_snake");
+						String camel = ctx.getOptionValue("fromCamel");
+						String kebab = ctx.getOptionValue("from-kebab");
+						String pascal = ctx.getOptionValue("FromPascal");
+						return String.format("snake='%s' camel='%s' kebab='%s' pascal='%s' ", snake, camel, kebab, pascal);
+					})
 					.and()
 				.build();
 		}