Forward to v1.6.0

CHANGELOG.md

@@ -1,3 +1,20 @@
+# 1.6.0 (Unreleased)
+
+* Fixed various defects in the handling of sets containing unknown values. This will cause unknown values to now be returned in more situations, whereas before `cty` would often return incorrect results when working with sets containing unknown values. The list of defects fixed in this release includes:
+    - `cty`: The length of a set containing unknown values, as defined by `Value.Length`, is itself unknown, reflecting the fact that unknown values may be placeholders for values that are equal to other values in the set, which would thus coalesce into a single value.
+    - `cty:` Converting a set with unknown values to a list produces an unknown value, because type conversion can't predict which indices each element of the set should take (the unknown elements could appear anywhere in the sort order) or the length of the resulting list.
+    - `function/stdlib`: the `LengthFunc` and `ToList` functions wrap the behaviors described in the previous two items and are therefore also fixed in the same way.
+    - `function/stclib`: `FormatListFunc` depends on knowing the length of all of its sequence arguments (which includes support for sets), so it will return an unknown result if given a set with an unknown length.
+    - `function/stdlib`: The various set operation functions were previously producing incorrect results if one of their given sets contained unknown values, because they didn't consider that unknown values on one set may be placeholders for values that are equal to elements of the other set. For example, `SetSubtractFunc` now produces a wholly-unknown result if either of its arguments contains an unknown element, because it can't predict whether that unknown element represents a value equal to an element in the other set.
+    - `cty`: The `Value.Equal` function would previously incorrectly return a known `cty.False` if one of the given sets contained an unknown value. It will now return `cty.UnknownVal(cty.Bool)` in that case, reflecting that the result could be either `cty.True` or `cty.False` were the unknown values to be replaced with known values.
+    - `cty`: The `Value.LengthInt` function was also returning incorrect results for sets containing unknown elements. However, given that it is commonly used in conjunction with `ElementIterator` to determine the capacity for a slice to append elements to, it is not fixed and is instead redefined to return the _maximum possible length_, which would result if all of the unknown values represent values that are not equal to any other set element. Applications that use `Value.LengthInt` to determine lengths to return to users who are working in the space of `cty` values should switch to using `Value.Length` instead and handle the possibility of the length being unknown, to avoid returning incorrect results for sets with unknown values.
+
+    These are not classified as breaking changes because the previous behavior was defective per the design goals for unknown values. However, callers may notice their application behavior changes along with these fixes when upgrading. The new behaviors should all be more correct than the old; if you observe a change in behavior where there is now an _incorrect_ result for sets containing unknown values (that is, where `cty` claims it knows an answer that it should not actually know), please report that in a GitHub issue.
+
+    We advise callers which work with sets that may potentially contain unknown values to review their own set-handling functions to check if they too might be handling sets with unknown values incorrectly, particularly if they work with sets using [integration methods rather than operation methods](./docs/types.md#common-operations-and-integration-methods) (for example, using `Value.ValueList` or `Value.ValueSet` to extract elements directly). It seems that incorrect handling of sets with unknown values has been a common hazard, particularly in codepaths that aim to treat lists and sets as being interchangable.
+* `function/stdlib`: The `element` function will no longer panic if given a negative index. Instead, it will return a proper error. ([#62](https://github.com/zclconf/go-cty/pull/62))
+* `convert`: **Experimental** support for annotating one or more attributes of an object type as "optional", which the `convert` package can then use to suppress the error that would normally be returned if the source type has no corresponding attribute, and can substitute a correctly-typed null value instead. This new behavior is subject to change even in minor release of `cty`, until it has been tested in experimental releases of downstream applications and potentially modified in response.
+
 # 1.5.1 (Unreleased)
 
 * `function/stdlib`: The `merge` function will no longer panic if all given maps are empty. ([#58](https://github.com/zclconf/go-cty/pull/58))

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)
@@ -458,6 +466,16 @@ func conversionMapToObject(mapType cty.Type, objType cty.Type, unsafe bool) conv
 			elems[name.AsString()] = val
 		}
 
+		for name, aty := range objectAtys {
+			if _, exists := elems[name]; !exists {
+				if optional := objType.AttributeOptional(name); optional {
+					elems[name] = cty.NullVal(aty)
+				} else {
+					return cty.NilVal, path.NewErrorf("map has no element for required attribute %q", name)
+				}
+			}
+		}
+
 		return cty.ObjectVal(elems), nil
 	}
 }

cty/convert/conversion_object.go

@@ -11,17 +11,29 @@ import (
 // type, meaning that each attribute of the output type has a corresponding
 // attribute in the input type where a recursive conversion is available.
 //
+// If the "out" type has any optional attributes, those attributes may be
+// absent in the "in" type, in which case null values will be used in their
+// place in the result.
+//
 // Shallow object conversions work the same for both safe and unsafe modes,
 // but the safety flag is passed on to recursive conversions and may thus
 // limit the above definition of "subset".
 func conversionObjectToObject(in, out cty.Type, unsafe bool) conversion {
 	inAtys := in.AttributeTypes()
 	outAtys := out.AttributeTypes()
+	outOptionals := out.OptionalAttributes()
 	attrConvs := make(map[string]conversion)
 
 	for name, outAty := range outAtys {
 		inAty, exists := inAtys[name]
 		if !exists {
+			if _, optional := outOptionals[name]; optional {
+				// If it's optional then we'll skip inserting an
+				// attribute conversion and then deal with inserting
+				// the default value in our overall conversion logic
+				// later.
+				continue
+			}
 			// No conversion is available, then.
 			return nil
 		}
@@ -71,6 +83,13 @@ func conversionObjectToObject(in, out cty.Type, unsafe bool) conversion {
 			attrVals[name] = val
 		}
 
+		for name := range outOptionals {
+			if _, exists := attrVals[name]; !exists {
+				wantTy := outAtys[name]
+				attrVals[name] = cty.NullVal(wantTy)
+			}
+		}
+
 		return cty.ObjectVal(attrVals), nil
 	}
 }

cty/convert/mismatch_msg.go

@@ -78,7 +78,9 @@ func mismatchMessageObjects(got, want cty.Type) string {
 	for name, wantAty := range wantAtys {
 		gotAty, exists := gotAtys[name]
 		if !exists {
-			missingAttrs = append(missingAttrs, name)
+			if !want.AttributeOptional(name) {
+				missingAttrs = append(missingAttrs, name)
+			}
 			continue
 		}
 

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),
@@ -381,6 +410,32 @@ func TestConvert(t *testing.T) {
 				"name": cty.StringVal("John"),
 			}),
 		},
+		{
+			Value: cty.MapVal(map[string]cty.Value{
+				"name": cty.StringVal("John"),
+			}),
+			Type: cty.Object(map[string]cty.Type{
+				"name":     cty.String,
+				"greeting": cty.String,
+			}),
+			WantError: true, // map has no element for required attribute "greeting"
+		},
+		{
+			Value: cty.MapVal(map[string]cty.Value{
+				"name": cty.StringVal("John"),
+			}),
+			Type: cty.ObjectWithOptionalAttrs(
+				map[string]cty.Type{
+					"name":     cty.String,
+					"greeting": cty.String,
+				},
+				[]string{"greeting"},
+			),
+			Want: cty.ObjectVal(map[string]cty.Value{
+				"greeting": cty.NullVal(cty.String),
+				"name":     cty.StringVal("John"),
+			}),
+		},
 		{
 			Value: cty.MapVal(map[string]cty.Value{
 				"a": cty.NumberIntVal(2),
@@ -476,6 +531,50 @@ func TestConvert(t *testing.T) {
 			}),
 			WantError: true, // given value must have superset object type
 		},
+		{
+			Value: cty.ObjectVal(map[string]cty.Value{
+				"bar": cty.StringVal("bar value"),
+			}),
+			Type: cty.ObjectWithOptionalAttrs(
+				map[string]cty.Type{
+					"foo": cty.String,
+					"bar": cty.String,
+				},
+				[]string{"foo"},
+			),
+			Want: cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.NullVal(cty.String),
+				"bar": cty.StringVal("bar value"),
+			}),
+		},
+		{
+			Value: cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.StringVal("foo value"),
+				"bar": cty.StringVal("bar value"),
+			}),
+			Type: cty.ObjectWithOptionalAttrs(
+				map[string]cty.Type{
+					"foo": cty.String,
+					"bar": cty.String,
+				},
+				[]string{"foo"},
+			),
+			Want: cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.StringVal("foo value"),
+				"bar": cty.StringVal("bar value"),
+			}),
+		},
+		{
+			Value: cty.EmptyObjectVal,
+			Type: cty.ObjectWithOptionalAttrs(
+				map[string]cty.Type{
+					"foo": cty.String,
+					"bar": cty.String,
+				},
+				[]string{"foo"},
+			),
+			WantError: true, // Attribute "bar" is required
+		},
 		{
 			Value: cty.ObjectVal(map[string]cty.Value{
 				"foo": cty.True,

cty/function/stdlib/collection.go

@@ -138,6 +138,13 @@ var ElementFunc = function.New(&function.Spec{
 	},
 	Type: func(args []cty.Value) (cty.Type, error) {
 		list := args[0]
+		index := args[1]
+		if index.IsKnown() {
+			if index.LessThan(cty.NumberIntVal(0)).True() {
+				return cty.DynamicPseudoType, fmt.Errorf("cannot use element function with a negative index")
+			}
+		}
+
 		listTy := list.Type()
 		switch {
 		case listTy.IsListType():
@@ -173,6 +180,10 @@ var ElementFunc = function.New(&function.Spec{
 			return cty.DynamicVal, fmt.Errorf("invalid index: %s", err)
 		}
 
+		if args[1].LessThan(cty.NumberIntVal(0)).True() {
+			return cty.DynamicVal, fmt.Errorf("cannot use element function with a negative index")
+		}
+
 		if !args[0].IsKnown() {
 			return cty.UnknownVal(retType), nil
 		}
@@ -492,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()
@@ -872,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.
@@ -1019,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
 	}