Optional attributes fixes

cty/convert/conversion.go

@@ -33,14 +33,25 @@ func getConversion(in cty.Type, out cty.Type, unsafe bool) conversion {
 			// Conversion to DynamicPseudoType always just passes through verbatim.
 			return in, nil
 		}
-		if !in.IsKnown() {
-			return cty.UnknownVal(out), nil
-		}
-		if in.IsNull() {
-			// We'll pass through nulls, albeit type converted, and let
-			// the caller deal with whatever handling they want to do in
-			// case null values are considered valid in some applications.
-			return cty.NullVal(out), nil
+		if isKnown, isNull := in.IsKnown(), in.IsNull(); !isKnown || isNull {
+			// Avoid constructing unknown or null values with types which
+			// include optional attributes. Known or non-null object values
+			// will be passed to a conversion function which drops the optional
+			// attributes from the type. Unknown and null pass through values
+			// must do the same to ensure that homogeneous collections have a
+			// single element type.
+			out = out.WithoutOptionalAttributesDeep()
+
+			if !isKnown {
+				return cty.UnknownVal(out), nil
+			}
+
+			if isNull {
+				// We'll pass through nulls, albeit type converted, and let
+				// the caller deal with whatever handling they want to do in
+				// case null values are considered valid in some applications.
+				return cty.NullVal(out), nil
+			}
 		}
 
 		return conv(in, path)

cty/convert/public_test.go

@@ -575,6 +575,45 @@ func TestConvert(t *testing.T) {
 			),
 			WantError: true, // Attribute "bar" is required
 		},
+		{
+			Value: cty.NullVal(cty.DynamicPseudoType),
+			Type: cty.ObjectWithOptionalAttrs(
+				map[string]cty.Type{
+					"foo": cty.String,
+					"bar": cty.String,
+				},
+				[]string{"foo"},
+			),
+			Want: cty.NullVal(cty.Object(map[string]cty.Type{
+				"foo": cty.String,
+				"bar": cty.String,
+			})),
+		},
+		{
+			Value: cty.ListVal([]cty.Value{
+				cty.NullVal(cty.DynamicPseudoType),
+				cty.ObjectVal(map[string]cty.Value{
+					"bar": cty.StringVal("bar value"),
+				}),
+			}),
+			Type: cty.List(cty.ObjectWithOptionalAttrs(
+				map[string]cty.Type{
+					"foo": cty.String,
+					"bar": cty.String,
+				},
+				[]string{"foo"},
+			)),
+			Want: cty.ListVal([]cty.Value{
+				cty.NullVal(cty.Object(map[string]cty.Type{
+					"foo": cty.String,
+					"bar": cty.String,
+				})),
+				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.True,
@@ -748,6 +787,92 @@ func TestConvert(t *testing.T) {
 				"b": cty.MapValEmpty(cty.String),
 			}),
 		},
+		// reduction of https://github.com/hashicorp/terraform/issues/27269
+		{
+			Value: cty.TupleVal([]cty.Value{
+				cty.ObjectVal(map[string]cty.Value{
+					"a": cty.NullVal(cty.DynamicPseudoType),
+				}),
+				cty.ObjectVal(map[string]cty.Value{
+					"a": cty.ObjectVal(map[string]cty.Value{
+						"b": cty.ListVal([]cty.Value{
+							cty.ObjectVal(map[string]cty.Value{
+								"c": cty.StringVal("d"),
+							}),
+						}),
+					}),
+				}),
+			}),
+			Type: cty.List(cty.Object(map[string]cty.Type{
+				"a": cty.Object(map[string]cty.Type{
+					"b": cty.List(cty.ObjectWithOptionalAttrs(map[string]cty.Type{
+						"c": cty.String,
+						"d": cty.String,
+					}, []string{"d"})),
+				}),
+			})),
+			Want: cty.ListVal([]cty.Value{
+				cty.ObjectVal(map[string]cty.Value{
+					"a": cty.NullVal(cty.Object(map[string]cty.Type{
+						"b": cty.List(cty.Object(map[string]cty.Type{
+							"c": cty.String,
+							"d": cty.String,
+						})),
+					})),
+				}),
+				cty.ObjectVal(map[string]cty.Value{
+					"a": cty.ObjectVal(map[string]cty.Value{
+						"b": cty.ListVal([]cty.Value{
+							cty.ObjectVal(map[string]cty.Value{
+								"c": cty.StringVal("d"),
+								"d": cty.NullVal(cty.String),
+							}),
+						}),
+					}),
+				}),
+			}),
+		},
+		// When converting null values into nested types which include objects
+		// with optional attributes, we expect the resulting value to be of a
+		// recursively concretized type.
+		{
+			Value: cty.NullVal(cty.DynamicPseudoType),
+			Type: cty.Object(
+				map[string]cty.Type{
+					"foo": cty.ObjectWithOptionalAttrs(
+						map[string]cty.Type{
+							"bar": cty.String,
+						},
+						[]string{"bar"},
+					),
+				},
+			),
+			Want: cty.NullVal(cty.Object(map[string]cty.Type{
+				"foo": cty.Object(map[string]cty.Type{
+					"bar": cty.String,
+				}),
+			})),
+		},
+		// The same nested optional attributes flattening should happen for
+		// unknown values, too.
+		{
+			Value: cty.UnknownVal(cty.DynamicPseudoType),
+			Type: cty.Object(
+				map[string]cty.Type{
+					"foo": cty.ObjectWithOptionalAttrs(
+						map[string]cty.Type{
+							"bar": cty.String,
+						},
+						[]string{"bar"},
+					),
+				},
+			),
+			Want: cty.UnknownVal(cty.Object(map[string]cty.Type{
+				"foo": cty.Object(map[string]cty.Type{
+					"bar": cty.String,
+				}),
+			})),
+		},
 		// https://github.com/hashicorp/terraform/issues/21588:
 		{
 			Value: cty.TupleVal([]cty.Value{

cty/convert/unify.go

@@ -29,13 +29,19 @@ func unify(types []cty.Type, unsafe bool) (cty.Type, []Conversion) {
 	// unification purposes.
 	{
 		mapCt := 0
+		listCt := 0
+		setCt := 0
 		objectCt := 0
 		tupleCt := 0
 		dynamicCt := 0
 		for _, ty := range types {
 			switch {
 			case ty.IsMapType():
 				mapCt++
+			case ty.IsListType():
+				listCt++
+			case ty.IsSetType():
+				setCt++
 			case ty.IsObjectType():
 				objectCt++
 			case ty.IsTupleType():
@@ -48,7 +54,11 @@ func unify(types []cty.Type, unsafe bool) (cty.Type, []Conversion) {
 		}
 		switch {
 		case mapCt > 0 && (mapCt+dynamicCt) == len(types):
-			return unifyMapTypes(types, unsafe, dynamicCt > 0)
+			return unifyCollectionTypes(cty.Map, types, unsafe, dynamicCt > 0)
+		case listCt > 0 && (listCt+dynamicCt) == len(types):
+			return unifyCollectionTypes(cty.List, types, unsafe, dynamicCt > 0)
+		case setCt > 0 && (setCt+dynamicCt) == len(types):
+			return unifyCollectionTypes(cty.Set, types, unsafe, dynamicCt > 0)
 		case objectCt > 0 && (objectCt+dynamicCt) == len(types):
 			return unifyObjectTypes(types, unsafe, dynamicCt > 0)
 		case tupleCt > 0 && (tupleCt+dynamicCt) == len(types):
@@ -100,7 +110,7 @@ Preferences:
 	return cty.NilType, nil
 }
 
-func unifyMapTypes(types []cty.Type, unsafe bool, hasDynamic bool) (cty.Type, []Conversion) {
+func unifyCollectionTypes(collectionType func(cty.Type) cty.Type, types []cty.Type, unsafe bool, hasDynamic bool) (cty.Type, []Conversion) {
 	// If we had any dynamic types in the input here then we can't predict
 	// what path we'll take through here once these become known types, so
 	// we'll conservatively produce DynamicVal for these.
@@ -117,7 +127,7 @@ func unifyMapTypes(types []cty.Type, unsafe bool, hasDynamic bool) (cty.Type, []
 		return cty.NilType, nil
 	}
 
-	retTy := cty.Map(retElemType)
+	retTy := collectionType(retElemType)
 
 	conversions := make([]Conversion, len(types))
 	for i, ty := range types {

cty/type.go

@@ -115,6 +115,44 @@ func (t Type) HasDynamicTypes() bool {
 	}
 }
 
+// WithoutOptionalAttributesDeep returns a type equivalent to the receiver but
+// with any objects with optional attributes converted into fully concrete
+// object types. This operation is applied recursively.
+func (t Type) WithoutOptionalAttributesDeep() Type {
+	switch {
+	case t == DynamicPseudoType, t.IsPrimitiveType(), t.IsCapsuleType():
+		return t
+	case t.IsMapType():
+		return Map(t.ElementType().WithoutOptionalAttributesDeep())
+	case t.IsListType():
+		return List(t.ElementType().WithoutOptionalAttributesDeep())
+	case t.IsSetType():
+		return Set(t.ElementType().WithoutOptionalAttributesDeep())
+	case t.IsTupleType():
+		originalElemTypes := t.TupleElementTypes()
+		elemTypes := make([]Type, len(originalElemTypes))
+		for i, et := range originalElemTypes {
+			elemTypes[i] = et.WithoutOptionalAttributesDeep()
+		}
+		return Tuple(elemTypes)
+	case t.IsObjectType():
+		originalAttrTypes := t.AttributeTypes()
+		attrTypes := make(map[string]Type, len(originalAttrTypes))
+		for k, t := range originalAttrTypes {
+			attrTypes[k] = t.WithoutOptionalAttributesDeep()
+		}
+
+		// This is the subtle line which does all the work of this function: by
+		// constructing a new Object type with these attribute types, we drop
+		// the list of optional attributes (if present). This results in a
+		// concrete Object type which requires all of the original attributes.
+		return Object(attrTypes)
+	default:
+		// Should never happen, since above should be exhaustive
+		panic("WithoutOptionalAttributesDeep does not support the given type")
+	}
+}
+
 type friendlyTypeNameMode rune
 
 const (

cty/type_test.go

@@ -55,6 +55,105 @@ func TestHasDynamicTypes(t *testing.T) {
 	}
 }
 
+func TestWithoutOptionalAttributesDeep(t *testing.T) {
+	tests := []struct {
+		ty       Type
+		expected Type
+	}{
+		{
+			DynamicPseudoType,
+			DynamicPseudoType,
+		},
+		{
+			List(DynamicPseudoType),
+			List(DynamicPseudoType),
+		},
+		{
+			Tuple([]Type{String, DynamicPseudoType}),
+			Tuple([]Type{String, DynamicPseudoType}),
+		},
+		{
+			Object(map[string]Type{
+				"a":       String,
+				"unknown": DynamicPseudoType,
+			}),
+			Object(map[string]Type{
+				"a":       String,
+				"unknown": DynamicPseudoType,
+			}),
+		},
+		{
+			ObjectWithOptionalAttrs(map[string]Type{
+				"a":       String,
+				"unknown": DynamicPseudoType,
+			}, []string{"a"}),
+			Object(map[string]Type{
+				"a":       String,
+				"unknown": DynamicPseudoType,
+			}),
+		},
+		{
+			Map(ObjectWithOptionalAttrs(map[string]Type{
+				"a":       String,
+				"unknown": DynamicPseudoType,
+			}, []string{"a"})),
+			Map(Object(map[string]Type{
+				"a":       String,
+				"unknown": DynamicPseudoType,
+			})),
+		},
+		{
+			Set(ObjectWithOptionalAttrs(map[string]Type{
+				"a":       String,
+				"unknown": DynamicPseudoType,
+			}, []string{"a"})),
+			Set(Object(map[string]Type{
+				"a":       String,
+				"unknown": DynamicPseudoType,
+			})),
+		},
+		{
+			List(ObjectWithOptionalAttrs(map[string]Type{
+				"a":       String,
+				"unknown": DynamicPseudoType,
+			}, []string{"a"})),
+			List(Object(map[string]Type{
+				"a":       String,
+				"unknown": DynamicPseudoType,
+			})),
+		},
+		{
+			Tuple([]Type{
+				ObjectWithOptionalAttrs(map[string]Type{
+					"a":       String,
+					"unknown": DynamicPseudoType,
+				}, []string{"a"}),
+				ObjectWithOptionalAttrs(map[string]Type{
+					"b": Number,
+				}, []string{"b"}),
+			}),
+			Tuple([]Type{
+				Object(map[string]Type{
+					"a":       String,
+					"unknown": DynamicPseudoType,
+				}),
+				Object(map[string]Type{
+					"b": Number,
+				}),
+			}),
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("%#v.HasDynamicTypes()", test.ty), func(t *testing.T) {
+			got := test.ty.WithoutOptionalAttributesDeep()
+			if !test.expected.Equals(got) {
+				t.Errorf("Equals returned %#v; want %#v", got, test.expected)
+			}
+		})
+	}
+}
+
 func TestNilTypeEquals(t *testing.T) {
 	var typ Type
 	if !typ.Equals(NilType) {