Skip to content

Commit

Permalink
function: include marks when returning early with an unknown value
Browse files Browse the repository at this point in the history 
If a function parameter does not AllowUnknown, the function Call method
returns early with the appropriate unknown value. However, it was missing
any possible marks that were known from the given arguments.

We'll delay the early return slightly, so that all argument marks can be
collected and applied to the unknown return value.
  • Loading branch information
@jbardin
jbardin authored Nov 11, 2024
1 parent ea922e7 commit da16ad4
Show file tree
Hide file tree
Showing 2 changed files with 174 additions and 10 deletions.
33 changes: 23 additions & 10 deletions cty/function/function.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,15 +251,25 @@ func (f Function) Call(args []cty.Value) (val cty.Value, err error) {
if err != nil {
return cty.NilVal, err
}

var resultMarks []cty.ValueMarks
// If we are returning an unknown early due to some unknown in the
// arguments, we first need to complete the iteration over all the args
// to ensure we collect as many marks as possible for resultMarks.
returnUnknown := false

if dynTypeArgs {
// returnTypeForValues sets this if any argument was inexactly typed
// and the corresponding parameter did not indicate it could deal with
// that. In that case we also avoid calling the implementation function
// because it will also typically not be ready to deal with that case.
return cty.UnknownVal(expectedType), nil
returnUnknown = true
}

if refineResult := f.spec.RefineResult; refineResult != nil {
// If returnUnknown is set already, it means we don't have a refinement
// because of dynTypeArgs, but we may still need to collect marks from the
// rest of the arguments.
if refineResult := f.spec.RefineResult; refineResult != nil && !returnUnknown {
// If this function has a refinement callback then we'll refine
// our result value in the same way regardless of how we return.
// It's the function author's responsibility to ensure that the
Expand All @@ -280,15 +290,10 @@ func (f Function) Call(args []cty.Value) (val cty.Value, err error) {
// values and marked values.
posArgs := args[:len(f.spec.Params)]
varArgs := args[len(f.spec.Params):]
var resultMarks []cty.ValueMarks

for i, spec := range f.spec.Params {
val := posArgs[i]

if !val.IsKnown() && !spec.AllowUnknown {
return cty.UnknownVal(expectedType), nil
}

if !spec.AllowMarked {
unwrappedVal, marks := val.UnmarkDeep()
if len(marks) > 0 {
Expand All @@ -305,14 +310,15 @@ func (f Function) Call(args []cty.Value) (val cty.Value, err error) {
args = newArgs
}
}

if !val.IsKnown() && !spec.AllowUnknown {
returnUnknown = true
}
}

if f.spec.VarParam != nil {
spec := f.spec.VarParam
for i, val := range varArgs {
if !val.IsKnown() && !spec.AllowUnknown {
return cty.UnknownVal(expectedType), nil
}
if !spec.AllowMarked {
unwrappedVal, marks := val.UnmarkDeep()
if len(marks) > 0 {
Expand All @@ -323,9 +329,16 @@ func (f Function) Call(args []cty.Value) (val cty.Value, err error) {
args = newArgs
}
}
if !val.IsKnown() && !spec.AllowUnknown {
returnUnknown = true
}
}
}

if returnUnknown {
return cty.UnknownVal(expectedType).WithMarks(resultMarks...), nil
}

var retVal cty.Value
{
// Intercept any panics from the function and return them as normal errors,
Expand Down
151 changes: 151 additions & 0 deletions cty/function/function_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,3 +493,154 @@ func stubType([]cty.Value) (cty.Type, error) {
func stubImpl([]cty.Value, cty.Type) (cty.Value, error) {
return cty.NilVal, fmt.Errorf("should not be called")
}

func TestFunctionCallWithUnknownVals(t *testing.T) {
t.Run("params", func(t *testing.T) {
f := New(&Spec{
Params: []Parameter{
{
Name: "foo",
Type: cty.String,
},
{
Name: "bar",
Type: cty.String,
},
},
Type: StaticReturnType(cty.String),
Impl: stubImpl,
})
marks := cty.NewValueMarks("special", "extra")
unknownWithMarks := cty.UnknownVal(cty.String).WithMarks(marks)
knownWithMarks := cty.StringVal("ok").WithMarks(marks)
got, err := f.Call([]cty.Value{unknownWithMarks, knownWithMarks})
if err != nil {
t.Error(err)
}
if !marks.Equal(got.Marks()) {
t.Errorf("unexpected marks\ngot: %s\nwant: %s", got.Marks(), marks)
}
})
t.Run("params-partial-marks", func(t *testing.T) {
f := New(&Spec{
Params: []Parameter{
{
Name: "foo",
Type: cty.String,
},
{
Name: "bar",
Type: cty.String,
// AllowMarked means we can't include this value's marks in
// the early return unknown value.
AllowMarked: true,
},
},
Type: StaticReturnType(cty.String),
Impl: stubImpl,
})
marks := cty.NewValueMarks("special")
unknownWithMarks := cty.UnknownVal(cty.String).WithMarks(marks)
knownWithMarks := cty.StringVal("ok").Mark("allow_marked")
got, err := f.Call([]cty.Value{unknownWithMarks, knownWithMarks})
if err != nil {
t.Error(err)
}
if !marks.Equal(got.Marks()) {
t.Errorf("unexpected marks\ngot: %s\nwant: %s", got.Marks(), marks)
}
})
t.Run("varparam", func(t *testing.T) {
f := New(&Spec{
VarParam: &Parameter{
Name: "foo",
Type: cty.String,
},
Type: StaticReturnType(cty.String),
Impl: stubImpl,
})
marks := cty.NewValueMarks("special", "extra")
unknownWithMarks := cty.UnknownVal(cty.String).WithMarks(marks)
got, err := f.Call([]cty.Value{unknownWithMarks})
if err != nil {
t.Error(err)
}
if !marks.Equal(got.Marks()) {
t.Errorf("unexpected marks\ngot: %s\nwant: %s", got.Marks(), marks)
}
})

t.Run("refined-marked", func(t *testing.T) {
f := New(&Spec{
Params: []Parameter{
{
Name: "first",
Type: cty.String,
},
{
Name: "second",
Type: cty.String,
AllowMarked: true,
AllowUnknown: true,
},
},
Type: StaticReturnType(cty.String),
RefineResult: func(b *cty.RefinementBuilder) *cty.RefinementBuilder {
return b.NotNull()
},
Impl: stubImpl,
})
got, err := f.Call([]cty.Value{
cty.UnknownVal(cty.String).Mark("first"),
cty.UnknownVal(cty.String).Mark("second"),
})
if err != nil {
t.Error(err)
}
// since the second parameter allows marked values, we should only
// expect the fist mark when given unknown arguments.
expected := cty.UnknownVal(cty.String).RefineNotNull().Mark("first")
if !got.RawEquals(expected) {
t.Errorf("expected %#v\ngot: %#v", expected, got)
}
})

t.Run("marked-dynamic-not-refined", func(t *testing.T) {
f := New(&Spec{
Params: []Parameter{
{
Name: "first",
Type: cty.String,
},
{
Name: "second",
Type: cty.String,
AllowMarked: true,
AllowUnknown: true,
},
},
Type: func([]cty.Value) (cty.Type, error) {
// this isn't called with known args, so a static dynamic type is OK
return cty.DynamicPseudoType, nil
},
RefineResult: func(b *cty.RefinementBuilder) *cty.RefinementBuilder {
return b.NotNull()
},
Impl: stubImpl,
})
got, err := f.Call([]cty.Value{
cty.DynamicVal.Mark("first"),
cty.DynamicVal.Mark("second"),
})
if err != nil {
t.Error(err)
}
// Since the second parameter allows marked values, we should only
// expect the fist mark when given unknown arguments.
// Because the type is unknown, the result should not be refined.
expected := cty.DynamicVal.Mark("first")
if !got.RawEquals(expected) {
t.Errorf("expected %#v\ngot: %#v", expected, got)
}
})
}

0 comments on commit da16ad4

Please sign in to comment.