Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strip dynamic types out of null and unknown values recursively #144

Merged
merged 3 commits into from
Nov 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cty/convert/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ func getConversion(in cty.Type, out cty.Type, unsafe bool) conversion {
out = out.WithoutOptionalAttributesDeep()

if !isKnown {
return cty.UnknownVal(out), nil
return cty.UnknownVal(dynamicReplace(in.Type(), out)), nil
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apparentlymart - I specifically wanted to fix Null values with this, but I think it makes sense that unknown and null values behave the same. In case, we don't want that let me know and I'll revert the change to this line.

}

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 cty.NullVal(dynamicReplace(in.Type(), out)), nil
}
}

Expand Down
104 changes: 104 additions & 0 deletions cty/convert/conversion_dynamic.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,107 @@ func dynamicFixup(wantType cty.Type) conversion {
func dynamicPassthrough(in cty.Value, path cty.Path) (cty.Value, error) {
return in, nil
}

// dynamicReplace aims to return the out type unchanged, but if it finds a
// dynamic type either directly or in any descendent elements it replaces them
// with the equivalent type from in.
//
// This function assumes that in and out are compatible from a Convert
// perspective, and will panic if it finds that they are not. For example if
// in is an object and out is a map, this function will still attempt to iterate
// through both as if they were the same.
func dynamicReplace(in, out cty.Type) cty.Type {
if in == cty.DynamicPseudoType || in == cty.NilType {
// Short circuit this case, there's no point worrying about this if in
// is a dynamic type or a nil type. Out is the best we can do.
return out
}

switch {
case out == cty.DynamicPseudoType:
// So replace out with in.
return in
case out.IsPrimitiveType(), out.IsCapsuleType():
// out is not dynamic and it doesn't contain descendent elements so just
// return it unchanged.
return out
case out.IsMapType():
var elemType cty.Type

// Maps are compatible with other maps or objects.
if in.IsMapType() {
elemType = dynamicReplace(in.ElementType(), out.ElementType())
}

if in.IsObjectType() {
var types []cty.Type
for _, t := range in.AttributeTypes() {
types = append(types, t)
}
unifiedType, _ := unify(types, true)
elemType = dynamicReplace(unifiedType, out.ElementType())
}

return cty.Map(elemType)
case out.IsObjectType():
// Objects are compatible with other objects and maps.
outTypes := map[string]cty.Type{}
if in.IsMapType() {
for attr, attrType := range out.AttributeTypes() {
outTypes[attr] = dynamicReplace(in.ElementType(), attrType)
}
}

if in.IsObjectType() {
for attr, attrType := range out.AttributeTypes() {
if !in.HasAttribute(attr) {
// If in does not have this attribute, then it is an
// optional attribute and there is nothing we can do except
// to return the type from out even if it is dynamic.
outTypes[attr] = attrType
continue
}
outTypes[attr] = dynamicReplace(in.AttributeType(attr), attrType)
}
}

return cty.Object(outTypes)
case out.IsSetType():
var elemType cty.Type

// Sets are compatible with other sets, lists, tuples.
if in.IsSetType() || in.IsListType() {
elemType = dynamicReplace(in.ElementType(), out.ElementType())
}

if in.IsTupleType() {
unifiedType, _ := unify(in.TupleElementTypes(), true)
elemType = dynamicReplace(unifiedType, out.ElementType())
}

return cty.Set(elemType)
case out.IsListType():
var elemType cty.Type

// Lists are compatible with other lists, sets, and tuples.
if in.IsSetType() || in.IsListType() {
elemType = dynamicReplace(in.ElementType(), out.ElementType())
}

if in.IsTupleType() {
unifiedType, _ := unify(in.TupleElementTypes(), true)
elemType = dynamicReplace(unifiedType, out.ElementType())
}

return cty.List(elemType)
case out.IsTupleType():
// Tuples are only compatible with other tuples
var types []cty.Type
for ix := 0; ix < len(out.TupleElementTypes()); ix++ {
types = append(types, dynamicReplace(in.TupleElementType(ix), out.TupleElementType(ix)))
}
return cty.Tuple(types)
default:
panic("unrecognized type " + out.FriendlyName())
}
}
85 changes: 85 additions & 0 deletions cty/convert/public_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1514,6 +1514,91 @@ func TestConvert(t *testing.T) {
})),
}),
},
// Collections should prefer concrete types over dynamic types.
{
Value: cty.ListValEmpty(cty.Number),
Type: cty.List(cty.DynamicPseudoType),
Want: cty.ListValEmpty(cty.Number),
},
{
Value: cty.NullVal(cty.List(cty.Number)),
Type: cty.List(cty.DynamicPseudoType),
Want: cty.NullVal(cty.List(cty.Number)),
},
{
Value: cty.NullVal(cty.List(cty.Number)),
Type: cty.Set(cty.DynamicPseudoType),
Want: cty.NullVal(cty.Set(cty.Number)),
},
{
Value: cty.MapValEmpty(cty.Number),
Type: cty.Map(cty.DynamicPseudoType),
Want: cty.MapValEmpty(cty.Number),
},
{
Value: cty.NullVal(cty.Map(cty.Number)),
Type: cty.Map(cty.DynamicPseudoType),
Want: cty.NullVal(cty.Map(cty.Number)),
},
{
Value: cty.NullVal(cty.Map(cty.Number)),
Type: cty.Object(map[string]cty.Type{
"a": cty.DynamicPseudoType,
}),
Want: cty.NullVal(cty.Object(map[string]cty.Type{
"a": cty.Number,
})),
},
{
Value: cty.SetValEmpty(cty.Number),
Type: cty.Set(cty.DynamicPseudoType),
Want: cty.SetValEmpty(cty.Number),
},
{
Value: cty.NullVal(cty.Set(cty.Number)),
Type: cty.Set(cty.DynamicPseudoType),
Want: cty.NullVal(cty.Set(cty.Number)),
},
{
Value: cty.NullVal(cty.Set(cty.Number)),
Type: cty.List(cty.DynamicPseudoType),
Want: cty.NullVal(cty.List(cty.Number)),
},
{
Value: cty.NullVal(cty.Object(map[string]cty.Type{
"a": cty.String,
})),
Type: cty.Map(cty.DynamicPseudoType),
Want: cty.NullVal(cty.Map(cty.String)),
},
{
Value: cty.NullVal(cty.Object(map[string]cty.Type{
"a": cty.Object(map[string]cty.Type{
"b": cty.String,
}),
})),
Type: cty.Object(map[string]cty.Type{
"a": cty.Object(map[string]cty.Type{
"b": cty.DynamicPseudoType,
}),
}),
Want: cty.NullVal(cty.Object(map[string]cty.Type{
"a": cty.Object(map[string]cty.Type{
"b": cty.String,
}),
})),
},
{
Value: cty.NullVal(cty.Tuple([]cty.Type{
cty.String,
})),
Type: cty.Tuple([]cty.Type{
cty.DynamicPseudoType,
}),
Want: cty.NullVal(cty.Tuple([]cty.Type{
cty.String,
})),
},
// We should strip optional attributes out of types even if they match.
{
Value: cty.MapVal(map[string]cty.Value{
Expand Down