diff --git a/internal/args/args.go b/internal/args/args.go index d4f5344689..90577a1b37 100644 --- a/internal/args/args.go +++ b/internal/args/args.go @@ -255,3 +255,66 @@ func GetArgType(argType reflect.Type, name string) (reflect.Type, error) { return recursiveFunc(argType, strings.Split(name, ".")) } + +var listArgTypeFieldsSkippedArguments = []string{ + "page", + "page-size", + "per-page", +} + +func listArgTypeFields(base string, argType reflect.Type) []string { + if argType.Kind() != reflect.Ptr { + // Can be a handled type like time.Time + // If so, use it like a scalar type + _, isHandled := unmarshalFuncs[argType] + if isHandled { + return []string{base} + } + } + + switch argType.Kind() { + case reflect.Ptr: + return listArgTypeFields(base, argType.Elem()) + + case reflect.Slice: + return listArgTypeFields(base+"."+sliceSchema, argType.Elem()) + + case reflect.Map: + return listArgTypeFields(base+"."+mapSchema, argType.Elem()) + + case reflect.Struct: + fields := []string(nil) + + for i := 0; i < argType.NumField(); i++ { + field := argType.Field(i) + fieldBase := base + + // If this is an embedded struct, skip adding its name to base + if field.Anonymous { + fields = append(fields, listArgTypeFields(fieldBase, field.Type)...) + continue + } + + if fieldBase == "" { + fieldBase = strcase.ToBashArg(field.Name) + } else { + fieldBase += "." + strcase.ToBashArg(field.Name) + } + fields = append(fields, listArgTypeFields(fieldBase, field.Type)...) + } + + return fields + default: + for _, skippedArg := range listArgTypeFieldsSkippedArguments { + if base == skippedArg { + return []string{} + } + } + return []string{base} + } +} + +// ListArgTypeFields take a go struct and return a list of name that comply with ArgSpec name notation (e.g "friends.{index}.name") +func ListArgTypeFields(argType reflect.Type) []string { + return listArgTypeFields("", argType) +} diff --git a/internal/core/arg_specs.go b/internal/core/arg_specs.go index b03640a890..e7f57b518f 100644 --- a/internal/core/arg_specs.go +++ b/internal/core/arg_specs.go @@ -3,11 +3,9 @@ package core import ( "context" "fmt" - "reflect" "strings" "github.com/scaleway/scaleway-sdk-go/scw" - "github.com/scaleway/scaleway-sdk-go/strcase" "github.com/scaleway/scaleway-sdk-go/validation" ) @@ -121,13 +119,6 @@ func (a *ArgSpec) ConflictWith(b *ArgSpec) bool { (a.OneOfGroup == b.OneOfGroup) } -// GetArgsTypeField returns the type of the argument in the given ArgsType -func (a *ArgSpec) GetArgsTypeField(argsType reflect.Type) (reflect.Type, error) { - argSpecGoName := strcase.ToPublicGoName(a.Name) - - return getTypeForFieldByName(argsType, strings.Split(argSpecGoName, ".")) -} - type DefaultFunc func(ctx context.Context) (value string, doc string) func ZoneArgSpec(zones ...scw.Zone) *ArgSpec { diff --git a/internal/core/arg_specs_test.go b/internal/core/arg_specs_test.go index 633d249ffa..e5b39c7ae5 100644 --- a/internal/core/arg_specs_test.go +++ b/internal/core/arg_specs_test.go @@ -1,7 +1,6 @@ package core import ( - "reflect" "testing" "github.com/alecthomas/assert" @@ -37,35 +36,3 @@ func TestOneOf(t *testing.T) { assert.False(t, a.ConflictWith(c)) assert.False(t, e.ConflictWith(e)) } - -func TestArgSpecGetArgsTypeField(t *testing.T) { - data := struct { - Field string - FieldStruct struct { - NestedField int - } - FieldSlice []float32 - FieldMap map[string]bool - }{} - dataType := reflect.TypeOf(data) - - fieldSpec := ArgSpec{Name: "field"} - typ, err := fieldSpec.GetArgsTypeField(dataType) - assert.Nil(t, err) - assert.Equal(t, reflect.TypeOf("string"), typ, "%s is not string", typ.Name()) - - fieldSpec = ArgSpec{Name: "field-struct.nested-field"} - typ, err = fieldSpec.GetArgsTypeField(dataType) - assert.Nil(t, err) - assert.Equal(t, reflect.TypeOf(int(1)), typ, "%s is not int", typ.Name()) - - fieldSpec = ArgSpec{Name: "field-slice.{index}"} - typ, err = fieldSpec.GetArgsTypeField(dataType) - assert.Nil(t, err) - assert.Equal(t, reflect.TypeOf(float32(1)), typ, "%s is not float32", typ.Name()) - - fieldSpec = ArgSpec{Name: "field-map.{key}"} - typ, err = fieldSpec.GetArgsTypeField(dataType) - assert.Nil(t, err) - assert.Equal(t, reflect.TypeOf(true), typ, "%s is not bool", typ.Name()) -} diff --git a/internal/core/reflect.go b/internal/core/reflect.go index 6c64a20d8e..2180476fbc 100644 --- a/internal/core/reflect.go +++ b/internal/core/reflect.go @@ -109,32 +109,3 @@ func getValuesForFieldByName(value reflect.Value, parts []string) (values []refl return nil, fmt.Errorf("case is not handled") } - -// getTypeForFieldByName recursively search for fields in an ArgsType -// The search is based on the name of the field. -func getTypeForFieldByName(value reflect.Type, parts []string) (reflect.Type, error) { - if len(parts) == 0 { - return value, nil - } - - switch value.Kind() { - case reflect.Ptr: - return getTypeForFieldByName(value.Elem(), parts) - - case reflect.Slice: - return getTypeForFieldByName(value.Elem(), parts[1:]) - - case reflect.Map: - return getTypeForFieldByName(value.Elem(), parts[1:]) - - case reflect.Struct: - fieldName := strcase.ToPublicGoName(parts[0]) - field, hasField := value.FieldByName(fieldName) - if !hasField { - return nil, fmt.Errorf("field %v does not exist for %v", fieldName, value.Name()) - } - return getTypeForFieldByName(field.Type, parts[1:]) - } - - return nil, fmt.Errorf("type kind %s is not handled", value.Kind().String()) -} diff --git a/internal/qa/argspec.go b/internal/qa/argspec.go new file mode 100644 index 0000000000..f091bed6df --- /dev/null +++ b/internal/qa/argspec.go @@ -0,0 +1,74 @@ +package qa + +import ( + "fmt" + "reflect" + + "github.com/scaleway/scaleway-cli/v2/internal/args" + "github.com/scaleway/scaleway-cli/v2/internal/core" +) + +type ArgSpecInvalidError struct { + Command *core.Command + argSpec *core.ArgSpec + innerError error +} + +func (err ArgSpecInvalidError) Error() string { + return fmt.Sprintf("command has invalid argspecs '%s' '%s' '%s'", + err.Command.GetCommandLine("scw"), + err.argSpec.Name, + err.innerError, + ) +} + +// testArgSpecInvalidError tests that all argspecs have a corresponding in their command's argstype. +func testArgSpecInvalidError(commands *core.Commands) []error { + errors := []error(nil) + + for _, command := range commands.GetAll() { + for _, arg := range command.ArgSpecs { + _, err := args.GetArgType(command.ArgsType, arg.Name) + if err != nil { + errors = append(errors, &ArgSpecInvalidError{Command: command, argSpec: arg, innerError: err}) + continue + } + } + } + + return errors +} + +type ArgSpecMissingError struct { + Command *core.Command + argName string +} + +func (err ArgSpecMissingError) Error() string { + return fmt.Sprintf("command has a missing argspec '%s' '%s'", + err.Command.GetCommandLine("scw"), + err.argName, + ) +} + +// testArgSpecInvalidError tests that all argstype fields have a corresponding argspec. +func testArgSpecMissingError(commands *core.Commands) []error { + errors := []error(nil) + + // Check all commands + for _, command := range commands.GetAll() { + if command.ArgsType == nil || command.ArgsType == reflect.TypeOf(args.RawArgs{}) { + continue + } + + supposedArgSpecs := args.ListArgTypeFields(command.ArgsType) + + for _, argSpecName := range supposedArgSpecs { + if command.ArgSpecs.GetByName(argSpecName) == nil { + errors = append(errors, &ArgSpecMissingError{Command: command, argName: argSpecName}) + } + } + } + + return errors +} diff --git a/internal/qa/qa.go b/internal/qa/qa.go index 9cbb8a0a85..c54c03ce07 100644 --- a/internal/qa/qa.go +++ b/internal/qa/qa.go @@ -19,6 +19,8 @@ func LintCommands(commands *core.Commands) []error { errors = append(errors, testDifferentLocalizationForNamespaceError(commands)...) errors = append(errors, testDuplicatedCommandError(commands)...) errors = append(errors, testAtLeastOneExampleIsPresentError(commands)...) + errors = append(errors, testArgSpecInvalidError(commands)...) + errors = append(errors, testArgSpecMissingError(commands)...) errors = filterIgnore(errors)