Skip to content

Commit

Permalink
convert: Only consider present map elements when converting to object…
Browse files Browse the repository at this point in the history
… type

Optional object attributes give us a new situation to consider here: the
source element type might not be compatible with all of the optional
attributes of a target object type, but that doesn't matter if the given
map doesn't include an element corresponding with the mismatching
attributes.

This is a bit awkward because we need to first allow the type conversion
logic to produce a valid conversion but then catch the attribute mismatch
only when applying the conversion function to the value. This is
effectively the same sequence of events that happens when converting from
string to number or string to bool: we optimistally assume that a
conversion will succeed when looking only at types, and then catch the
error dynamically once we have a final value to check.
  • Loading branch information
liamcervante authored and apparentlymart committed Oct 19, 2022
1 parent 82eaa67 commit 6d8908a
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 5 deletions.
52 changes: 47 additions & 5 deletions cty/convert/conversion_collection.go
Expand Up @@ -448,13 +448,28 @@ func conversionMapToObject(mapType cty.Type, objType cty.Type, unsafe bool) conv

elemConvs[name] = getConversion(mapEty, objectAty, unsafe)
if elemConvs[name] == nil {
// If any of our element conversions are impossible, then the our
// whole conversion is impossible.
// This means that this conversion is impossible. Typically, we
// would give up at this point and declare the whole conversion
// impossible. But, if this attribute is optional then maybe we will
// be able to do this conversion anyway provided the actual concrete
// map doesn't have this value set.
//
// We only do this in "unsafe" mode, because we cannot guarantee
// that the returned conversion will actually succeed once applied.
if objType.AttributeOptional(name) && unsafe {
// This attribute is optional, so let's leave this conversion in
// as a nil, and we can error later if we actually have to
// convert this.
continue
}

// Otherwise, give up. This conversion is impossible as we have a
// required attribute that doesn't match the map's inner type.
return nil
}
}

// If we fall out here then a conversion is possible, using the
// If we fall out here then a conversion may be possible, using the
// element conversions in elemConvs
return func(val cty.Value, path cty.Path) (cty.Value, error) {
elems := make(map[string]cty.Value, len(elemConvs))
Expand All @@ -474,12 +489,39 @@ func conversionMapToObject(mapType cty.Type, objType cty.Type, unsafe bool) conv
Key: name,
}

conv := elemConvs[name.AsString()]
if conv != nil {
// There are 3 cases here:
// 1. This attribute is not in elemConvs
// 2. This attribute is in elemConvs and is not nil
// 3. This attribute is in elemConvs and is nil.

// In case 1, we do not enter any of the branches below. This case
// means the attribute type is the same between the map and the
// object, and we don't need to do any conversion.

if conv, ok := elemConvs[name.AsString()]; conv != nil {
// This is case 2. The attribute type is different between the
// map and the object, and we know how to convert between them.
// So, we reset val to be the converted value and carry on.
val, err = conv(val, elemPath)
if err != nil {
return cty.NilVal, err
}
} else if ok {
// This is case 3 and it is an error. The attribute types are
// different between the map and the object, but we cannot
// convert between them.
//
// Now typically, this would be picked earlier on when we were
// building elemConvs. However, in the case of optional
// attributes there was a chance we could still convert the
// overall object even if this particular attribute was not
// convertable. This is because it could have not been set in
// the map, and we could skip over it here and set a null value.
//
// Since we reached this branch, we know that map did actually
// contain a non-convertable optional attribute. This means we
// error.
return cty.NilVal, path.NewErrorf("map element type is incompatible with attribute %q: %s", name.AsString(), MismatchMessage(val.Type(), objType.AttributeType(name.AsString())))
}

elems[name.AsString()] = val
Expand Down
73 changes: 73 additions & 0 deletions cty/convert/public_test.go
Expand Up @@ -993,6 +993,79 @@ func TestConvert(t *testing.T) {
cty.NullVal(cty.DynamicPseudoType),
}),
},
{
Value: cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("boop"),
// It's okay to use a map of string to convert to this
// target type as long as the source map does not include
// any of the optional attributes that cannot be assigned
// from a string.
}),
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"a": cty.String,
"b": cty.String,
"c": cty.Object(map[string]cty.Type{
"d": cty.String,
}),
}, []string{"b", "c"}),
Want: cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("boop"),
"b": cty.NullVal(cty.String),
"c": cty.NullVal(cty.Object(map[string]cty.Type{
"d": cty.String,
})),
}),
},
{
Value: cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("boop"),
}),
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"a": cty.String,
"b": cty.String,
"c": cty.Object(map[string]cty.Type{
"d": cty.DynamicPseudoType,
}),
}, []string{"b", "c"}),
Want: cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("boop"),
"b": cty.NullVal(cty.String),
"c": cty.NullVal(cty.Object(map[string]cty.Type{
"d": cty.DynamicPseudoType,
})),
}),
},
{
Value: cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("boop"),
}),
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"a": cty.String,
"b": cty.String,
"c": cty.DynamicPseudoType,
}, []string{"b", "c"}),
Want: cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("boop"),
"b": cty.NullVal(cty.String),
"c": cty.NullVal(cty.DynamicPseudoType),
}),
},
{
Value: cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("boop"),
// This case is invalid, because an element of a map of
// string cannot be assigned to an object-typed attribute.
"c": cty.StringVal("foobar"),
}),
Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"a": cty.String,
"b": cty.String,
"c": cty.Object(map[string]cty.Type{
"d": cty.String,
}),
}, []string{"b", "c"}),
WantError: `map element type is incompatible with attribute "c": object required`,
},
{
Value: cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
Expand Down

0 comments on commit 6d8908a

Please sign in to comment.