Unfurl CLI

api/maven-api-cli/src/main/java/org/apache/maven/api/cli/InvokerRequest.java

@@ -50,7 +50,7 @@ public interface InvokerRequest {
     /**
      * Flag representing parser processing result: if there were some fatal errors during
      * {@link Parser#parseInvocation(ParserRequest)} this method will return {@code true} and invoker should
-     * handle this request as "early failure".
+     * handle this request as "early failure". In these cases, {@link #options()} usually is absent.
      */
     boolean parsingFailed();
 
@@ -195,16 +195,17 @@ default Optional<OutputStream> stdErr() {
     /**
      * Returns the options associated with this invocation request.
      *
-     * @return the options object
+     * @return the options optional. It will be absent if {@link #parsingFailed()} return {@code true}.
      */
     @Nonnull
-    Options options();
+    Optional<Options> options();
 
     /**
      * This method returns "verbose" option value derived from multiple places: CLI options, but also CI detection,
      * if applicable.
      */
     default boolean effectiveVerbose() {
-        return options().verbose().orElse(ciInfo().isPresent() && ciInfo().get().isVerbose());
+        return options().isPresent() && options().orElseThrow().verbose().orElse(false)
+                || ciInfo().isPresent() && ciInfo().orElseThrow().isVerbose();
     }
 }

api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java

@@ -20,7 +20,6 @@
 
 import java.util.List;
 import java.util.Optional;
-import java.util.function.UnaryOperator;
 
 import org.apache.maven.api.annotations.Experimental;
 import org.apache.maven.api.annotations.Nonnull;
@@ -221,14 +220,4 @@ public interface MavenOptions extends Options {
      */
     @Nonnull
     Optional<List<String>> goals();
-
-    /**
-     * Returns a new instance of {@link MavenOptions} with values interpolated using the given callback.
-     *
-     * @param callback a callback to use for interpolation
-     * @return a new MavenOptions instance with interpolated values
-     */
-    @Nonnull
-    @Override
-    MavenOptions interpolate(@Nonnull UnaryOperator<String> callback);
 }

api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvnenc/EncryptOptions.java

@@ -20,7 +20,6 @@
 
 import java.util.List;
 import java.util.Optional;
-import java.util.function.UnaryOperator;
 
 import org.apache.maven.api.annotations.Experimental;
 import org.apache.maven.api.annotations.Nonnull;
@@ -56,14 +55,4 @@ public interface EncryptOptions extends Options {
      */
     @Nonnull
     Optional<List<String>> goals();
-
-    /**
-     * Returns a new instance of EncryptOptions with values interpolated using the given properties.
-     *
-     * @param callback a callback to use for interpolation
-     * @return a new EncryptOptions instance with interpolated values
-     */
-    @Nonnull
-    @Override
-    EncryptOptions interpolate(UnaryOperator<String> callback);
 }

api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvnsh/ShellOptions.java

@@ -18,10 +18,7 @@
  */
 package org.apache.maven.api.cli.mvnsh;
 
-import java.util.function.UnaryOperator;
-
 import org.apache.maven.api.annotations.Experimental;
-import org.apache.maven.api.annotations.Nonnull;
 import org.apache.maven.api.cli.Options;
 
 /**
@@ -31,14 +28,4 @@
  * @since 4.0.0
  */
 @Experimental
-public interface ShellOptions extends Options {
-    /**
-     * Returns a new instance of ShellOptions with values interpolated using the given properties.
-     *
-     * @param callback a callback to use for interpolation
-     * @return a new EncryptOptions instance with interpolated values
-     */
-    @Nonnull
-    @Override
-    ShellOptions interpolate(UnaryOperator<String> callback);
-}
+public interface ShellOptions extends Options {}

api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvnup/UpgradeOptions.java

@@ -20,7 +20,6 @@
 
 import java.util.List;
 import java.util.Optional;
-import java.util.function.UnaryOperator;
 
 import org.apache.maven.api.annotations.Experimental;
 import org.apache.maven.api.annotations.Nonnull;
@@ -111,13 +110,4 @@ public interface UpgradeOptions extends Options {
      */
     @Nonnull
     Optional<Boolean> all();
-
-    /**
-     * Returns a new instance of UpgradeOptions with values interpolated using the given properties.
-     *
-     * @param callback a callback to use for interpolation
-     * @return a new UpgradeOptions instance with interpolated values
-     */
-    @Nonnull
-    UpgradeOptions interpolate(UnaryOperator<String> callback);
 }

impl/maven-cli/src/main/java/org/apache/maven/cling/MavenCling.java

@@ -75,7 +75,7 @@ public MavenCling(ClassWorld classWorld) {
     @Override
     protected Invoker createInvoker() {
         return new MavenInvoker(
-                ProtoLookup.builder().addMapping(ClassWorld.class, classWorld).build());
+                ProtoLookup.builder().addMapping(ClassWorld.class, classWorld).build(), null);
     }
 
     @Override

impl/maven-cli/src/main/java/org/apache/maven/cling/MavenEncCling.java

@@ -75,7 +75,7 @@ public MavenEncCling(ClassWorld classWorld) {
     @Override
     protected Invoker createInvoker() {
         return new EncryptInvoker(
-                ProtoLookup.builder().addMapping(ClassWorld.class, classWorld).build());
+                ProtoLookup.builder().addMapping(ClassWorld.class, classWorld).build(), null);
     }
 
     @Override

impl/maven-cli/src/main/java/org/apache/maven/cling/MavenShellCling.java

@@ -75,7 +75,7 @@ public MavenShellCling(ClassWorld classWorld) {
     @Override
     protected Invoker createInvoker() {
         return new ShellInvoker(
-                ProtoLookup.builder().addMapping(ClassWorld.class, classWorld).build());
+                ProtoLookup.builder().addMapping(ClassWorld.class, classWorld).build(), null);
     }
 
     @Override

impl/maven-cli/src/main/java/org/apache/maven/cling/MavenUpCling.java

@@ -75,7 +75,7 @@ public MavenUpCling(ClassWorld classWorld) {
     @Override
     protected Invoker createInvoker() {
         return new UpgradeInvoker(
-                ProtoLookup.builder().addMapping(ClassWorld.class, classWorld).build());
+                ProtoLookup.builder().addMapping(ClassWorld.class, classWorld).build(), null);
     }
 
     @Override

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseInvokerRequest.java

@@ -27,12 +27,13 @@
 import org.apache.maven.api.annotations.Nullable;
 import org.apache.maven.api.cli.CoreExtensions;
 import org.apache.maven.api.cli.InvokerRequest;
+import org.apache.maven.api.cli.Options;
 import org.apache.maven.api.cli.ParserRequest;
 import org.apache.maven.api.cli.cisupport.CIInfo;
 
 import static java.util.Objects.requireNonNull;
 
-public abstract class BaseInvokerRequest implements InvokerRequest {
+public class BaseInvokerRequest implements InvokerRequest {
     private final ParserRequest parserRequest;
     private final boolean parsingFailed;
     private final Path cwd;
@@ -44,6 +45,7 @@ public abstract class BaseInvokerRequest implements InvokerRequest {
     private final Path rootDirectory;
     private final List<CoreExtensions> coreExtensions;
     private final CIInfo ciInfo;
+    private final Options options;
 
     @SuppressWarnings("ParameterNumber")
     public BaseInvokerRequest(
@@ -57,7 +59,8 @@ public BaseInvokerRequest(
             @Nonnull Path topDirectory,
             @Nullable Path rootDirectory,
             @Nullable List<CoreExtensions> coreExtensions,
-            @Nullable CIInfo ciInfo) {
+            @Nullable CIInfo ciInfo,
+            @Nullable Options options) {
         this.parserRequest = requireNonNull(parserRequest);
         this.parsingFailed = parsingFailed;
         this.cwd = requireNonNull(cwd);
@@ -70,6 +73,7 @@ public BaseInvokerRequest(
         this.rootDirectory = rootDirectory;
         this.coreExtensions = coreExtensions;
         this.ciInfo = ciInfo;
+        this.options = options;
     }
 
     @Override
@@ -126,4 +130,8 @@ public Optional<List<CoreExtensions>> coreExtensions() {
     public Optional<CIInfo> ciInfo() {
         return Optional.ofNullable(ciInfo);
     }
+
+    public Optional<Options> options() {
+        return Optional.ofNullable(options);
+    }
 }

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java

@@ -91,6 +91,7 @@ public LocalContext(ParserRequest parserRequest) {
         @Nullable
         public CIInfo ciInfo;
 
+        @Nullable
         public Options options;
 
         public Map<String, String> extraInterpolationSource() {
@@ -149,24 +150,14 @@ public InvokerRequest parseInvocation(ParserRequest parserRequest) {
         }
 
         // options
-        List<Options> parsedOptions;
         try {
-            parsedOptions = parseCliOptions(context);
+            context.options = parseCliOptions(context);
         } catch (Exception e) {
             context.parsingFailed = true;
-            parsedOptions = List.of(emptyOptions());
+            context.options = null;
             parserRequest.logger().error("Error parsing program arguments", e);
         }
 
-        // assemble options if needed
-        try {
-            context.options = assembleOptions(parsedOptions);
-        } catch (Exception e) {
-            context.parsingFailed = true;
-            context.options = emptyOptions();
-            parserRequest.logger().error("Error assembling program arguments", e);
-        }
-
         // system and user properties
         try {
             context.systemProperties = populateSystemProperties(context);
@@ -184,8 +175,12 @@ public InvokerRequest parseInvocation(ParserRequest parserRequest) {
         }
 
         // options: interpolate
-        context.options = context.options.interpolate(Interpolator.chain(
-                context.extraInterpolationSource()::get, context.userProperties::get, context.systemProperties::get));
+        if (context.options != null) {
+            context.options = context.options.interpolate(Interpolator.chain(
+                    context.extraInterpolationSource()::get,
+                    context.userProperties::get,
+                    context.systemProperties::get));
+        }
 
         // core extensions
         try {
@@ -255,9 +250,21 @@ protected void failIfFileNotExists(LocalContext context, String fileName, String
         }
     }
 
-    protected abstract Options emptyOptions();
-
-    protected abstract InvokerRequest getInvokerRequest(LocalContext context);
+    protected InvokerRequest getInvokerRequest(LocalContext context) {
+        return new BaseInvokerRequest(
+                context.parserRequest,
+                context.parsingFailed,
+                context.cwd,
+                context.installationDirectory,
+                context.userHomeDirectory,
+                context.userProperties,
+                context.systemProperties,
+                context.topDirectory,
+                context.rootDirectory,
+                context.extensions,
+                context.ciInfo,
+                context.options);
+    }
 
     protected Path getCwd(LocalContext context) {
         if (context.parserRequest.cwd() != null) {
@@ -435,9 +442,7 @@ protected Map<String, String> populateUserProperties(LocalContext context) {
         return toMap(userProperties);
     }
 
-    protected abstract List<Options> parseCliOptions(LocalContext context);
-
-    protected abstract Options assembleOptions(List<Options> parsedOptions);
+    protected abstract Options parseCliOptions(LocalContext context);
 
     /**
      * Important: This method must return list of {@link CoreExtensions} in precedence order.

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/CommonsCliOptions.java

@@ -21,10 +21,13 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.ListIterator;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Consumer;
+import java.util.function.UnaryOperator;
 
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.DefaultParser;
@@ -34,12 +37,15 @@
 import org.apache.commons.cli.ParseException;
 import org.apache.maven.api.cli.Options;
 import org.apache.maven.api.cli.ParserRequest;
+import org.apache.maven.api.services.Interpolator;
+import org.apache.maven.api.services.InterpolatorException;
 import org.apache.maven.jline.MessageUtils;
 
 import static java.util.Objects.requireNonNull;
+import static org.apache.maven.cling.invoker.CliUtils.createInterpolator;
 import static org.apache.maven.cling.invoker.CliUtils.toMap;
 
-public abstract class CommonsCliOptions implements Options {
+public class CommonsCliOptions implements Options {
 
     protected final String source;
     protected final CLIManager cliManager;
@@ -250,6 +256,35 @@ public void warnAboutDeprecatedOptions(ParserRequest request, Consumer<String> p
         }
     }
 
+    @Override
+    public final Options interpolate(UnaryOperator<String> callback) {
+        try {
+            // now that we have properties, interpolate all arguments
+            Interpolator interpolator = createInterpolator();
+            CommandLine.Builder commandLineBuilder = CommandLine.builder();
+            commandLineBuilder.setDeprecatedHandler(o -> {});
+            for (Option option : commandLine.getOptions()) {
+                if (!CommonsCliOptions.CLIManager.USER_PROPERTY.equals(option.getOpt())) {
+                    List<String> values = option.getValuesList();
+                    for (ListIterator<String> it = values.listIterator(); it.hasNext(); ) {
+                        it.set(interpolator.interpolate(it.next(), callback));
+                    }
+                }
+                commandLineBuilder.addOption(option);
+            }
+            for (String arg : commandLine.getArgList()) {
+                commandLineBuilder.addArg(interpolator.interpolate(arg, callback));
+            }
+            return copy(source, cliManager, commandLineBuilder.build());
+        } catch (InterpolatorException e) {
+            throw new IllegalArgumentException("Could not interpolate CommonsCliOptions", e);
+        }
+    }
+
+    protected CommonsCliOptions copy(String source, CLIManager cliManager, CommandLine commandLine) {
+        return new CommonsCliOptions(source, cliManager, commandLine);
+    }
+
     @Override
     public void displayHelp(ParserRequest request, Consumer<String> printStream) {
         cliManager.displayHelp(request.command(), printStream);