From da16ad4791eda831a83c2f336863ba12fb8c264c Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 11 Nov 2024 16:53:39 -0500 Subject: [PATCH] function: include marks when returning early with an unknown value 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. --- cty/function/function.go | 33 +++++--- cty/function/function_test.go | 151 ++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+), 10 deletions(-) diff --git a/cty/function/function.go b/cty/function/function.go index 6fc96828..4d7c61d0 100644 --- a/cty/function/function.go +++ b/cty/function/function.go @@ -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 @@ -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 { @@ -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 { @@ -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, diff --git a/cty/function/function_test.go b/cty/function/function_test.go index 26bd5772..7de9e830 100644 --- a/cty/function/function_test.go +++ b/cty/function/function_test.go @@ -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) + } + }) +}