From ac2741ab3e4afe42f9fc006e008c3bc4e834df89 Mon Sep 17 00:00:00 2001 From: Jerome Quere Date: Mon, 15 Jun 2020 19:47:53 +0200 Subject: [PATCH] feat(core): add support for autocomplete on bool value (#1081) --- internal/args/args.go | 56 ++++++++++++++++++++++++++ internal/args/args_test.go | 64 ++++++++++++++++++++++++++++++ internal/core/autocomplete.go | 32 +++++++++++---- internal/core/autocomplete_test.go | 11 ++++- 4 files changed, 155 insertions(+), 8 deletions(-) diff --git a/internal/args/args.go b/internal/args/args.go index e192e9046a..a285ad466c 100644 --- a/internal/args/args.go +++ b/internal/args/args.go @@ -1,9 +1,12 @@ package args import ( + "fmt" "reflect" "regexp" "strings" + + "github.com/scaleway/scaleway-sdk-go/strcase" ) // validArgNameRegex regex to check that args words are lower-case or digit starting and ending with a letter. @@ -199,3 +202,56 @@ func isPositionalArg(arg string) bool { pos := strings.IndexRune(arg, '=') return pos == -1 } + +// This function take a go struct and a name that comply with ArgSpec name notation (e.g "friends.{index}.name") +func GetArgType(argType reflect.Type, name string) (reflect.Type, error) { + var recursiveFunc func(argType reflect.Type, parts []string) (reflect.Type, error) + recursiveFunc = func(argType reflect.Type, parts []string) (reflect.Type, error) { + switch { + case argType.Kind() == reflect.Ptr: + return recursiveFunc(argType.Elem(), parts) + case len(parts) == 0: + return argType, nil + case parts[0] == sliceSchema: + return recursiveFunc(argType.Elem(), parts[1:]) + case parts[0] == mapSchema: + return recursiveFunc(argType.Elem(), parts[1:]) + default: + // We cannot rely on dest.GetFieldByName() as reflect library is doing deep traversing when using anonymous field. + // Because of that we should rely on our own logic + // + // - First we try to find a field with the correct name in the current struct + // - If it does not exist we try to find it in all nested anonymous fields + // Anonymous fields are traversed from last to first as the last one in the struct declaration should take precedence + + // We construct two caches: + anonymousFieldIndexes := []int(nil) + fieldIndexByName := map[string]int{} + for i := 0; i < argType.NumField(); i++ { + field := argType.Field(i) + if field.Anonymous { + anonymousFieldIndexes = append(anonymousFieldIndexes, i) + } else { + fieldIndexByName[field.Name] = i + } + } + + // Try to find the correct field in the current struct. + fieldName := strcase.ToPublicGoName(parts[0]) + if fieldIndex, exist := fieldIndexByName[fieldName]; exist { + return recursiveFunc(argType.Field(fieldIndex).Type, parts[1:]) + } + + // If it does not exist we try to find it in nested anonymous field + for i := len(anonymousFieldIndexes) - 1; i >= 0; i-- { + argType, err := recursiveFunc(argType.Field(anonymousFieldIndexes[i]).Type, parts) + if err == nil { + return argType, nil + } + } + } + return nil, fmt.Errorf("count not find %s", name) + } + + return recursiveFunc(argType, strings.Split(name, ".")) +} diff --git a/internal/args/args_test.go b/internal/args/args_test.go index 6e74563b67..61ce218c66 100644 --- a/internal/args/args_test.go +++ b/internal/args/args_test.go @@ -2,11 +2,13 @@ package args import ( "fmt" + "reflect" "strings" "testing" "github.com/alecthomas/assert" "github.com/scaleway/scaleway-sdk-go/scw" + "github.com/stretchr/testify/require" ) type Basic struct { @@ -154,3 +156,65 @@ func TestRawArgs_GetAll(t *testing.T) { assert.Equal(t, a.GetAll("countries.{key}.cities.{key}.street.{key}"), []string{"pouet", "tati", "anglais", "rouge"}) }) } + +func TestGetArgType(t *testing.T) { + type TestCase struct { + ArgType reflect.Type + Name string + ExpectedKind reflect.Kind + expectedError string + } + + run := func(tc *TestCase) func(*testing.T) { + return func(t *testing.T) { + res, err := GetArgType(tc.ArgType, tc.Name) + if tc.expectedError == "" { + require.NoError(t, err) + assert.Equal(t, tc.ExpectedKind, res.Kind()) + } else { + require.Equal(t, tc.expectedError, err.Error()) + } + } + } + + t.Run("Simple", run(&TestCase{ + ArgType: reflect.TypeOf(&Basic{}), + Name: "string", + ExpectedKind: reflect.String, + })) + t.Run("Simple int", run(&TestCase{ + ArgType: reflect.TypeOf(&Basic{}), + Name: "int-64", + ExpectedKind: reflect.Int64, + })) + t.Run("Ptr", run(&TestCase{ + ArgType: reflect.TypeOf(&Basic{}), + Name: "string-ptr", + ExpectedKind: reflect.String, + })) + t.Run("simple slice", run(&TestCase{ + ArgType: reflect.TypeOf(&Slice{}), + Name: "strings.{index}", + ExpectedKind: reflect.String, + })) + t.Run("simple slice ptr", run(&TestCase{ + ArgType: reflect.TypeOf(&Slice{}), + Name: "slice-ptr.{index}", + ExpectedKind: reflect.String, + })) + t.Run("nested simple", run(&TestCase{ + ArgType: reflect.TypeOf(&Nested{}), + Name: "basic.string", + ExpectedKind: reflect.String, + })) + t.Run("merge simple", run(&TestCase{ + ArgType: reflect.TypeOf(&Merge{}), + Name: "merge1", + ExpectedKind: reflect.String, + })) + t.Run("merge simple all", run(&TestCase{ + ArgType: reflect.TypeOf(&Merge{}), + Name: "all", + ExpectedKind: reflect.String, + })) +} diff --git a/internal/core/autocomplete.go b/internal/core/autocomplete.go index 568f7b628d..9d66660ab0 100644 --- a/internal/core/autocomplete.go +++ b/internal/core/autocomplete.go @@ -2,10 +2,13 @@ package core import ( "context" + "reflect" "regexp" "sort" "strconv" "strings" + + "github.com/scaleway/scaleway-cli/internal/args" ) // AutocompleteSuggestions is a list of words to be set to the shell as autocomplete suggestions. @@ -104,11 +107,12 @@ func NewAutoCompleteCommandNode() *AutoCompleteNode { // NewArgAutoCompleteNode creates a new node corresponding to a command argument. // These nodes are leaf nodes. -func NewAutoCompleteArgNode(argSpec *ArgSpec) *AutoCompleteNode { +func NewAutoCompleteArgNode(cmd *Command, argSpec *ArgSpec) *AutoCompleteNode { return &AutoCompleteNode{ Children: make(map[string]*AutoCompleteNode), ArgSpec: argSpec, Type: AutoCompleteNodeTypeArgument, + Command: cmd, } } @@ -207,10 +211,10 @@ func BuildAutoCompleteTree(commands *Commands) *AutoCompleteNode { // We consider ArgSpecs as leaf in the autocomplete tree. for _, argSpec := range cmd.ArgSpecs { if argSpec.Positional { - node.Children[positionalValueNodeID] = NewAutoCompleteArgNode(argSpec) + node.Children[positionalValueNodeID] = NewAutoCompleteArgNode(cmd, argSpec) continue } - node.Children[argSpec.Name+"="] = NewAutoCompleteArgNode(argSpec) + node.Children[argSpec.Name+"="] = NewAutoCompleteArgNode(cmd, argSpec) } if cmd.WaitFunc != nil { @@ -306,7 +310,7 @@ func AutoComplete(ctx context.Context, leftWords []string, wordToComplete string // We try to complete the value of an unknown arg return &AutocompleteResponse{} } - suggestions := AutoCompleteArgValue(ctx, argNode.ArgSpec, argValuePrefix) + suggestions := AutoCompleteArgValue(ctx, argNode.Command, argNode.ArgSpec, argValuePrefix) // We need to prefix suggestions with the argName to enable the arg value auto-completion. for k, s := range suggestions { @@ -320,7 +324,7 @@ func AutoComplete(ctx context.Context, leftWords []string, wordToComplete string suggestions := []string(nil) for key, child := range node.Children { if key == positionalValueNodeID { - for _, positionalSuggestion := range AutoCompleteArgValue(ctx, child.ArgSpec, wordToComplete) { + for _, positionalSuggestion := range AutoCompleteArgValue(ctx, child.Command, child.ArgSpec, wordToComplete) { if _, exists := completedArgs[positionalSuggestion]; !exists { suggestions = append(suggestions, positionalSuggestion) } @@ -362,15 +366,29 @@ func AutoComplete(ctx context.Context, leftWords []string, wordToComplete string // AutoCompleteArgValue returns suggestions for a (argument name, argument value prefix) pair. // Priority is given to the AutoCompleteFunc from the ArgSpec, if it is set. // Otherwise, we use EnumValues from the ArgSpec. -func AutoCompleteArgValue(ctx context.Context, argSpec *ArgSpec, argValuePrefix string) []string { +func AutoCompleteArgValue(ctx context.Context, cmd *Command, argSpec *ArgSpec, argValuePrefix string) []string { if argSpec == nil { return nil } if argSpec.AutoCompleteFunc != nil { return argSpec.AutoCompleteFunc(ctx, argValuePrefix) } + + possibleValues := []string(nil) + + if fieldType, err := args.GetArgType(cmd.ArgsType, argSpec.Name); err == nil { + switch fieldType.Kind() { + case reflect.Bool: + possibleValues = []string{"true", "false"} + } + } + + if len(argSpec.EnumValues) > 0 { + possibleValues = argSpec.EnumValues + } + suggestions := []string(nil) - for _, value := range argSpec.EnumValues { + for _, value := range possibleValues { if strings.HasPrefix(value, argValuePrefix) { suggestions = append(suggestions, value) } diff --git a/internal/core/autocomplete_test.go b/internal/core/autocomplete_test.go index 08478e6e7a..914bb6d083 100644 --- a/internal/core/autocomplete_test.go +++ b/internal/core/autocomplete_test.go @@ -2,6 +2,7 @@ package core import ( "context" + "reflect" "regexp" "strings" "testing" @@ -15,6 +16,7 @@ func testAutocompleteGetCommands() *Commands { Namespace: "test", Resource: "flower", Verb: "create", + ArgsType: reflect.TypeOf(struct{}{}), ArgSpecs: ArgSpecs{ { Name: "name", @@ -47,6 +49,9 @@ func testAutocompleteGetCommands() *Commands { Namespace: "test", Resource: "flower", Verb: "delete", + ArgsType: reflect.TypeOf(struct { + WithLeaves bool + }{}), ArgSpecs: ArgSpecs{ { Name: "name", @@ -127,6 +132,7 @@ func TestAutocomplete(t *testing.T) { t.Run("scw test flower create leaves.", run(&testCase{Suggestions: AutocompleteSuggestions{"leaves.0.size="}})) t.Run("scw test flower create leaves.0", run(&testCase{Suggestions: AutocompleteSuggestions{"leaves.0.size="}})) t.Run("scw test flower create leaves.0.", run(&testCase{Suggestions: AutocompleteSuggestions{"leaves.0.size="}})) + t.Run("scw test flower create leaves.0.size=M", run(&testCase{Suggestions: AutocompleteSuggestions{"leaves.0.size=M"}})) t.Run("scw test flower create leaves.0.size=M leaves", run(&testCase{Suggestions: AutocompleteSuggestions{"leaves.1.size="}})) t.Run("scw test flower create leaves.0.size=M leaves leaves.1.size=M", run(&testCase{WordToCompleteIndex: 5, Suggestions: AutocompleteSuggestions{"leaves.2.size="}})) t.Run("scw test flower delete ", run(&testCase{Suggestions: AutocompleteSuggestions{"anemone", "hibiscus", "with-leaves="}})) @@ -135,8 +141,11 @@ func TestAutocomplete(t *testing.T) { t.Run("scw test flower delete with-leaves=true ", run(&testCase{Suggestions: AutocompleteSuggestions{"anemone", "hibiscus"}})) // invalid notation t.Run("scw test flower delete hibiscus n", run(&testCase{Suggestions: nil})) t.Run("scw test flower delete hibiscus w", run(&testCase{Suggestions: AutocompleteSuggestions{"with-leaves="}})) - t.Run("scw test flower delete hibiscus with-leaves=true", run(&testCase{Suggestions: nil})) + t.Run("scw test flower delete hibiscus with-leaves=true", run(&testCase{Suggestions: AutocompleteSuggestions{"with-leaves=true"}})) t.Run("scw test flower delete hibiscus with-leaves=true ", run(&testCase{Suggestions: AutocompleteSuggestions{"anemone"}})) + t.Run("scw test flower delete hibiscus with-leaves=", run(&testCase{Suggestions: AutocompleteSuggestions{"with-leaves=false", "with-leaves=true"}})) + t.Run("scw test flower delete hibiscus with-leaves=tr", run(&testCase{Suggestions: AutocompleteSuggestions{"with-leaves=true"}})) + t.Run("scw test flower delete hibiscus with-leaves=yes", run(&testCase{Suggestions: nil})) t.Run("scw test flower create leaves.0.size=", run(&testCase{Suggestions: AutocompleteSuggestions{"leaves.0.size=L", "leaves.0.size=M", "leaves.0.size=S", "leaves.0.size=XL", "leaves.0.size=XXL"}})) t.Run("scw -", run(&testCase{Suggestions: AutocompleteSuggestions{"--debug", "--help", "--output", "--profile", "-D", "-h", "-o", "-p"}})) t.Run("scw test -o j", run(&testCase{Suggestions: AutocompleteSuggestions{"json"}}))