Revisit positional arguments

- Add better mapping logic
- Add better type conversion
- More docs for arity and positional option configuration
- Fixes #616
This commit is contained in:
Janne Valkealahti
2023-01-13 15:11:38 +00:00
parent b10786814a
commit 95e2829dc3
8 changed files with 266 additions and 20 deletions

View File

@@ -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.
@@ -269,7 +269,10 @@ public interface CommandParser {
// 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(" "))));
// as we now have a candicate option, try to see if there is a
// conversion we can do and the use it.
Object value = convertOptionType(o, oargs);
results.add(new DefaultCommandParserResult(o, value));
requiredOptions.remove(o);
}
});
@@ -285,6 +288,15 @@ public interface CommandParser {
return new DefaultCommandParserResults(results, positional, errors);
}
private Object convertOptionType(CommandOption option, Object value) {
if (conversionService != null && option.getType() != null && value != null) {
if (conversionService.canConvert(value.getClass(), option.getType().getRawClass())) {
value = conversionService.convert(value, option.getType().getRawClass());
}
}
return value;
}
private static class ParserResult {
private CommandOption option;
private List<String> args;
@@ -335,12 +347,7 @@ public interface CommandParser {
if (holder.error != null) {
return Stream.of(ParserResult.of(o, subArgs, null, holder.error));
}
Object value = holder.value;
if (conversionService != null && o.getType() != null && value != null) {
if (conversionService.canConvert(value.getClass(), o.getType().getRawClass())) {
value = conversionService.convert(value, o.getType().getRawClass());
}
}
Object value = convertOptionType(o, holder.value);
Stream<ParserResult> unmapped = holder.unmapped.stream()
.map(um -> ParserResult.of(null, Arrays.asList(um), null, null));
Stream<ParserResult> res = Stream.of(ParserResult.of(o, subArgs, value, null));

View File

@@ -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.
@@ -60,6 +60,8 @@ public abstract class AbstractCommandTests {
public int method7Arg3;
public int method8Count;
public float[] method8Arg1;
public int method9Count;
public String[] method9Arg1;
public void method1(CommandContext ctx) {
method1Ctx = ctx;
@@ -107,5 +109,10 @@ public abstract class AbstractCommandTests {
method8Arg1 = arg1;
method8Count++;
}
public void method9(String[] arg1) {
method9Arg1 = arg1;
method9Count++;
}
}
}

View File

@@ -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.
@@ -195,7 +195,47 @@ public class CommandExecutionTests extends AbstractCommandTests {
.build();
execution.evaluate(r1, new String[]{"myarg1value1", "myarg1value2"});
assertThat(pojo1.method4Count).isEqualTo(1);
assertThat(pojo1.method4Arg1).isEqualTo("myarg1value1 myarg1value2");
assertThat(pojo1.method4Arg1).isEqualTo("myarg1value1,myarg1value2");
}
@Test
public void testMethodMultiPositionalArgsAllToArray1() {
CommandRegistration r1 = CommandRegistration.builder()
.command("command1")
.description("help")
.withOption()
.longNames("arg1")
.description("some arg1")
.position(0)
.arity(OptionArity.ONE_OR_MORE)
.and()
.withTarget()
.method(pojo1, "method9")
.and()
.build();
execution.evaluate(r1, new String[]{"myarg1value1", "myarg1value2"});
assertThat(pojo1.method9Count).isEqualTo(1);
assertThat(pojo1.method9Arg1).isEqualTo(new String[] { "myarg1value1", "myarg1value2" });
}
@Test
public void testMethodMultiPositionalArgsAllToArray2() {
CommandRegistration r1 = CommandRegistration.builder()
.command("command1")
.description("help")
.withOption()
.longNames("arg1")
.description("some arg1")
.position(0)
.arity(OptionArity.ONE_OR_MORE)
.and()
.withTarget()
.method(pojo1, "method8")
.and()
.build();
execution.evaluate(r1, new String[]{"1", "2"});
assertThat(pojo1.method8Count).isEqualTo(1);
assertThat(pojo1.method8Arg1).isEqualTo(new float[] { 1, 2 });
}
@Test

View File

@@ -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.
@@ -198,7 +198,8 @@ public class CommandParserTests extends AbstractCommandTests {
CommandParserResults results = parser.parse(options, args);
assertThat(results.results()).hasSize(1);
assertThat(results.results().get(0).option()).isSameAs(option1);
assertThat(results.results().get(0).value()).isEqualTo("value foo");
// no type so we get raw list
assertThat(results.results().get(0).value()).isEqualTo(Arrays.asList("value", "foo"));
assertThat(results.positional()).containsExactly("value", "foo");
}
@@ -210,7 +211,7 @@ public class CommandParserTests extends AbstractCommandTests {
CommandParserResults results = parser.parse(options, args);
assertThat(results.results()).hasSize(1);
assertThat(results.results().get(0).option()).isSameAs(option1);
assertThat(results.results().get(0).value()).isEqualTo("value foo");
assertThat(results.results().get(0).value()).isEqualTo("value,foo");
assertThat(results.positional()).containsExactly("value", "foo");
}
@@ -331,10 +332,35 @@ public class CommandParserTests extends AbstractCommandTests {
assertThat(results2.results().get(0).value()).isNull();
}
@Test
public void testMapToIntArray() {
CommandOption option1 = CommandOption.of(
new String[] { "arg1" },
null,
null,
ResolvableType.forType(int[].class),
false,
null,
0,
1,
2,
null,
null);
List<CommandOption> options = Arrays.asList(option1);
String[] args = new String[]{"1", "2"};
CommandParserResults results = parser.parse(options, args);
assertThat(results.errors()).hasSize(0);
assertThat(results.results()).hasSize(1);
assertThat(results.results().get(0).option()).isSameAs(option1);
assertThat(results.results().get(0).value()).isEqualTo(new int[] { 1, 2 });
}
@Test
public void testMapPositionalArgs1() {
CommandOption option1 = longOption("arg1", 0, 1, 1);
CommandOption option2 = longOption("arg2", 1, 1, 2);
CommandOption option1 = longOption("arg1", ResolvableType.forType(String.class), false, 0, 1, 1);
CommandOption option2 = longOption("arg2", ResolvableType.forType(String.class), false, 1, 1, 2);
List<CommandOption> options = Arrays.asList(option1, option2);
String[] args = new String[]{"--arg1", "1", "2"};
CommandParserResults results = parser.parse(options, args);
@@ -346,9 +372,25 @@ public class CommandParserTests extends AbstractCommandTests {
}
@Test
public void testMapPositionalArgs2() {
public void testMapPositionalArgs11() {
CommandOption option1 = longOption("arg1", 0, 1, 1);
CommandOption option2 = longOption("arg2", 1, 1, 2);
List<CommandOption> options = Arrays.asList(option1, option2);
String[] args = new String[]{"--arg1", "1", "2"};
CommandParserResults results = parser.parse(options, args);
assertThat(results.results()).hasSize(2);
assertThat(results.results().get(0).option()).isSameAs(option1);
assertThat(results.results().get(1).option()).isSameAs(option2);
assertThat(results.results().get(0).value()).isEqualTo("1");
// no type so we get raw list
assertThat(results.results().get(1).value()).isEqualTo(Arrays.asList("2"));
}
@Test
public void testMapPositionalArgs2() {
CommandOption option1 = longOption("arg1", ResolvableType.forType(String.class), false, 0, 1, 1);
CommandOption option2 = longOption("arg2", ResolvableType.forType(String.class), false, 1, 1, 2);
List<CommandOption> options = Arrays.asList(option1, option2);
String[] args = new String[]{"1", "2"};
CommandParserResults results = parser.parse(options, args);

View File

@@ -4,7 +4,7 @@ ifndef::snippets[:snippets: ../../test/java/org/springframework/shell/docs]
Sometimes, you want to have more fine control of how many parameters with an option
are processed when parsing operations happen. Arity is defined as min and max
values, where min must be a positive integer and max has to be more or equal to min.
values, where min must be zero or a positive integer and max has to be more or equal to min.
====
[source, java, indent=0]
@@ -14,7 +14,7 @@ include::{snippets}/OptionSnippets.java[tag=option-registration-arityints]
====
Arity can also be defined as an `OptionArity` enum, which are shortcuts
within the following table:
shown in below table <<using-shell-options-arity-optionarity-table>>.
====
[source, java, indent=0]
@@ -23,6 +23,7 @@ include::{snippets}/OptionSnippets.java[tag=option-registration-arityenum]
----
====
[[using-shell-options-arity-optionarity-table]]
.OptionArity
|===
|Value |min/max
@@ -51,3 +52,35 @@ The annotation model supports defining only the max value of an arity.
include::{snippets}/OptionSnippets.java[tag=option-with-annotation-arity]
----
====
One of a use cases to manually define arity is to impose restrictions how
many parameters option accepts.
====
[source, java, indent=0]
----
include::{snippets}/OptionSnippets.java[tag=option-registration-aritystrings-sample]
----
====
In above example we have option _arg1_ and it's defined as type _String[]_. Arity
defines that it needs at least 1 parameter and not more that 2. As seen in below
spesific exceptions _TooManyArgumentsOptionException_ and
_NotEnoughArgumentsOptionException_ are thrown to indicate arity mismatch.
====
[source, bash]
----
shell:>e2e reg arity-errors --arg1
Not enough arguments --arg1 requires at least 1.
shell:>e2e reg arity-errors --arg1 one
Hello [one]
shell:>e2e reg arity-errors --arg1 one two
Hello [one, two]
shell:>e2e reg arity-errors --arg1 one two three
Too many arguments --arg1 requires at most 2.
----
====

View File

@@ -10,3 +10,61 @@ Positional information is mostly related to a command target method:
include::{snippets}/OptionSnippets.java[tag=option-registration-positional]
----
====
NOTE: Be careful with positional parameters as it may soon
become confusing which options those are mapped to.
Usually arguments are mapped to an option when those are defined in a
command line whether it's a long or short option. Generally speaking
there are _options_, _option arguments_ and _arguments_ where latter
are the ones which are not mapped to any spesific option.
Unrecognised arguments can then have a secondary mapping logic where
positional information is important. With option position you're
essentially telling command parsing how to interpret plain raw
ambiguous arguments.
Let's look what happens when we don't define a position.
====
[source, java, indent=0]
----
include::{snippets}/OptionSnippets.java[tag=option-registration-aritystrings-noposition]
----
====
Option _arg1_ is required and there is no info what to do with argument
`one` resulting error for missing option.
====
[source, bash]
----
shell:>arity-strings-1 one
Missing mandatory option --arg1.
----
====
Now let's define a position `0`.
====
[source, java, indent=0]
----
include::{snippets}/OptionSnippets.java[tag=option-registration-aritystrings-position]
----
====
Arguments are processed until we get up to 2 arguments.
====
[source, bash]
----
shell:>arity-strings-2 one
Hello [one]
shell:>arity-strings-2 one two
Hello [one, two]
shell:>arity-strings-2 one two three
Hello [one, two]
----
====

View File

@@ -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.
@@ -15,6 +15,8 @@
*/
package org.springframework.shell.docs;
import java.util.Arrays;
import org.springframework.shell.command.CommandRegistration;
import org.springframework.shell.command.CommandRegistration.OptionArity;
import org.springframework.shell.standard.ShellOption;
@@ -144,6 +146,61 @@ public class OptionSnippets {
.build();
// end::option-registration-arityints[]
// tag::option-registration-aritystrings-sample[]
CommandRegistration.builder()
.command("arity-errors")
.withOption()
.longNames("arg1")
.type(String[].class)
.required()
.arity(1, 2)
.and()
.withTarget()
.function(ctx -> {
String[] arg1 = ctx.getOptionValue("arg1");
return "Hello " + Arrays.asList(arg1);
})
.and()
.build();
// end::option-registration-aritystrings-sample[]
// tag::option-registration-aritystrings-position[]
CommandRegistration.builder()
.command("arity-strings-2")
.withOption()
.longNames("arg1")
.required()
.type(String[].class)
.arity(0, 2)
.position(0)
.and()
.withTarget()
.function(ctx -> {
String[] arg1 = ctx.getOptionValue("arg1");
return "Hello " + Arrays.asList(arg1);
})
.and()
.build();
// end::option-registration-aritystrings-position[]
// tag::option-registration-aritystrings-noposition[]
CommandRegistration.builder()
.command("arity-strings-1")
.withOption()
.longNames("arg1")
.required()
.type(String[].class)
.arity(0, 2)
.and()
.withTarget()
.function(ctx -> {
String[] arg1 = ctx.getOptionValue("arg1");
return "Hello " + Arrays.asList(arg1);
})
.and()
.build();
// end::option-registration-aritystrings-noposition[]
// tag::option-registration-optional[]
CommandRegistration.builder()
.withOption()

View File

@@ -75,8 +75,10 @@ public class ArityCommands extends BaseE2ECommands {
.group(GROUP)
.withOption()
.longNames("arg1")
.required()
.type(String[].class)
.arity(0, 3)
.position(0)
.and()
.withTarget()
.function(ctx -> {