cty: Fix various quirks of handling sets with unknown values

Early in the implementation of cty I made an oversight which has,
unfortunately, played out as a variety of incorrect behaviors throughout
the cty functions: when a set contains unknown values, those unknown
values can potentially be standing in for values that are equal to others
in the set, and thus the effective length of the set can't be predicted.

Previously the Length function would return, in effect, the maximum
possible size of the set, which would result if all of the values
represented by the unknowns are non-equal. The caller might then get a
different known result from a call with unknowns than from a subsequent
call with those unknowns replaced with known values, which violates the
guarantees that cty intends to make when handling unknown values.

This changeset therefore starts by addressing that root bug: Length will
now return an unknown number if called on a set containing unknown values,
correctly representing that the final length is unpredictable.

This in turn had some downstream consequences for other functionality.
In particular, it should not have been possible to convert a set whose
length is unknown to a list, because it's not possible for a list to have
an unknown length. Therefore this changeset also includes a fix to the
convert package so that an unknown-length set converts to an unknown
list, which also addresses the related problem that a conversion from a
set to list can't predict where the unknown values will appear in the
sort order and thus can't predict the list indices even for the known
elements.

The rest of the changes here are similar adjustments to account for the
possibility of an unknown set length, in most cases similarly returning
an unknown result.

This changeset causes a difference in observable behavior from the
previous version, but it's not considered to be a breaking change because
the previous behavior was defective. With that said, callers that were
inadvertently depending on the defective behavior will find that their
calling application now behaves slightly differently, and so may wish to
make adjustments if the defective prior behavior was actually desirable
for some unusual situation.

As a pragmatic accommodation for existing callers, the Value.LengthInt
function is now defined as returning the maximum possible length in
situations where the length is unknown. This aligns with the number of
iterations a caller would encounter using an ElementIterator on such a
value, and also reflects a suitable capacity to use when allocating a
Go slice to append iterated elements into.

cty/convert/conversion_collection.go

@@ -13,6 +13,14 @@ import (
 // if we're converting from a set into a list of the same element type.)
 func conversionCollectionToList(ety cty.Type, conv conversion) conversion {
 	return func(val cty.Value, path cty.Path) (cty.Value, error) {
+		if !val.Length().IsKnown() {
+			// If the input collection has an unknown length (which is true
+			// for a set containing unknown values) then our result must be
+			// an unknown list, because we can't predict how many elements
+			// the resulting list should have.
+			return cty.UnknownVal(cty.List(val.Type().ElementType())), nil
+		}
+
 		elems := make([]cty.Value, 0, val.LengthInt())
 		i := int64(0)
 		elemPath := append(path.Copy(), nil)

cty/convert/public_test.go

@@ -143,6 +143,14 @@ func TestConvert(t *testing.T) {
 			Type:      cty.List(cty.DynamicPseudoType),
 			WantError: true, // there is no type that both tuple elements can unify to for conversion to list
 		},
+		{
+			Value: cty.SetVal([]cty.Value{
+				cty.StringVal("5"),
+				cty.UnknownVal(cty.String),
+			}),
+			Type: cty.Set(cty.Number),
+			Want: cty.SetVal([]cty.Value{cty.NumberIntVal(5), cty.UnknownVal(cty.Number)}),
+		},
 		{
 			Value: cty.SetVal([]cty.Value{
 				cty.StringVal("5"),
@@ -182,6 +190,27 @@ func TestConvert(t *testing.T) {
 				cty.StringVal("10"),
 			}),
 		},
+		{
+			Value: cty.SetVal([]cty.Value{
+				cty.StringVal("5"),
+				cty.UnknownVal(cty.String),
+			}),
+			Type: cty.List(cty.String),
+			Want: cty.UnknownVal(cty.List(cty.String)),
+		},
+		{
+			Value: cty.SetVal([]cty.Value{
+				cty.UnknownVal(cty.String),
+			}),
+			Type: cty.List(cty.String),
+			// We get a known list value this time because even though we
+			// don't know the single value that's in the list, we _do_ know
+			// that there are no other values in the set for it to coalesce
+			// with.
+			Want: cty.ListVal([]cty.Value{
+				cty.UnknownVal(cty.String),
+			}),
+		},
 		{
 			Value: cty.ListVal([]cty.Value{
 				cty.NumberIntVal(5),

cty/function/stdlib/collection.go

@@ -503,6 +503,11 @@ var FlattenFunc = function.New(&function.Spec{
 // We can flatten lists with unknown values, as long as they are not
 // lists themselves.
 func flattener(flattenList cty.Value) ([]cty.Value, bool) {
+	if !flattenList.Length().IsKnown() {
+		// If we don't know the length of what we're flattening then we can't
+		// predict the length of our result yet either.
+		return nil, false
+	}
 	out := make([]cty.Value, 0)
 	for it := flattenList.ElementIterator(); it.Next(); {
 		_, val := it.Element()
@@ -883,6 +888,10 @@ var SetProductFunc = function.New(&function.Spec{
 
 		total := 1
 		for _, arg := range args {
+			if !arg.Length().IsKnown() {
+				return cty.UnknownVal(retType), nil
+			}
+
 			// Because of our type checking function, we are guaranteed that
 			// all of the arguments are known, non-null values of types that
 			// support LengthInt.
@@ -1030,7 +1039,8 @@ func sliceIndexes(args []cty.Value) (int, int, bool, error) {
 	var startIndex, endIndex, length int
 	var startKnown, endKnown, lengthKnown bool
 
-	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
+	// If it's a tuple then we always know the length by the type, but collections might be unknown or have unknown length
+	if args[0].Type().IsTupleType() || args[0].Length().IsKnown() {
 		length = args[0].LengthInt()
 		lengthKnown = true
 	}

cty/function/stdlib/collection_test.go

@@ -169,6 +169,16 @@ func TestContains(t *testing.T) {
 			cty.BoolVal(true),
 			false,
 		},
+		{
+			cty.ListVal([]cty.Value{
+				cty.UnknownVal(cty.String),
+				cty.StringVal("brown"),
+				cty.StringVal("fox"),
+			}),
+			cty.StringVal("quick"),
+			cty.UnknownVal(cty.Bool),
+			false,
+		},
 		{ // set val
 			cty.SetVal([]cty.Value{
 				cty.StringVal("quick"),
@@ -179,6 +189,16 @@ func TestContains(t *testing.T) {
 			cty.BoolVal(true),
 			false,
 		},
+		{
+			cty.SetVal([]cty.Value{
+				cty.UnknownVal(cty.String),
+				cty.StringVal("brown"),
+				cty.StringVal("fox"),
+			}),
+			cty.StringVal("quick"),
+			cty.UnknownVal(cty.Bool),
+			false,
+		},
 		{ // nested unknown
 			cty.ListVal([]cty.Value{
 				cty.ObjectVal(map[string]cty.Value{
@@ -222,6 +242,7 @@ func TestContains(t *testing.T) {
 		})
 	}
 }
+
 func TestMerge(t *testing.T) {
 	tests := []struct {
 		Values []cty.Value
@@ -630,6 +651,18 @@ func TestLength(t *testing.T) {
 			cty.SetVal([]cty.Value{cty.True}),
 			cty.NumberIntVal(1),
 		},
+		{
+			cty.SetVal([]cty.Value{cty.True, cty.False}),
+			cty.NumberIntVal(2),
+		},
+		{
+			cty.SetVal([]cty.Value{cty.True, cty.UnknownVal(cty.Bool)}),
+			cty.UnknownVal(cty.Number), // Don't know if the unknown in the input represents cty.True or cty.False
+		},
+		{
+			cty.SetVal([]cty.Value{cty.UnknownVal(cty.Bool)}),
+			cty.NumberIntVal(1), // Will be one regardless of what value the unknown in the input is representing
+		},
 		{
 			cty.MapValEmpty(cty.Bool),
 			cty.NumberIntVal(0),

cty/function/stdlib/format.go

@@ -85,7 +85,7 @@ var FormatListFunc = function.New(&function.Spec{
 			argTy := arg.Type()
 			switch {
 			case (argTy.IsListType() || argTy.IsSetType() || argTy.IsTupleType()) && !arg.IsNull():
-				if !argTy.IsTupleType() && !arg.IsKnown() {
+				if !argTy.IsTupleType() && !(arg.IsKnown() && arg.Length().IsKnown()) {
 					// We can't iterate this one at all yet then, so we can't
 					// yet produce a result.
 					unknowns[i] = true

cty/function/stdlib/format_test.go

@@ -825,6 +825,17 @@ func TestFormatList(t *testing.T) {
 			cty.UnknownVal(cty.List(cty.String)),
 			`argument 2 has length 2, which is inconsistent with argument 1 of length 1`,
 		},
+		22: {
+			cty.StringVal("%v"),
+			[]cty.Value{
+				cty.SetVal([]cty.Value{
+					cty.StringVal("hello"),
+					cty.UnknownVal(cty.String),
+				}),
+			},
+			cty.UnknownVal(cty.List(cty.String)),
+			``,
+		},
 	}
 
 	for i, test := range tests {

cty/function/stdlib/set.go

@@ -44,7 +44,7 @@ var SetUnionFunc = function.New(&function.Spec{
 	Type: setOperationReturnType,
 	Impl: setOperationImpl(func(s1, s2 cty.ValueSet) cty.ValueSet {
 		return s1.Union(s2)
-	}),
+	}, true),
 })
 
 var SetIntersectionFunc = function.New(&function.Spec{
@@ -63,7 +63,7 @@ var SetIntersectionFunc = function.New(&function.Spec{
 	Type: setOperationReturnType,
 	Impl: setOperationImpl(func(s1, s2 cty.ValueSet) cty.ValueSet {
 		return s1.Intersection(s2)
-	}),
+	}, false),
 })
 
 var SetSubtractFunc = function.New(&function.Spec{
@@ -82,7 +82,7 @@ var SetSubtractFunc = function.New(&function.Spec{
 	Type: setOperationReturnType,
 	Impl: setOperationImpl(func(s1, s2 cty.ValueSet) cty.ValueSet {
 		return s1.Subtract(s2)
-	}),
+	}, false),
 })
 
 var SetSymmetricDifferenceFunc = function.New(&function.Spec{
@@ -100,8 +100,8 @@ var SetSymmetricDifferenceFunc = function.New(&function.Spec{
 	},
 	Type: setOperationReturnType,
 	Impl: setOperationImpl(func(s1, s2 cty.ValueSet) cty.ValueSet {
-		return s1.Subtract(s2)
-	}),
+		return s1.SymmetricDifference(s2)
+	}, false),
 })
 
 // SetHasElement determines whether the given set contains the given value as an
@@ -187,20 +187,32 @@ func setOperationReturnType(args []cty.Value) (ret cty.Type, err error) {
 	return cty.Set(newEty), nil
 }
 
-func setOperationImpl(f func(s1, s2 cty.ValueSet) cty.ValueSet) function.ImplFunc {
+func setOperationImpl(f func(s1, s2 cty.ValueSet) cty.ValueSet, allowUnknowns bool) function.ImplFunc {
 	return func(args []cty.Value, retType cty.Type) (ret cty.Value, err error) {
 		first := args[0]
 		first, err = convert.Convert(first, retType)
 		if err != nil {
 			return cty.NilVal, function.NewArgError(0, err)
 		}
+		if !allowUnknowns && !first.IsWhollyKnown() {
+			// This set function can produce a correct result only when all
+			// elements are known, because eventually knowing the unknown
+			// values may cause the result to have fewer known elements, or
+			// might cause a result with no unknown elements at all to become
+			// one with a different length.
+			return cty.UnknownVal(retType), nil
+		}
 
 		set := first.AsValueSet()
 		for i, arg := range args[1:] {
 			arg, err := convert.Convert(arg, retType)
 			if err != nil {
 				return cty.NilVal, function.NewArgError(i+1, err)
 			}
+			if !allowUnknowns && !arg.IsWhollyKnown() {
+				// (For the same reason as we did this check for "first" above.)
+				return cty.UnknownVal(retType), nil
+			}
 
 			argSet := arg.AsValueSet()
 			set = f(set, argSet)

cty/function/stdlib/set_test.go

@@ -83,6 +83,13 @@ func TestSetUnion(t *testing.T) {
 			},
 			cty.UnknownVal(cty.Set(cty.String)),
 		},
+		{
+			[]cty.Value{
+				cty.SetVal([]cty.Value{cty.StringVal("5")}),
+				cty.SetVal([]cty.Value{cty.UnknownVal(cty.String)}),
+			},
+			cty.SetVal([]cty.Value{cty.StringVal("5"), cty.UnknownVal(cty.String)}),
+		},
 	}
 
 	for _, test := range tests {
@@ -173,6 +180,13 @@ func TestSetIntersection(t *testing.T) {
 			},
 			cty.UnknownVal(cty.Set(cty.String)),
 		},
+		{
+			[]cty.Value{
+				cty.SetVal([]cty.Value{cty.StringVal("5")}),
+				cty.SetVal([]cty.Value{cty.UnknownVal(cty.String)}),
+			},
+			cty.UnknownVal(cty.Set(cty.String)),
+		},
 	}
 
 	for _, test := range tests {
@@ -245,6 +259,11 @@ func TestSetSubtract(t *testing.T) {
 			cty.UnknownVal(cty.Set(cty.Number)),
 			cty.UnknownVal(cty.Set(cty.String)),
 		},
+		{
+			cty.SetVal([]cty.Value{cty.StringVal("5")}),
+			cty.SetVal([]cty.Value{cty.UnknownVal(cty.String)}),
+			cty.UnknownVal(cty.Set(cty.String)),
+		},
 	}
 
 	for _, test := range tests {
@@ -261,3 +280,80 @@ func TestSetSubtract(t *testing.T) {
 		})
 	}
 }
+
+func TestSetSymmetricDifference(t *testing.T) {
+	tests := []struct {
+		InputA cty.Value
+		InputB cty.Value
+		Want   cty.Value
+	}{
+		{
+			cty.SetValEmpty(cty.String),
+			cty.SetValEmpty(cty.String),
+			cty.SetValEmpty(cty.String),
+		},
+		{
+			cty.SetVal([]cty.Value{cty.True}),
+			cty.SetValEmpty(cty.String),
+			cty.SetVal([]cty.Value{cty.StringVal("true")}),
+		},
+		{
+			cty.SetVal([]cty.Value{cty.True}),
+			cty.SetVal([]cty.Value{cty.False}),
+			cty.SetVal([]cty.Value{cty.True, cty.False}),
+		},
+		{
+			cty.SetVal([]cty.Value{
+				cty.StringVal("a"),
+				cty.StringVal("b"),
+				cty.StringVal("c"),
+			}),
+			cty.SetVal([]cty.Value{
+				cty.StringVal("a"),
+				cty.StringVal("c"),
+			}),
+			cty.SetVal([]cty.Value{
+				cty.StringVal("b"),
+			}),
+		},
+		{
+			cty.SetVal([]cty.Value{cty.StringVal("a")}),
+			cty.SetValEmpty(cty.DynamicPseudoType),
+			cty.SetVal([]cty.Value{cty.StringVal("a")}),
+		},
+		{
+			cty.SetVal([]cty.Value{cty.EmptyObjectVal}),
+			cty.SetValEmpty(cty.DynamicPseudoType),
+			cty.SetVal([]cty.Value{cty.EmptyObjectVal}),
+		},
+		{
+			cty.SetValEmpty(cty.DynamicPseudoType),
+			cty.SetValEmpty(cty.DynamicPseudoType),
+			cty.SetValEmpty(cty.DynamicPseudoType),
+		},
+		{
+			cty.SetVal([]cty.Value{cty.StringVal("5")}),
+			cty.UnknownVal(cty.Set(cty.Number)),
+			cty.UnknownVal(cty.Set(cty.String)),
+		},
+		{
+			cty.SetVal([]cty.Value{cty.StringVal("5")}),
+			cty.SetVal([]cty.Value{cty.UnknownVal(cty.Number)}),
+			cty.UnknownVal(cty.Set(cty.String)),
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("SetSymmetricDifference(%#v, %#v)", test.InputA, test.InputB), func(t *testing.T) {
+			got, err := SetSymmetricDifference(test.InputA, test.InputB)
+
+			if err != nil {
+				t.Fatalf("unexpected error: %s", err)
+			}
+
+			if !got.RawEquals(test.Want) {
+				t.Errorf("wrong result\ngot:  %#v\nwant: %#v", got, test.Want)
+			}
+		})
+	}
+}

cty/unknown.go

@@ -5,7 +5,8 @@ package cty
 type unknownType struct {
 }
 
-// Unknown is a special value that can be
+// unknown is a special value that can be used as the internal value of a
+// Value to create a placeholder for a value that isn't yet known.
 var unknown interface{} = &unknownType{}
 
 // UnknownVal returns an Value that represents an unknown value of the given