Skip to content

Commit

Permalink
function/stdlib: Fix setproduct bug with unknowns
Browse files Browse the repository at this point in the history
If an argument to setproduct is a set with unknown values, its length is
unknown, as the unknown values may be duplicates of others in the set.
This should result in returning an unknown value propagating all marks.

Previously this crashed due to a misuse of `.Mark(marks)` instead of
`.WithMarks(marks)`. This commit also fixes an issue with the previous
logic, which would shortcut return and omit marks from any arguments
after the set with unknown length.

Also includes a couple of tests for setproduct that were in my local
working copy for some reason.
  • Loading branch information
alisdair authored and apparentlymart committed Jun 21, 2021
1 parent 4e5310f commit 7ef3c08
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
10 changes: 9 additions & 1 deletion cty/function/stdlib/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -948,12 +948,16 @@ var SetProductFunc = function.New(&function.Spec{
var retMarks cty.ValueMarks

total := 1
var hasUnknownLength bool
for _, arg := range args {
arg, marks := arg.Unmark()
retMarks = cty.NewValueMarks(retMarks, marks)

// Continue processing after we find an argument with unknown
// length to ensure that we cover all the marks
if !arg.Length().IsKnown() {
return cty.UnknownVal(retType).Mark(marks), nil
hasUnknownLength = true
continue
}

// Because of our type checking function, we are guaranteed that
Expand All @@ -962,6 +966,10 @@ var SetProductFunc = function.New(&function.Spec{
total *= arg.LengthInt()
}

if hasUnknownLength {
return cty.UnknownVal(retType).WithMarks(retMarks), nil
}

if total == 0 {
// If any of the arguments was an empty collection then our result
// is also an empty collection, which we'll short-circuit here.
Expand Down
30 changes: 30 additions & 0 deletions cty/function/stdlib/collection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2324,6 +2324,36 @@ func TestSetproduct(t *testing.T) {
}).Mark("b"),
``,
},
{
// Empty lists with marks should propagate the marks
[]cty.Value{
cty.ListValEmpty(cty.String).Mark("a"),
cty.ListValEmpty(cty.Bool).Mark("b"),
},
cty.ListValEmpty(cty.Tuple([]cty.Type{cty.String, cty.Bool})).WithMarks(cty.NewValueMarks("a", "b")),
``,
},
{
// Empty sets with marks should propagate the marks
[]cty.Value{
cty.SetValEmpty(cty.String).Mark("a"),
cty.SetValEmpty(cty.Bool).Mark("b"),
},
cty.SetValEmpty(cty.Tuple([]cty.Type{cty.String, cty.Bool})).WithMarks(cty.NewValueMarks("a", "b")),
``,
},
{
// Arguments which are sets with partially unknown values results
// in unknown length (since the unknown values may already be
// present in the set). This gives an unknown result preserving all
// marks
[]cty.Value{
cty.SetVal([]cty.Value{cty.StringVal("x"), cty.UnknownVal(cty.String)}).Mark("a"),
cty.SetVal([]cty.Value{cty.True, cty.False}).Mark("b"),
},
cty.UnknownVal(cty.Set(cty.Tuple([]cty.Type{cty.String, cty.Bool}))).WithMarks(cty.NewValueMarks("a", "b")),
``,
},
}

for _, test := range tests {
Expand Down

0 comments on commit 7ef3c08

Please sign in to comment.