fix(core): allow required nil values (#881)

* fix(core): allow required nil values

Signed-off-by: Patrik Cyvoct <[email protected]>

Author: Sh4d1

internal/args/args.go

@@ -15,6 +15,13 @@ const emptySliceValue = "none"
 // RawArgs allows to retrieve a simple []string using UnmarshalStruct()
 type RawArgs []string
 
+// ExistsArgByName checks if the given argument exists in the raw args
+func (a RawArgs) ExistsArgByName(name string) bool {
+	argsMap := SplitRawMap(a)
+	_, ok := argsMap[name]
+	return ok
+}
+
 var (
 	scalarKinds = map[reflect.Kind]bool{
 		reflect.Int:     true,

internal/core/cobra_utils.go

@@ -73,7 +73,7 @@ func cobraRun(ctx context.Context, cmd *Command) func(*cobra.Command, []string)
 		if cmd.ValidateFunc != nil {
 			validateFunc = cmd.ValidateFunc
 		}
-		err = validateFunc(cmd, cmdArgs)
+		err = validateFunc(cmd, cmdArgs, rawArgs)
 		if err != nil {
 			return err
 		}

internal/core/validate.go

@@ -3,6 +3,7 @@ package core
 import (
 	"fmt"
 	"reflect"
+	"strconv"
 	"strings"
 
 	"github.com/scaleway/scaleway-cli/internal/args"
@@ -13,19 +14,19 @@ import (
 
 // CommandValidateFunc validates en entire command.
 // Used in core.cobraRun().
-type CommandValidateFunc func(cmd *Command, cmdArgs interface{}) error
+type CommandValidateFunc func(cmd *Command, cmdArgs interface{}, rawArgs args.RawArgs) error
 
 // ArgSpecValidateFunc validates one argument of a command.
 type ArgSpecValidateFunc func(argSpec *ArgSpec, value interface{}) error
 
 // DefaultCommandValidateFunc is the default validation function for commands.
 func DefaultCommandValidateFunc() CommandValidateFunc {
-	return func(cmd *Command, cmdArgs interface{}) error {
+	return func(cmd *Command, cmdArgs interface{}, rawArgs args.RawArgs) error {
 		err := validateArgValues(cmd, cmdArgs)
 		if err != nil {
 			return err
 		}
-		err = validateRequiredArgs(cmd, cmdArgs)
+		err = validateRequiredArgs(cmd, cmdArgs, rawArgs)
 		if err != nil {
 			return err
 		}
@@ -59,8 +60,13 @@ func validateArgValues(cmd *Command, cmdArgs interface{}) error {
 // validateRequiredArgs checks for missing required args with no default value.
 // Returns an error for the first missing required arg.
 // Returns nil otherwise.
-func validateRequiredArgs(cmd *Command, cmdArgs interface{}) error {
+// TODO refactor this method which uses a mix of reflect and string arrays
+func validateRequiredArgs(cmd *Command, cmdArgs interface{}, rawArgs args.RawArgs) error {
 	for _, arg := range cmd.ArgSpecs {
+		if !arg.Required {
+			continue
+		}
+
 		fieldName := strcase.ToPublicGoName(arg.Name)
 		fieldValues, err := getValuesForFieldByName(reflect.ValueOf(cmdArgs), strings.Split(fieldName, "."))
 		if err != nil {
@@ -72,9 +78,13 @@ func validateRequiredArgs(cmd *Command, cmdArgs interface{}) error {
 			panic(validationErr)
 		}
 
-		for _, fieldValue := range fieldValues {
-			if arg.Required && (fieldValue.IsZero() || !fieldValue.IsValid()) {
-				return MissingRequiredArgumentError(arg.Name)
+		// Either fieldsValues have a length for 1 and we check for existence in the rawArgs
+		// or it has multiple values and we loop through each one to get the right element in
+		// the corresponding rawArgs array and replace {index} by the element's index.
+		// TODO handle required maps
+		for i := range fieldValues {
+			if !rawArgs.ExistsArgByName(strings.Replace(arg.Name, "{index}", strconv.Itoa(i), 1)) {
+				return MissingRequiredArgumentError(strings.Replace(arg.Name, "{index}", strconv.Itoa(i), 1))
 			}
 		}
 	}

internal/core/validate_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/alecthomas/assert"
+	"github.com/scaleway/scaleway-cli/internal/args"
 )
 
 type Element struct {
@@ -31,11 +32,12 @@ func Test_DefaultCommandValidateFunc(t *testing.T) {
 	type TestCase struct {
 		command         *Command
 		parsedArguments interface{}
+		rawArgs         args.RawArgs
 	}
 
 	run := func(testCase TestCase) func(t *testing.T) {
 		return func(t *testing.T) {
-			err := DefaultCommandValidateFunc()(testCase.command, testCase.parsedArguments)
+			err := DefaultCommandValidateFunc()(testCase.command, testCase.parsedArguments, testCase.rawArgs)
 			assert.Equal(t, fmt.Errorf("arg validation called"), err)
 		}
 	}
@@ -187,18 +189,19 @@ func Test_DefaultCommandRequiredFunc(t *testing.T) {
 	type TestCase struct {
 		command         *Command
 		parsedArguments interface{}
+		rawArgs         args.RawArgs
 	}
 
 	runOK := func(testCase TestCase) func(t *testing.T) {
 		return func(t *testing.T) {
-			err := DefaultCommandValidateFunc()(testCase.command, testCase.parsedArguments)
+			err := DefaultCommandValidateFunc()(testCase.command, testCase.parsedArguments, testCase.rawArgs)
 			assert.Equal(t, nil, err)
 		}
 	}
 
 	runErr := func(testCase TestCase, argName string) func(t *testing.T) {
 		return func(t *testing.T) {
-			err := DefaultCommandValidateFunc()(testCase.command, testCase.parsedArguments)
+			err := DefaultCommandValidateFunc()(testCase.command, testCase.parsedArguments, testCase.rawArgs)
 			assert.Equal(t, MissingRequiredArgumentError(argName), err)
 		}
 	}
@@ -212,6 +215,7 @@ func Test_DefaultCommandRequiredFunc(t *testing.T) {
 				},
 			},
 		},
+		rawArgs: []string{"first-nested-element.second-nested-element=test"},
 		parsedArguments: &elementCustom{
 			Element: &Element{
 				Name: "nested",
@@ -238,4 +242,49 @@ func Test_DefaultCommandRequiredFunc(t *testing.T) {
 			},
 		},
 	}, "first-nested-element.second-nested-element"))
+
+	t.Run("required-index", runOK(TestCase{
+		command: &Command{
+			ArgSpecs: ArgSpecs{
+				{
+					Name:     "elements-slice.{index}.id",
+					Required: true,
+				},
+			},
+		},
+		rawArgs: []string{"elements-slice.0.id=1"},
+		parsedArguments: &Element{
+			ElementsSlice: []Element{
+				{
+					ID:   0,
+					Name: "1",
+				},
+			},
+		},
+	}))
+
+	t.Run("fail-required-index", runErr(TestCase{
+		command: &Command{
+			ArgSpecs: ArgSpecs{
+				{
+					Name:     "elements-slice.{index}.id",
+					Required: true,
+				},
+			},
+		},
+		rawArgs: []string{"elements-slice.0.id=1"},
+		parsedArguments: &Element{
+			ElementsSlice: []Element{
+				{
+					ID:   0,
+					Name: "1",
+				},
+				{
+					ID:   1,
+					Name: "0",
+				},
+			},
+		},
+	}, "elements-slice.1.id"))
+
 }