feat(core): support positional args as dynamic args as well (#4621)

Signed-off-by: Olivier Cano <[email protected]>

Author: kindermoumoute

core/arg_specs.go

@@ -14,13 +14,11 @@ var AllLocalities = "all"
 
 type ArgSpecs []*ArgSpec
 
+// GetPositionalArg returns the last positional argument from the arg specs.
 func (s ArgSpecs) GetPositionalArg() *ArgSpec {
 	var positionalArg *ArgSpec
 	for _, argSpec := range s {
 		if argSpec.Positional {
-			if positionalArg != nil {
-				panic(fmt.Errorf("more than one positional parameter detected: %s and %s are flagged as positional arg", positionalArg.Name, argSpec.Name))
-			}
 			positionalArg = argSpec
 		}
 	}

core/cobra_utils.go

@@ -41,8 +41,11 @@ func cobraRun(ctx context.Context, cmd *Command) func(*cobra.Command, []string)
 
 		positionalArgSpec := cmd.ArgSpecs.GetPositionalArg()
 
-		// If this command has no positional argument we execute the run
-		if positionalArgSpec == nil {
+		// If this command has no positional argument or the positional arg is already passed, we execute the run
+		if positionalArgSpec == nil || rawArgs.Has(positionalArgSpec.Name) {
+			if positionalArgSpec != nil && rawArgs.Has(positionalArgSpec.Name) {
+				rawArgs = rawArgs.RemoveAllPositional()
+			}
 			data, err := run(ctx, cobraCmd, cmd, rawArgs)
 			if err != nil {
 				return err
@@ -55,22 +58,11 @@ func cobraRun(ctx context.Context, cmd *Command) func(*cobra.Command, []string)
 
 		positionalArgs := rawArgs.GetPositionalArgs()
 
-		// Return an error if a positional argument was provide using `key=value` syntax.
-		value, exist := rawArgs.Get(positionalArgSpec.Name)
-		if exist {
-			otherArgs := rawArgs.Remove(positionalArgSpec.Name)
-
-			return &CliError{
-				Err:  errors.New("a positional argument is required for this command"),
-				Hint: positionalArgHint(meta.BinaryName, cmd, value, otherArgs, len(positionalArgs) > 0),
-			}
-		}
-
 		// If no positional arguments were provided, return an error
 		if len(positionalArgs) == 0 {
 			return &CliError{
 				Err:  errors.New("a positional argument is required for this command"),
-				Hint: positionalArgHint(meta.BinaryName, cmd, "<"+positionalArgSpec.Name+">", rawArgs, false),
+				Hint: positionalArgHint(meta.BinaryName, cmd, "<"+positionalArgSpec.Name+">", rawArgs.Remove(positionalArgSpec.Name), false),
 			}
 		}
 

core/cobra_utils_test.go

@@ -17,6 +17,11 @@ type testType struct {
 	Tag    string
 }
 
+type testTypeManyTags struct {
+	NameID string
+	Tags   []string
+}
+
 type testDate struct {
 	Date *time.Time
 }
@@ -78,6 +83,45 @@ func testGetCommands() *core.Commands {
 				return argsI, nil
 			},
 		},
+		&core.Command{
+			Namespace: "test",
+			Resource:  "many-positional",
+			ArgSpecs: core.ArgSpecs{
+				{
+					Name:       "name-id",
+					Positional: true,
+				},
+				{
+					Name:       "tag",
+					Positional: true,
+				},
+			},
+			AllowAnonymousClient: true,
+			ArgsType:             reflect.TypeOf(testType{}),
+			Run: func(_ context.Context, argsI interface{}) (i interface{}, e error) {
+				return argsI, nil
+			},
+		},
+		&core.Command{
+			Namespace: "test",
+			Resource:  "many-multi-positional",
+			ArgSpecs: core.ArgSpecs{
+				{
+					Name:       "name-id",
+					Positional: true,
+				},
+				{
+					Name:       "tags",
+					Positional: true,
+				},
+			},
+			AcceptMultiplePositionalArgs: true,
+			AllowAnonymousClient:         true,
+			ArgsType:                     reflect.TypeOf(testTypeManyTags{}),
+			Run: func(_ context.Context, argsI interface{}) (i interface{}, e error) {
+				return argsI, nil
+			},
+		},
 		&core.Command{
 			Namespace:            "test",
 			Resource:             "raw-args",
@@ -196,42 +240,6 @@ func Test_PositionalArg(t *testing.T) {
 				}),
 			),
 		}))
-
-		t.Run("Invalid1", core.Test(&core.TestConfig{
-			Commands: testGetCommands(),
-			Cmd:      "scw test positional name-id=plop tag=world",
-			Check: core.TestCheckCombine(
-				core.TestCheckExitCode(1),
-				core.TestCheckError(&core.CliError{
-					Err:  errors.New("a positional argument is required for this command"),
-					Hint: "Try running: scw test positional plop tag=world",
-				}),
-			),
-		}))
-
-		t.Run("Invalid2", core.Test(&core.TestConfig{
-			Commands: testGetCommands(),
-			Cmd:      "scw test positional tag=world name-id=plop",
-			Check: core.TestCheckCombine(
-				core.TestCheckExitCode(1),
-				core.TestCheckError(&core.CliError{
-					Err:  errors.New("a positional argument is required for this command"),
-					Hint: "Try running: scw test positional plop tag=world",
-				}),
-			),
-		}))
-
-		t.Run("Invalid3", core.Test(&core.TestConfig{
-			Commands: testGetCommands(),
-			Cmd:      "scw test positional plop name-id=plop",
-			Check: core.TestCheckCombine(
-				core.TestCheckExitCode(1),
-				core.TestCheckError(&core.CliError{
-					Err:  errors.New("a positional argument is required for this command"),
-					Hint: "Try running: scw test positional plop",
-				}),
-			),
-		}))
 	})
 
 	t.Run("simple", core.Test(&core.TestConfig{
@@ -240,6 +248,24 @@ func Test_PositionalArg(t *testing.T) {
 		Check:    core.TestCheckExitCode(0),
 	}))
 
+	t.Run("simple2", core.Test(&core.TestConfig{
+		Commands: testGetCommands(),
+		Cmd:      "scw test positional name-id=plop tag=world",
+		Check:    core.TestCheckExitCode(0),
+	}))
+
+	t.Run("simple3", core.Test(&core.TestConfig{
+		Commands: testGetCommands(),
+		Cmd:      "scw test positional tag=world name-id=plop",
+		Check:    core.TestCheckExitCode(0),
+	}))
+
+	t.Run("simple4", core.Test(&core.TestConfig{
+		Commands: testGetCommands(),
+		Cmd:      "scw test positional plop name-id=plop",
+		Check:    core.TestCheckExitCode(0),
+	}))
+
 	t.Run("full command", core.Test(&core.TestConfig{
 		Commands: testGetCommands(),
 		Cmd:      "scw test positional plop tag=world",
@@ -272,6 +298,41 @@ func Test_PositionalArg(t *testing.T) {
 			core.TestCheckGolden(),
 		),
 	}))
+
+	t.Run("many positional", core.Test(&core.TestConfig{
+		Commands: testGetCommands(),
+		Cmd:      "scw test many-positional tag1 name-id=plop",
+		Check:    core.TestCheckExitCode(0),
+	}))
+
+	t.Run("many positional", core.Test(&core.TestConfig{
+		Commands: testGetCommands(),
+		Cmd:      "scw test many-positional tag1 name-id=plop",
+		Check: core.TestCheckCombine(
+			core.TestCheckExitCode(0),
+			func(t *testing.T, ctx *core.CheckFuncCtx) {
+				t.Helper()
+				res := ctx.Result.(*testType)
+				assert.Equal(t, "plop", res.NameID)
+				assert.Equal(t, "tag1", res.Tag)
+			},
+		),
+	}))
+
+	t.Run("many multi-positional", core.Test(&core.TestConfig{
+		Commands: testGetCommands(),
+		Cmd:      "scw test many-multi-positional pos1 pos2 name-id=plop",
+		Check: core.TestCheckCombine(
+			core.TestCheckExitCode(0),
+			func(t *testing.T, ctx *core.CheckFuncCtx) {
+				t.Helper()
+				res := ctx.Result.(*testTypeManyTags)
+				assert.Equal(t, "plop", res.NameID)
+				assert.Equal(t, "pos1", res.Tags[0])
+				assert.Equal(t, "pos2", res.Tags[1])
+			},
+		),
+	}))
 }
 
 func Test_MultiPositionalArg(t *testing.T) {