Skip to content

Commit

Permalink
fix nil input for Make*() (#26)
Browse files Browse the repository at this point in the history
* make dict for nil and empty

* add comments

* for nil MakeList

* for nil MakeTuple

* for MakeSetFromSlice

* for MakeStringDict

* for nil func

* panic it
  • Loading branch information
hyorigo authored Apr 1, 2024
1 parent 8fe1d93 commit 825da6b
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 23 deletions.
53 changes: 31 additions & 22 deletions convert/conv.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,13 @@ func FromValue(v starlark.Value) interface{} {
}

// MakeStringDict makes a StringDict from the given arg. The types supported are the same as ToValue.
// It returns an empty dict for nil input.
func MakeStringDict(m map[string]interface{}) (starlark.StringDict, error) {
return makeStringDictTag(m, emptyStr)
}

// MakeStringDictWithTag makes a StringDict from the given arg with custom tag. The types supported are the same as ToValueWithTag.
// It returns an empty dict for nil input.
func MakeStringDictWithTag(m map[string]interface{}, tagName string) (starlark.StringDict, error) {
return makeStringDictTag(m, tagName)
}
Expand All @@ -210,6 +212,7 @@ func FromStringDict(m starlark.StringDict) map[string]interface{} {
}

// MakeTuple makes a Starlark Tuple from the given Go slice. The types supported are the same as ToValue.
// It returns an empty tuple for nil input.
func MakeTuple(v []interface{}) (starlark.Tuple, error) {
tuple := make(starlark.Tuple, len(v))
for i, val := range v {
Expand All @@ -232,6 +235,7 @@ func FromTuple(v starlark.Tuple) []interface{} {
}

// MakeList makes a Starlark List from the given Go slice. The types supported are the same as ToValue.
// It returns an empty list for nil input.
func MakeList(v []interface{}) (*starlark.List, error) {
values := make([]starlark.Value, len(v))
for i := range v {
Expand Down Expand Up @@ -265,33 +269,38 @@ func FromList(l *starlark.List) []interface{} {
}

// MakeDict makes a Dict from the given map. The acceptable keys and values are the same as ToValue.
// For nil input, it returns an empty Dict. It panics if the input is not a map.
func MakeDict(v interface{}) (starlark.Value, error) {
return makeDictTag(reflect.ValueOf(v), emptyStr)
}

// MakeDictWithTag makes a Dict from the given map with custom tag. The acceptable keys and values are the same as ToValueWithTag.
// For nil input, it returns an empty Dict. It panics if the input is not a map.
func MakeDictWithTag(v interface{}, tagName string) (starlark.Value, error) {
return makeDictTag(reflect.ValueOf(v), tagName)
}

func makeDictTag(val reflect.Value, tagName string) (starlark.Value, error) {
if val.Kind() != reflect.Map {
dict := starlark.NewDict(1)
// check if the value is not nil and is a map
if valid := val.IsValid(); valid && val.Kind() != reflect.Map {
// panic if not a map
panic(fmt.Errorf("can't make map of %T", val.Interface()))
}

dict := starlark.Dict{}
for _, k := range val.MapKeys() {
vk, err := adjustedToValue(k, tagName)
if err != nil {
return nil, err
}
vv, err := adjustedToValue(val.MapIndex(k), tagName)
if err != nil {
return nil, err
} else if valid {
// iterate over the map and convert each key and value
for _, k := range val.MapKeys() {
vk, err := adjustedToValue(k, tagName)
if err != nil {
return nil, err
}
vv, err := adjustedToValue(val.MapIndex(k), tagName)
if err != nil {
return nil, err
}
dict.SetKey(vk, vv)
}
dict.SetKey(vk, vv)
}
return &dict, nil
return dict, nil
}

// Helper method that checks the input value for interface{} and adjusts the conversion accordingly.
Expand Down Expand Up @@ -323,6 +332,7 @@ func FromDict(m *starlark.Dict) map[interface{}]interface{} {
}

// MakeSet makes a Set from the given map. The acceptable keys the same as ToValue.
// For nil input, it returns an empty Set.
func MakeSet(s map[interface{}]bool) (*starlark.Set, error) {
set := starlark.Set{}
for k := range s {
Expand All @@ -338,6 +348,7 @@ func MakeSet(s map[interface{}]bool) (*starlark.Set, error) {
}

// MakeSetFromSlice makes a Set from the given slice. The acceptable keys the same as ToValue.
// For nil input, it returns an empty Set.
func MakeSetFromSlice(s []interface{}) (*starlark.Set, error) {
set := starlark.Set{}
for i := range s {
Expand Down Expand Up @@ -390,14 +401,12 @@ func FromKwargs(kwargs []starlark.Tuple) ([]Kwarg, error) {
return args, nil
}

// MakeStarFn creates a wrapper around the given function that can be called from
// a starlark script. Argument support is the same as ToValue. If the last value
// the function returns is an error, it will cause an error to be returned from
// the starlark function. If there are no other errors, the function will return
// None. If there's exactly one other value, the function will return the
// starlark equivalent of that value. If there is more than one return value,
// they'll be returned as a tuple. MakeStarFn will panic if you pass it
// something other than a function.
// MakeStarFn creates a wrapper around the given function that can be called from a starlark script. Argument support is the same as ToValue.
// If the last value the function returns is an error, it will cause an error to be returned from the starlark function.
// If there are no other errors, the function will return None.
// If there's exactly one other value, the function will return the starlark equivalent of that value.
// If there is more than one return value, they'll be returned as a tuple.
// MakeStarFn will panic if you pass it something other than a function, like nil or a non-function.
func MakeStarFn(name string, gofn interface{}) *starlark.Builtin {
v := reflect.ValueOf(gofn)
if v.Kind() != reflect.Func {
Expand Down
45 changes: 44 additions & 1 deletion convert/conv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,21 @@ import (
)

func TestMakeTuple(t *testing.T) {
tuple0, err := MakeTuple([]interface{}{})
if err != nil {
t.Errorf("unexpected error 0: %v", err)
return
} else if tuple0 == nil {
t.Errorf("expected tuple0 to be non-nil")
return
}
tuple1, err := MakeTuple(nil)
if err != nil {
t.Errorf("unexpected error 1: %v", err)
return
} else if tuple1 == nil {
t.Errorf("expected tuple1 to be non-nil")
return
}
tuple2, err := MakeTuple([]interface{}{"a", 1, true, 0.1})
if err != nil {
Expand Down Expand Up @@ -84,10 +95,21 @@ t2 = ("a", 1, True, 0.1)
}

func TestMakeList(t *testing.T) {
list0, err := MakeList([]interface{}{})
if err != nil {
t.Errorf("unexpected error 0: %v", err)
return
} else if list0 == nil {
t.Errorf("expected list0 to be non-nil")
return
}
list1, err := MakeList(nil)
if err != nil {
t.Errorf("unexpected error 1: %v", err)
return
} else if list1 == nil {
t.Errorf("expected list1 to be non-nil")
return
}
list2, err := MakeList([]interface{}{"a", 1, true, 0.1})
if err != nil {
Expand Down Expand Up @@ -133,9 +155,19 @@ t2d = type(list_has[3])
}

func TestMakeSet(t *testing.T) {
if _, err := MakeSet(nil); err != nil {
if s0, err := MakeSet(map[interface{}]bool{}); err != nil {
t.Errorf("unexpected error 0: %v", err)
return
} else if s0 == nil {
t.Errorf("expected s0 to be non-nil")
return
}
if s1, err := MakeSet(nil); err != nil {
t.Errorf("unexpected error 1: %v", err)
return
} else if s1 == nil {
t.Errorf("expected s1 to be non-nil")
return
}
if _, err := MakeSet(map[interface{}]bool{"a": true, 1: true, true: true, 0.1: true}); err != nil {
t.Errorf("unexpected error 2: %v", err)
Expand All @@ -148,10 +180,21 @@ func TestMakeSet(t *testing.T) {
}

func TestMakeSetFromSlice(t *testing.T) {
set0, err := MakeSetFromSlice([]interface{}{})
if err != nil {
t.Errorf("unexpected error 0: %v", err)
return
} else if set0 == nil {
t.Errorf("expected set0 to be non-nil")
return
}
set1, err := MakeSetFromSlice(nil)
if err != nil {
t.Errorf("unexpected error 1: %v", err)
return
} else if set1 == nil {
t.Errorf("expected set1 to be non-nil")
return
}
set2, err := MakeSetFromSlice([]interface{}{"a", 1, true, 0.1})
if err != nil {
Expand Down
6 changes: 6 additions & 0 deletions convert/func_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,12 @@ func TestMakeStarFnArgumentType(t *testing.T) {
wantErr bool
}
testCases := []testCase{
{
name: "Nil",
funcToConvert: nil,
codeSnippet: `x = boo()`,
shouldPanic: true,
},
{
name: "Non-function type",
funcToConvert: 123,
Expand Down
28 changes: 28 additions & 0 deletions convert/struct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,34 @@ p.Value += 1
}
}

func TestMakeStringDict_Nil(t *testing.T) {
s1, err := convert.MakeStringDict(nil)
if err != nil {
t.Fatal(err)
}
s2, err := convert.MakeStringDictWithTag(nil, "star")
if err != nil {
t.Fatal(err)
}
if len(s1) != 0 || len(s2) != 0 {
t.Fatalf("expected empty string dict, but got %v and %v", s1, s2)
}
}

func TestMakeStringDict_Empty(t *testing.T) {
s1, err := convert.MakeStringDict(map[string]interface{}{})
if err != nil {
t.Fatal(err)
}
s2, err := convert.MakeStringDictWithTag(map[string]interface{}{}, "star")
if err != nil {
t.Fatal(err)
}
if len(s1) != 0 || len(s2) != 0 {
t.Fatalf("expected empty string dict, but got %v and %v", s1, s2)
}
}

func TestMakeStringDict(t *testing.T) {
type contact struct {
Name string `sl:"name"`
Expand Down
21 changes: 21 additions & 0 deletions convert/value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,18 @@ def double(x):
return globals["double"].(*starlark.Function)
}

func TestMakeDict_Panic(t *testing.T) {
defer func() {
if r := recover(); r == nil {
t.Errorf("MakeDict did not panic")
}
}()
_, _ = convert.MakeDict(123)
}

func TestMakeDict(t *testing.T) {
sd0 := starlark.NewDict(0)

sd1 := starlark.NewDict(1)
_ = sd1.SetKey(starlark.String("a"), starlark.String("b"))

Expand All @@ -495,6 +506,16 @@ func TestMakeDict(t *testing.T) {
wantErr bool
strMatch bool
}{
{
name: "nil",
v: nil,
want: sd0,
},
{
name: "empty",
v: map[string]interface{}{},
want: sd0,
},
{
name: "map[string]string",
v: map[string]string{"a": "b"},
Expand Down

0 comments on commit 825da6b

Please sign in to comment.