Fix issue with multiple positional args, update tests [ci fast]

Signed-off-by: Ben Sherman <[email protected]>

Author: bentsherman

modules/nextflow/src/main/groovy/nextflow/cli/v2/PluginCmd.groovy

@@ -41,10 +41,10 @@ class PluginCmd extends AbstractCmd {
     @ParentCommand
     private Launcher launcher
 
-    @Parameters
+    @Parameters(index = '0')
     String command
 
-    @Parameters
+    @Parameters(index = '1..*')
     List<String> args
 
     @Override

modules/nextflow/src/main/groovy/nextflow/cli/v2/RunCmd.groovy

@@ -52,10 +52,10 @@ class RunCmd extends AbstractCmd implements RunImpl.Options, HubOptions {
     @ParentCommand
     private Launcher launcher
 
-    @Parameters(description = 'Project name or repository url')
+    @Parameters(index = '0', description = 'Project name or repository url')
     String pipeline
 
-    @Parameters(description = 'Pipeline script args')
+    @Parameters(index = '1..*', description = 'Pipeline script args')
     List<String> args
 
     @Option(names = ['--ansi-log'], arity = '1', description = 'Use ANSI logging')
@@ -224,54 +224,88 @@ class RunCmd extends AbstractCmd implements RunImpl.Options, HubOptions {
     @Option(names = ['--with-weblog'], arity = '0..1', fallbackValue = '-', description = 'Send workflow status messages via HTTP to target URL')
     String withWebLog
 
-    @Parameters(description = 'Pipeline parameters')
-    List<String> params
+    private List<String> pipelineArgs = null
 
-    private Map<String,String> paramsMap = null
+    private Map<String,String> pipelineParams = null
 
     /**
-     * Get the pipeline params as a map.
+     * Parse the pipeline args and params from the positional
+     * args parsed by picocli. This method assumes that the first
+     * positional arg that starts with '--' is the first param,
+     * and parses the remaining args as params.
      *
-     * The double-dash ('--') notation is normally used to separate
-     * positional parameters from options. As a result, params will also
-     * contain the positional parameters of the `run` command (i.e. args),
-     * so they must be skipped when constructing the params map.
-     *
-     * This method assumes that params are specified as option-value pairs
-     * separated by a space. The equals-sign ('=') separator is not supported.
+     * NOTE: While the double-dash ('--') notation can be used to
+     * distinguish pipeline params from CLI options, it cannot be
+     * used to distinguish pipeline params from pipeline args.
      */
-    @Override
-    Map<String,String> getParams() {
-        if( paramsMap == null ) {
-            paramsMap = [:]
-
-            int i = args.size()
-            while( i < params.size() ) {
-                String current = params[i++]
-
-                String key
-                String value
-                if( current.contains('=') ) {
-                    int split = current.indexOf('=')
-                    key = current.substring(0, split)
-                    value = current.substring(split+1)
-                }
-                else if( i < params.size() && !params[i].startsWith('--') ) {
-                    key = current
-                    value = params[i++]
-                }
-                else {
-                    key = current
-                    value = 'true'
-                }
-
-                paramsMap.put(key, value)
+    private void parseArgs() {
+        // parse pipeline args
+        int i = args.findIndexOf { it.startsWith('--') }
+        pipelineArgs = args[0..<i]
+
+        // parse pipeline params
+        pipelineParams = [:]
+
+        if( i == -1 )
+            return
+
+        while( i < args.size() ) {
+            String current = args[i++]
+            if( !current.startsWith('--') ) {
+                throw new IllegalArgumentException("Invalid argument '${current}' -- unable to parse it as a pipeline arg, pipeline param, or CLI option")
+            }
+
+            String key
+            String value
+
+            // parse '--param=value'
+            if( current.contains('=') ) {
+                int split = current.indexOf('=')
+                key = current.substring(2, split)
+                value = current.substring(split+1)
+            }
+
+            // parse '--param value'
+            else if( i < args.size() && !args[i].startsWith('--') ) {
+                key = current.substring(2)
+                value = args[i++]
             }
 
-            log.trace "Parsing params from CLI: $paramsMap"
+            // parse '--param1 --param2 ...' as '--param1 true --param2 ...'
+            else {
+                key = current.substring(2)
+                value = 'true'
+            }
+
+            pipelineParams.put(key, value)
+        }
+
+        log.trace "Parsing pipeline args from CLI: $pipelineArgs"
+        log.trace "Parsing pipeline params from CLI: $pipelineParams"
+    }
+
+    /**
+     * Get the list of pipeline args.
+     */
+    @Override
+    List<String> getArgs() {
+        if( pipelineArgs == null ) {
+            parseArgs()
+        }
+
+        return pipelineArgs
+    }
+
+    /**
+     * Get the map of pipeline params.
+     */
+    @Override
+    Map<String,String> getParams() {
+        if( pipelineParams == null ) {
+            parseArgs()
         }
 
-        return paramsMap
+        return pipelineParams
     }
 
     @Override

modules/nextflow/src/test/groovy/nextflow/cli/HubOptionsTest.groovy

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2020-2022, Seqera Labs
+ * Copyright 2013-2019, Centre for Genomic Regulation (CRG)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package nextflow.cli
+
+import spock.lang.Specification
+
+/**
+ *
+ * @author Paolo Di Tommaso <[email protected]>
+ */
+class HubOptionsTest extends Specification {
+
+    def testUserV1() {
+
+        when:
+        def cmd = [:] as v1.HubOptions
+        cmd.hubUserCli = credential
+        then:
+        cmd.getHubUser() == user
+        cmd.getHubPassword() == password
+
+        where:
+        credential      | user     | password
+        null            | null     | null
+        'paolo'         | 'paolo'  | null
+        'paolo:secret'  | 'paolo'  | 'secret'
+
+    }
+
+    def testUserV2() {
+
+        when:
+        def cmd = [:] as v2.HubOptions
+        cmd.hubUserCli = credential
+        then:
+        cmd.getHubUser() == user
+        cmd.getHubPassword() == password
+
+        where:
+        credential      | user     | password
+        null            | null     | null
+        'paolo'         | 'paolo'  | null
+        'paolo:secret'  | 'paolo'  | 'secret'
+
+    }
+
+}

modules/nextflow/src/test/groovy/nextflow/cli/v1/LauncherTest.groovy

@@ -45,7 +45,6 @@ class LauncherTest extends Specification {
         then:
         assert launcher.options.fullVersion
 
-
     }
 
     def 'should return `help` command' () {
@@ -135,9 +134,11 @@ class LauncherTest extends Specification {
         launcher.command.hubProvider == 'github'
 
         when:
-        launcher = new Launcher().parseMainArgs('run', 'script.nf', '--alpha', '0', '--omega', '9')
+        launcher = new Launcher().parseMainArgs('run', 'script.nf', 'arg1', 'arg2', '--alpha', '0', '--omega', '9')
         then:
         launcher.command instanceof RunCmd
+        launcher.command.pipeline == 'script.nf'
+        launcher.command.args == ['arg1', 'arg2']
         launcher.command.params.'alpha' == '0'
         launcher.command.params.'omega' == '9'
 
@@ -357,4 +358,5 @@ class LauncherTest extends Specification {
         String opt4
 
     }
+
 }