Skip to content

Commit

Permalink
function/stdlib: LookupFunc precise handling of marks
Browse files Browse the repository at this point in the history
Several changes to Lookup to handle marks in a way that I think makes
more sense:

- If the entire collection is marked, preserve the marks on any result
  (whether successful or fallback)
- If a returned value from the collection is marked, preserve the marks
  from only that value, combined with any overall collection marks
- Retain marks on the fallback value when it is returned, combined with
  any overall collection marks
- Disregard marks on the key value, as map keys and object attributes
  cannot be marked
- Retain collection marks when returning an unknown value for a not
  wholly-known collection
  • Loading branch information
alisdair committed Apr 20, 2021
1 parent ed64ef9 commit ac6b489
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 13 deletions.
35 changes: 22 additions & 13 deletions cty/function/stdlib/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,16 +623,19 @@ var KeysFunc = function.New(&function.Spec{
var LookupFunc = function.New(&function.Spec{
Params: []function.Parameter{
{
Name: "inputMap",
Type: cty.DynamicPseudoType,
Name: "inputMap",
Type: cty.DynamicPseudoType,
AllowMarked: true,
},
{
Name: "key",
Type: cty.String,
Name: "key",
Type: cty.String,
AllowMarked: true,
},
{
Name: "default",
Type: cty.DynamicPseudoType,
Name: "default",
Type: cty.DynamicPseudoType,
AllowMarked: true,
},
},
Type: func(args []cty.Value) (ret cty.Type, err error) {
Expand All @@ -644,7 +647,8 @@ var LookupFunc = function.New(&function.Spec{
return cty.DynamicPseudoType, nil
}

key := args[1].AsString()
keyVal, _ := args[1].Unmark()
key := keyVal.AsString()
if ty.HasAttribute(key) {
return args[0].GetAttr(key).Type(), nil
} else if len(args) == 3 {
Expand All @@ -666,28 +670,33 @@ var LookupFunc = function.New(&function.Spec{
}
},
Impl: func(args []cty.Value, retType cty.Type) (ret cty.Value, err error) {
// leave default value marked
defaultVal := args[2]

mapVar := args[0]
lookupKey := args[1].AsString()
// unmark collection, retain marks to reapply later
mapVar, marks := args[0].Unmark()

// disregard marks on the key
keyVal, _ := args[1].Unmark()
lookupKey := keyVal.AsString()

if !mapVar.IsWhollyKnown() {
return cty.UnknownVal(retType), nil
return cty.UnknownVal(retType).WithMarks(marks), nil
}

if mapVar.Type().IsObjectType() {
if mapVar.Type().HasAttribute(lookupKey) {
return mapVar.GetAttr(lookupKey), nil
return mapVar.GetAttr(lookupKey).WithMarks(marks), nil
}
} else if mapVar.HasIndex(cty.StringVal(lookupKey)) == cty.True {
return mapVar.Index(cty.StringVal(lookupKey)), nil
return mapVar.Index(cty.StringVal(lookupKey)).WithMarks(marks), nil
}

defaultVal, err = convert.Convert(defaultVal, retType)
if err != nil {
return cty.NilVal, err
}
return defaultVal, nil
return defaultVal.WithMarks(marks), nil
},
})

Expand Down
61 changes: 61 additions & 0 deletions cty/function/stdlib/collection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,67 @@ func TestLookup(t *testing.T) {
cty.StringVal("nope"),
cty.StringVal("bar"),
},
{ // successful marked collection lookup returns marked value
cty.MapVal(map[string]cty.Value{
"boop": cty.StringVal("beep"),
}).Mark("a"),
cty.StringVal("boop"),
cty.StringVal("nope"),
cty.StringVal("beep").Mark("a"),
},
{ // apply collection marks to unknown return vaue
cty.MapVal(map[string]cty.Value{
"boop": cty.StringVal("beep"),
"frob": cty.UnknownVal(cty.String),
}).Mark("a"),
cty.StringVal("boop"),
cty.StringVal("nope"),
cty.UnknownVal(cty.String).Mark("a"),
},
{ // propagate collection marks to default when returning
cty.MapVal(map[string]cty.Value{
"boop": cty.StringVal("beep"),
}).Mark("a"),
cty.StringVal("frob"),
cty.StringVal("nope").Mark("b"),
cty.StringVal("nope").WithMarks(cty.NewValueMarks("a", "b")),
},
{ // on unmarked collection, return only marks from found value
cty.MapVal(map[string]cty.Value{
"boop": cty.StringVal("beep").Mark("a"),
"frob": cty.StringVal("honk").Mark("b"),
}),
cty.StringVal("frob"),
cty.StringVal("nope").Mark("c"),
cty.StringVal("honk").Mark("b"),
},
{ // on unmarked collection, return default exactly on missing
cty.MapVal(map[string]cty.Value{
"boop": cty.StringVal("beep").Mark("a"),
"frob": cty.StringVal("honk").Mark("b"),
}),
cty.StringVal("squish"),
cty.StringVal("nope").Mark("c"),
cty.StringVal("nope").Mark("c"),
},
{ // retain marks on default if converted
cty.MapVal(map[string]cty.Value{
"boop": cty.StringVal("beep").Mark("a"),
"frob": cty.StringVal("honk").Mark("b"),
}),
cty.StringVal("squish"),
cty.NumberIntVal(5).Mark("c"),
cty.StringVal("5").Mark("c"),
},
{ // disregard marks on key
cty.MapVal(map[string]cty.Value{
"boop": cty.StringVal("beep"),
"frob": cty.StringVal("honk"),
}),
cty.StringVal("boop").Mark("a"),
cty.StringVal("nope"),
cty.StringVal("beep"),
},
}

for _, test := range tests {
Expand Down

0 comments on commit ac6b489

Please sign in to comment.