From 81e04c3050ec3ccb5ec298f296148d1e111d6739 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 30 Apr 2019 13:12:00 -0400 Subject: [PATCH] more precise type handling in slice When slicing a list containing unknown values, the list itself is known, and slice should return the proper slice of that list. Make SliceFunc return the correct type for slices of tuples, and disallow slicing sets. --- lang/funcs/collection.go | 86 ++++++++++++++++++++++------------- lang/funcs/collection_test.go | 59 ++++++++++++++++++++++-- 2 files changed, 110 insertions(+), 35 deletions(-) diff --git a/lang/funcs/collection.go b/lang/funcs/collection.go index 5422a0f9d..d72d7261b 100644 --- a/lang/funcs/collection.go +++ b/lang/funcs/collection.go @@ -1,6 +1,7 @@ package funcs import ( + "errors" "fmt" "sort" @@ -1040,7 +1041,7 @@ var SliceFunc = function.New(&function.Spec{ Params: []function.Parameter{ { Name: "list", - Type: cty.List(cty.DynamicPseudoType), + Type: cty.DynamicPseudoType, }, { Name: "startIndex", @@ -1052,50 +1053,71 @@ var SliceFunc = function.New(&function.Spec{ }, }, Type: func(args []cty.Value) (cty.Type, error) { - return args[0].Type(), nil + arg := args[0] + argTy := arg.Type() + + if !argTy.IsListType() && !argTy.IsTupleType() { + return cty.NilType, errors.New("cannot slice a set, because its elements do not have indices; use the tolist function to force conversion to list if the ordering of the result is not important") + } + + if argTy.IsListType() { + return argTy, nil + } + + startIndex, endIndex, err := sliceIndexes(args, args[0].LengthInt()) + if err != nil { + return cty.NilType, err + } + + return cty.Tuple(argTy.TupleElementTypes()[startIndex:endIndex]), nil }, Impl: func(args []cty.Value, retType cty.Type) (ret cty.Value, err error) { inputList := args[0] - if !inputList.IsWhollyKnown() { - return cty.UnknownVal(retType), nil - } - var startIndex, endIndex int - if err = gocty.FromCtyValue(args[1], &startIndex); err != nil { - return cty.NilVal, fmt.Errorf("invalid start index: %s", err) - } - if err = gocty.FromCtyValue(args[2], &endIndex); err != nil { - return cty.NilVal, fmt.Errorf("invalid start index: %s", err) + startIndex, endIndex, err := sliceIndexes(args, inputList.LengthInt()) + if err != nil { + return cty.NilVal, err } - if startIndex < 0 { - return cty.NilVal, fmt.Errorf("from index must be >= 0") - } - if endIndex > inputList.LengthInt() { - return cty.NilVal, fmt.Errorf("to index must be <= length of the input list") - } - if startIndex > endIndex { - return cty.NilVal, fmt.Errorf("from index must be <= to index") - } - - var outputList []cty.Value - - i := 0 - for it := inputList.ElementIterator(); it.Next(); { - _, v := it.Element() - if i >= startIndex && i < endIndex { - outputList = append(outputList, v) + if endIndex-startIndex == 0 { + if retType.IsTupleType() { + return cty.EmptyTupleVal, nil } - i++ - } - - if len(outputList) == 0 { return cty.ListValEmpty(retType.ElementType()), nil } + + outputList := inputList.AsValueSlice()[startIndex:endIndex] + + if retType.IsTupleType() { + return cty.TupleVal(outputList), nil + } + return cty.ListVal(outputList), nil }, }) +func sliceIndexes(args []cty.Value, max int) (int, int, error) { + var startIndex, endIndex int + + if err := gocty.FromCtyValue(args[1], &startIndex); err != nil { + return 0, 0, fmt.Errorf("invalid start index: %s", err) + } + if err := gocty.FromCtyValue(args[2], &endIndex); err != nil { + return 0, 0, fmt.Errorf("invalid start index: %s", err) + } + + if startIndex < 0 { + return 0, 0, errors.New("from index must be greater than or equal to 0") + } + if endIndex > max { + return 0, 0, errors.New("to index must be less than or equal to the length of the input list") + } + if startIndex > endIndex { + return 0, 0, errors.New("from index must be less than or equal to index") + } + return startIndex, endIndex, nil +} + // TransposeFunc contructs a function that takes a map of lists of strings and // swaps the keys and values to produce a new map of lists of strings. var TransposeFunc = function.New(&function.Spec{ diff --git a/lang/funcs/collection_test.go b/lang/funcs/collection_test.go index 4339d3af4..eadc3cddb 100644 --- a/lang/funcs/collection_test.go +++ b/lang/funcs/collection_test.go @@ -2354,6 +2354,11 @@ func TestSlice(t *testing.T) { cty.StringVal("a"), cty.UnknownVal(cty.String), }) + tuple := cty.TupleVal([]cty.Value{ + cty.StringVal("a"), + cty.NumberIntVal(1), + cty.UnknownVal(cty.List(cty.String)), + }) tests := []struct { List cty.Value StartIndex cty.Value @@ -2370,10 +2375,24 @@ func TestSlice(t *testing.T) { }), false, }, - { // unknowns in the list + { // slice only an unknown value listWithUnknowns, cty.NumberIntVal(1), cty.NumberIntVal(2), + cty.ListVal([]cty.Value{cty.UnknownVal(cty.String)}), + false, + }, + { // slice multiple values, which contain an unknown + listWithUnknowns, + cty.NumberIntVal(0), + cty.NumberIntVal(2), + listWithUnknowns, + false, + }, + { // an unknown list should be slicable, returning an unknown list + cty.UnknownVal(cty.List(cty.String)), + cty.NumberIntVal(0), + cty.NumberIntVal(2), cty.UnknownVal(cty.List(cty.String)), false, }, @@ -2414,10 +2433,44 @@ func TestSlice(t *testing.T) { cty.NilVal, true, }, + { // sets are not slice-able + cty.SetVal([]cty.Value{ + cty.StringVal("x"), + cty.StringVal("y"), + }), + cty.NumberIntVal(0), + cty.NumberIntVal(0), + cty.NilVal, + true, + }, + { // tuple slice + tuple, + cty.NumberIntVal(1), + cty.NumberIntVal(3), + cty.TupleVal([]cty.Value{ + cty.NumberIntVal(1), + cty.UnknownVal(cty.List(cty.String)), + }), + false, + }, + { // empty list slice + listOfStrings, + cty.NumberIntVal(2), + cty.NumberIntVal(2), + cty.ListValEmpty(cty.String), + false, + }, + { // empty tuple slice + tuple, + cty.NumberIntVal(3), + cty.NumberIntVal(3), + cty.EmptyTupleVal, + false, + }, } - for _, test := range tests { - t.Run(fmt.Sprintf("slice(%#v, %#v, %#v)", test.List, test.StartIndex, test.EndIndex), func(t *testing.T) { + for i, test := range tests { + t.Run(fmt.Sprintf("%d-slice(%#v, %#v, %#v)", i, test.List, test.StartIndex, test.EndIndex), func(t *testing.T) { got, err := Slice(test.List, test.StartIndex, test.EndIndex) if test.Err {