From 742deca3e946e1502fb690ffaa6ff0d471e53ce0 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Thu, 16 May 2019 17:16:24 -0700 Subject: [PATCH] lang/funcs: Short-circuit if start or end index is unknown Previously the type-selection codepath for an input tuple referred unconditionally to the start and end index values. In the Type callback, only the types of the arguments are guaranteed to be known, so any access to the values must be guarded with an .IsKnown check, or else the function will not short-circuit properly when an unknown value is passed. Now we will check the start and end indices are in range when we have enough information to do so, but we'll return an approximate result if either is unknown. --- lang/funcs/collection.go | 77 ++++++++++++++++++++-------- lang/funcs/collection_test.go | 94 +++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 22 deletions(-) diff --git a/lang/funcs/collection.go b/lang/funcs/collection.go index bc516f2c3..71b7a8466 100644 --- a/lang/funcs/collection.go +++ b/lang/funcs/collection.go @@ -1064,11 +1064,11 @@ var SliceFunc = function.New(&function.Spec{ Type: cty.DynamicPseudoType, }, { - Name: "startIndex", + Name: "start_index", Type: cty.Number, }, { - Name: "endIndex", + Name: "end_index", Type: cty.Number, }, }, @@ -1076,25 +1076,39 @@ var SliceFunc = function.New(&function.Spec{ arg := args[0] argTy := arg.Type() + if argTy.IsSetType() { + return cty.NilType, function.NewArgErrorf(0, "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() && !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") + return cty.NilType, function.NewArgErrorf(0, "must be a list or tuple value") + } + + startIndex, endIndex, idxsKnown, err := sliceIndexes(args) + if err != nil { + return cty.NilType, err } if argTy.IsListType() { return argTy, nil } - startIndex, endIndex, err := sliceIndexes(args, args[0].LengthInt()) - if err != nil { - return cty.NilType, err + if !idxsKnown { + // If we don't know our start/end indices then we can't predict + // the result type if we're planning to return a tuple. + return cty.DynamicPseudoType, nil } - return cty.Tuple(argTy.TupleElementTypes()[startIndex:endIndex]), nil }, Impl: func(args []cty.Value, retType cty.Type) (ret cty.Value, err error) { inputList := args[0] - startIndex, endIndex, err := sliceIndexes(args, inputList.LengthInt()) + if retType == cty.DynamicPseudoType { + return cty.DynamicVal, nil + } + + // we ignore idxsKnown return value here because the indices are always + // known here, or else the call would've short-circuited. + startIndex, endIndex, _, err := sliceIndexes(args) if err != nil { return cty.NilVal, err } @@ -1116,26 +1130,45 @@ var SliceFunc = function.New(&function.Spec{ }, }) -func sliceIndexes(args []cty.Value, max int) (int, int, error) { - var startIndex, endIndex int +func sliceIndexes(args []cty.Value) (int, int, bool, error) { + var startIndex, endIndex, length int + var startKnown, endKnown, lengthKnown bool - 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 args[0].Type().IsTupleType() || args[0].IsKnown() { // if it's a tuple then we always know the length by the type, but lists must be known + length = args[0].LengthInt() + lengthKnown = true } - if startIndex < 0 { - return 0, 0, errors.New("from index must be greater than or equal to 0") + if args[1].IsKnown() { + if err := gocty.FromCtyValue(args[1], &startIndex); err != nil { + return 0, 0, false, function.NewArgErrorf(1, "invalid start index: %s", err) + } + if startIndex < 0 { + return 0, 0, false, function.NewArgErrorf(1, "start index must not be less than zero") + } + if lengthKnown && startIndex > length { + return 0, 0, false, function.NewArgErrorf(1, "start index must not be greater than the length of the list") + } + startKnown = true } - if endIndex > max { - return 0, 0, errors.New("to index must be less than or equal to the length of the input list") + if args[2].IsKnown() { + if err := gocty.FromCtyValue(args[2], &endIndex); err != nil { + return 0, 0, false, function.NewArgErrorf(2, "invalid end index: %s", err) + } + if endIndex < 0 { + return 0, 0, false, function.NewArgErrorf(2, "end index must not be less than zero") + } + if lengthKnown && endIndex > length { + return 0, 0, false, function.NewArgErrorf(2, "end index must not be greater than the length of the list") + } + endKnown = true } - if startIndex > endIndex { - return 0, 0, errors.New("from index must be less than or equal to index") + if startKnown && endKnown { + if startIndex > endIndex { + return 0, 0, false, function.NewArgErrorf(1, "start index must not be greater than end index") + } } - return startIndex, endIndex, nil + return startIndex, endIndex, startKnown && endKnown, nil } // TransposeFunc contructs a function that takes a map of lists of strings and diff --git a/lang/funcs/collection_test.go b/lang/funcs/collection_test.go index dd112388f..d1a543797 100644 --- a/lang/funcs/collection_test.go +++ b/lang/funcs/collection_test.go @@ -2595,6 +2595,16 @@ func TestSlice(t *testing.T) { }), false, }, + { // unknown tuple slice + cty.UnknownVal(tuple.Type()), + cty.NumberIntVal(1), + cty.NumberIntVal(3), + cty.UnknownVal(cty.Tuple([]cty.Type{ + cty.Number, + cty.List(cty.String), + })), + false, + }, { // empty list slice listOfStrings, cty.NumberIntVal(2), @@ -2609,6 +2619,90 @@ func TestSlice(t *testing.T) { cty.EmptyTupleVal, false, }, + { // list with unknown start offset + listOfStrings, + cty.UnknownVal(cty.Number), + cty.NumberIntVal(2), + cty.UnknownVal(cty.List(cty.String)), + false, + }, + { // list with unknown start offset but end out of bounds + listOfStrings, + cty.UnknownVal(cty.Number), + cty.NumberIntVal(200), + cty.UnknownVal(cty.List(cty.String)), + true, + }, + { // list with unknown start offset but end < 0 + listOfStrings, + cty.UnknownVal(cty.Number), + cty.NumberIntVal(-4), + cty.UnknownVal(cty.List(cty.String)), + true, + }, + { // list with unknown end offset + listOfStrings, + cty.UnknownVal(cty.Number), + cty.NumberIntVal(0), + cty.UnknownVal(cty.List(cty.String)), + false, + }, + { // list with unknown end offset but start out of bounds + listOfStrings, + cty.UnknownVal(cty.Number), + cty.NumberIntVal(200), + cty.UnknownVal(cty.List(cty.String)), + true, + }, + { // list with unknown end offset but start < 0 + listOfStrings, + cty.UnknownVal(cty.Number), + cty.NumberIntVal(-3), + cty.UnknownVal(cty.List(cty.String)), + true, + }, + { // tuple slice with unknown start offset + tuple, + cty.UnknownVal(cty.Number), + cty.NumberIntVal(3), + cty.DynamicVal, + false, + }, + { // tuple slice with unknown start offset but end out of bounds + tuple, + cty.UnknownVal(cty.Number), + cty.NumberIntVal(200), + cty.DynamicVal, + true, + }, + { // tuple slice with unknown start offset but end < 0 + tuple, + cty.UnknownVal(cty.Number), + cty.NumberIntVal(-20), + cty.DynamicVal, + true, + }, + { // tuple slice with unknown end offset + tuple, + cty.NumberIntVal(0), + cty.UnknownVal(cty.Number), + cty.DynamicVal, + false, + }, + { // tuple slice with unknown end offset but start < 0 + tuple, + cty.NumberIntVal(-2), + cty.UnknownVal(cty.Number), + cty.DynamicVal, + true, + }, + { // tuple slice with unknown end offset but start out of bounds + tuple, + cty.NumberIntVal(200), + cty.UnknownVal(cty.Number), + cty.DynamicVal, + true, + }, } for i, test := range tests {