#676 bugfix: required should consider defaultValue _after_ variable i…

…nterpolation

Closes #676

RELEASE-NOTES.md

@@ -161,7 +161,7 @@ With the new execute API the ColorScheme class will start to play a more central
 - [#541] Improved exception handling for Runnable/Callable.
 - [#663] How to remove stacktraces on error. Thanks to [Nicolas Mingo](https:/nicolasmingo) and [jrevault](https:/jrevault) for raising this and subsequent discussion.
 - [#672] Need way to send errors back from subcommand. Thanks to [Garret Wilson](https:/garretwilson) for raising this.
-
+- [#676] Bugfix: non-defined variables in `defaultValue` now correctly resolve to `null`, and options and positional parameters are now correctly considered `required` only if their default value is `null` after variable interpolation. Thanks to [ifedorenko](https:/ifedorenko) for raising this.
 
 ## <a name="4.0.0-alpha-3-deprecated"></a> Deprecations
 All variants of the `run`, `call`, `invoke`, and `parseWithHandlers` methods are deprecated from this release, in favor of the new `execute` method.

src/main/java/picocli/CommandLine.java

@@ -4825,12 +4825,13 @@ public CommandSpec addPositional(PositionalParamSpec positional) {
  }
  private CommandSpec addArg(ArgSpec arg) {
  args.add(arg);
- if (arg.required() && arg.group() == null) { requiredArgs.add(arg); }
  arg.messages(usageMessage().messages());
  arg.commandSpec = this;
  if (arg.arity().isUnresolved()) {
  arg.arity = Range.valueOf(interpolator.interpolate(arg.arity().originalValue));
  }
+ // do this last: arg.required() needs to resolve variables in arg.defaultValue()
+ if (arg.required() && arg.group() == null) { requiredArgs.add(arg); }
  return this;
  }
 
@@ -6080,7 +6081,7 @@ private <T extends Builder<T>> ArgSpec(Builder<T> builder) {
  initialValue = builder.initialValue;
  hasInitialValue = builder.hasInitialValue;
  defaultValue = NO_DEFAULT_VALUE.equals(builder.defaultValue) ? null : builder.defaultValue;
- required = builder.required && defaultValue == null; //#261 not required if it has a default
+ required = builder.required;
  toString = builder.toString;
  getter = builder.getter;
  setter = builder.setter;
@@ -6130,11 +6131,13 @@ void applyInitialValue(Tracer tracer) {
  }
  }
 
- /** Returns whether this is a required option or positional parameter.
+ /** Returns whether this is a required option or positional parameter without a default value.
  * If this argument is part of a {@linkplain ArgGroup group}, this method returns whether this argument is required <em>within the group</em> (so it is not necessarily a required argument for the command).
  * @see Option#required() */
- public boolean required() { return required; }
-
+ public boolean required() {
+ //#261 not required if it has a default; #676 default value may be a variable
+ return required && defaultValue() == null && defaultValueFromProvider() == null;
+ }
  /** Returns whether this option will prompt the user to enter a value on the command line.
  * @see Option#interactive() */
  public boolean interactive() { return interactive; }

src/test/java/picocli/InterpolatedModelTest.java

@@ -6,6 +6,7 @@
 import org.junit.contrib.java.lang.system.RestoreSystemProperties;
 import org.junit.rules.TestRule;
 import picocli.CommandLine.Command;
+import picocli.CommandLine.MissingParameterException;
 import picocli.CommandLine.ParseResult;
 import picocli.CommandLine.Mixin;
 import picocli.CommandLine.Parameters;
@@ -265,6 +266,28 @@ void status(
  assertEquals(CommandLine.Range.valueOf("2..3"), status.getCommandSpec().findOption("x").arity());
  }
 
+ // https:/remkop/picocli/issues/676
+ @Test
+ public void testIssue676() {
+ class Issue676 {
+ @Option(names="--mypath", defaultValue = "${sys:MYPATH}", required = true)
+ private String path;
+ }
+ System.clearProperty("MYPATH");
+ Issue676 bean = new Issue676();
+ CommandLine cmd = new CommandLine(bean);
+ try {
+ cmd.parseArgs();
+ fail("Expected exception");
+ } catch (MissingParameterException ex) {
+ assertEquals("Missing required option '--mypath=<path>'", ex.getMessage());
+ }
+
+ System.setProperty("MYPATH", "abc");
+ cmd.parseArgs();
+ assertEquals("abc", bean.path);
+ }
+
  static class CommonMixinOne {
  @Parameters(index = "${sys:commonParam1}", paramLabel = "COMMON-PARAM-ONE")
  private String commonMixinOneParam;