From 5ba8e185bca0390d702b52d715d5561d98645ac2 Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Thu, 14 Jul 2022 20:21:52 +0100 Subject: [PATCH] Fix method argument without ShellOption - For annotated methods with arguments, change default arity to zero with booleans and one everything else regardless if @ShellOption is defined or not. - OptionArity.ZERO_OR_ONE had wrong upperbound value, change from MAX to 1. - These modification should take us a bit closer to old shell functionality and what ShellOption documents for arity. - For old functionality I'm referring to method `add(int a, int b)` and/or having @ShellOption and/or without arity setting. - Fixes #446 --- .../shell/command/CommandRegistration.java | 2 +- .../StandardMethodTargetRegistrar.java | 26 +++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/spring-shell-core/src/main/java/org/springframework/shell/command/CommandRegistration.java b/spring-shell-core/src/main/java/org/springframework/shell/command/CommandRegistration.java index 5660ad44..7cf769ad 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/command/CommandRegistration.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/command/CommandRegistration.java @@ -608,7 +608,7 @@ public interface CommandRegistration { break; case ZERO_OR_ONE: this.arityMin = 0; - this.arityMax = Integer.MAX_VALUE; + this.arityMax = 1; break; case EXACTLY_ONE: this.arityMin = 1; diff --git a/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java b/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java index 08599402..b36a0293 100644 --- a/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java +++ b/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java @@ -42,6 +42,7 @@ import org.springframework.shell.Utils; import org.springframework.shell.command.CommandCatalog; import org.springframework.shell.command.CommandRegistration; import org.springframework.shell.command.CommandRegistration.Builder; +import org.springframework.shell.command.CommandRegistration.OptionArity; import org.springframework.shell.command.CommandRegistration.OptionSpec; import org.springframework.shell.completion.CompletionResolver; import org.springframework.shell.standard.ShellOption.NoValueProvider; @@ -131,8 +132,9 @@ public class StandardMethodTargetRegistrar implements MethodTargetRegistrar, App } if (!longNames.isEmpty() || !shortNames.isEmpty()) { log.debug("Registering longNames='{}' shortNames='{}'", longNames, shortNames); + Class parameterType = mp.getParameterType(); OptionSpec optionSpec = builder.withOption() - .type(mp.getParameterType()) + .type(parameterType) .longNames(longNames.toArray(new String[0])) .shortNames(shortNames.toArray(new Character[0])) .position(mp.getParameterIndex()) @@ -140,6 +142,17 @@ public class StandardMethodTargetRegistrar implements MethodTargetRegistrar, App if (so.arity() > -1) { optionSpec.arity(0, so.arity()); } + else { + if (ClassUtils.isAssignable(boolean.class, parameterType)) { + optionSpec.arity(OptionArity.ZERO); + } + else if (ClassUtils.isAssignable(Boolean.class, parameterType)) { + optionSpec.arity(OptionArity.ZERO); + } + else { + optionSpec.arity(OptionArity.EXACTLY_ONE); + } + } if (!ObjectUtils.nullSafeEquals(so.defaultValue(), ShellOption.NONE) && !ObjectUtils.nullSafeEquals(so.defaultValue(), ShellOption.NULL)) { optionSpec.defaultValue(so.defaultValue()); @@ -163,11 +176,20 @@ public class StandardMethodTargetRegistrar implements MethodTargetRegistrar, App Class parameterType = mp.getParameterType(); if (longName != null) { log.debug("Using mp='{}' longName='{}' parameterType='{}'", mp, longName, parameterType); - builder.withOption() + OptionSpec optionSpec = builder.withOption() .longNames(longName) .type(parameterType) .required() .position(mp.getParameterIndex()); + if (ClassUtils.isAssignable(boolean.class, parameterType)) { + optionSpec.arity(OptionArity.ZERO); + } + else if (ClassUtils.isAssignable(Boolean.class, parameterType)) { + optionSpec.arity(OptionArity.ZERO); + } + else { + optionSpec.arity(OptionArity.EXACTLY_ONE); + } } } }