From d3cc973f05054fd9378f340348562245fb5f7d94 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 15 Feb 2024 20:43:32 -0500 Subject: [PATCH 01/15] switch to tuples and add reflection support --- function/arguments_data.go | 12 +- function/arguments_data_test.go | 127 +++++++++++++++- function/definition.go | 2 +- internal/fromproto5/arguments_data.go | 16 +- internal/fromproto5/arguments_data_test.go | 57 +++++--- internal/fromproto6/arguments_data.go | 15 +- internal/fromproto6/arguments_data_test.go | 57 +++++--- internal/fwserver/server_callfunction_test.go | 32 ++-- .../proto5server/server_callfunction_test.go | 9 +- .../proto6server/server_callfunction_test.go | 9 +- internal/reflect/into_test.go | 61 +++++++- internal/reflect/outof_test.go | 49 ++++++- internal/reflect/slice.go | 138 ++++++++++++++++-- 13 files changed, 496 insertions(+), 88 deletions(-) diff --git a/function/arguments_data.go b/function/arguments_data.go index 9b75eaf05..7c6e36321 100644 --- a/function/arguments_data.go +++ b/function/arguments_data.go @@ -45,9 +45,9 @@ func (d ArgumentsData) Equal(o ArgumentsData) bool { // Each target type must be acceptable for the data type in the parameter // definition. // -// Variadic parameter argument data must be consumed by a types.List or Go slice +// Variadic parameter argument data must be consumed by a types.Tuple or Go slice // type with an element type appropriate for the parameter definition ([]T). The -// framework automatically populates this list with elements matching the zero, +// framework automatically populates this tuple with elements matching the zero, // one, or more arguments passed. func (d ArgumentsData) Get(ctx context.Context, targets ...any) diag.Diagnostics { var diags diag.Diagnostics @@ -112,10 +112,10 @@ func (d ArgumentsData) Get(ctx context.Context, targets ...any) diag.Diagnostics // position and populates the target with the value. The target type must be // acceptable for the data type in the parameter definition. // -// Variadic parameter argument data must be consumed by a types.List or Go slice +// Variadic parameter argument data must be consumed by a types.Tuple or Go slice // type with an element type appropriate for the parameter definition ([]T) at // the position after all parameters. The framework automatically populates this -// list with elements matching the zero, one, or more arguments passed. +// tuple with elements matching the zero, one, or more arguments passed. func (d ArgumentsData) GetArgument(ctx context.Context, position int, target any) diag.Diagnostics { var diags diag.Diagnostics @@ -134,8 +134,8 @@ func (d ArgumentsData) GetArgument(ctx context.Context, position int, target any diags.AddError( "Invalid Argument Data Position", "When attempting to fetch argument data during the function call, the provider code attempted to read a non-existent argument position. "+ - "Function argument positions are 0-based and any final variadic parameter is represented as one argument position with an ordered list of the parameter data type. "+ - "This is always an error in the provider code and should be reported to the provider developers.\n\n"+ + "Function argument positions are 0-based and any final variadic parameter is represented as one argument position with a tuple where each element "+ + "type matches the parameter data type. This is always an error in the provider code and should be reported to the provider developers.\n\n"+ fmt.Sprintf("Given argument position: %d, last argument position: %d", position, len(d.values)-1), ) diff --git a/function/arguments_data_test.go b/function/arguments_data_test.go index bb9718118..01ab7b29b 100644 --- a/function/arguments_data_test.go +++ b/function/arguments_data_test.go @@ -173,6 +173,46 @@ func TestArgumentsDataGet(t *testing.T) { pointer(attr.Value(basetypes.NewStringValue("test"))), }, }, + "attr-value-variadic": { + argumentsData: function.NewArgumentsData([]attr.Value{ + basetypes.NewBoolNull(), + basetypes.NewInt64Unknown(), + basetypes.NewStringValue("test"), + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, + []attr.Value{ + basetypes.NewStringValue("test1"), + basetypes.NewStringValue("test2"), + }, + ), + }), + targets: []any{ + new(attr.Value), + new(attr.Value), + new(attr.Value), + new(attr.Value), + }, + expected: []any{ + pointer(attr.Value(basetypes.NewBoolNull())), + pointer(attr.Value(basetypes.NewInt64Unknown())), + pointer(attr.Value(basetypes.NewStringValue("test"))), + pointer(attr.Value( + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, + []attr.Value{ + basetypes.NewStringValue("test1"), + basetypes.NewStringValue("test2"), + }, + ), + )), + }, + }, "framework-type": { argumentsData: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolNull(), @@ -190,6 +230,46 @@ func TestArgumentsDataGet(t *testing.T) { pointer(basetypes.NewStringValue("test")), }, }, + "framework-type-variadic": { + argumentsData: function.NewArgumentsData([]attr.Value{ + basetypes.NewBoolNull(), + basetypes.NewInt64Unknown(), + basetypes.NewStringValue("test"), + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, + []attr.Value{ + basetypes.NewStringValue("test1"), + basetypes.NewStringValue("test2"), + }, + ), + }), + targets: []any{ + new(basetypes.BoolValue), + new(basetypes.Int64Value), + new(basetypes.StringValue), + new(basetypes.TupleValue), + }, + expected: []any{ + pointer(basetypes.NewBoolNull()), + pointer(basetypes.NewInt64Unknown()), + pointer(basetypes.NewStringValue("test")), + pointer( + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, + []attr.Value{ + basetypes.NewStringValue("test1"), + basetypes.NewStringValue("test2"), + }, + ), + ), + }, + }, "reflection": { argumentsData: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolNull(), @@ -204,6 +284,46 @@ func TestArgumentsDataGet(t *testing.T) { pointer("test"), }, }, + "reflection-variadic": { + argumentsData: function.NewArgumentsData([]attr.Value{ + basetypes.NewBoolNull(), + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, + []attr.Value{ + basetypes.NewStringValue("test1"), + basetypes.NewStringValue("test2"), + }, + ), + }), + targets: []any{ + new(*bool), + new([]string), + }, + expected: []any{ + pointer((*bool)(nil)), + pointer([]string{ + "test1", + "test2", + }), + }, + }, + "reflection-variadic-empty": { + argumentsData: function.NewArgumentsData([]attr.Value{ + basetypes.NewBoolNull(), + basetypes.NewTupleValueMust([]attr.Type{}, []attr.Value{}), + }), + targets: []any{ + new(*bool), + new([]string), + }, + expected: []any{ + pointer((*bool)(nil)), + pointer([]string{}), + }, + }, } for name, testCase := range testCases { @@ -225,6 +345,9 @@ func TestArgumentsDataGet(t *testing.T) { cmp.Transformer("StringValue", func(v *basetypes.StringValue) basetypes.StringValue { return *v }), + cmp.Transformer("TupleValue", func(v *basetypes.TupleValue) basetypes.TupleValue { + return *v + }), } if diff := cmp.Diff(testCase.targets, testCase.expected, options...); diff != "" { @@ -273,8 +396,8 @@ func TestArgumentsDataGetArgument(t *testing.T) { diag.NewErrorDiagnostic( "Invalid Argument Data Position", "When attempting to fetch argument data during the function call, the provider code attempted to read a non-existent argument position. "+ - "Function argument positions are 0-based and any final variadic parameter is represented as one argument position with an ordered list of the parameter data type. "+ - "This is always an error in the provider code and should be reported to the provider developers.\n\n"+ + "Function argument positions are 0-based and any final variadic parameter is represented as one argument position with a tuple where each element "+ + "type matches the parameter data type. This is always an error in the provider code and should be reported to the provider developers.\n\n"+ "Given argument position: 1, last argument position: 0", ), }, diff --git a/function/definition.go b/function/definition.go index af14b2bfa..b95ee29bc 100644 --- a/function/definition.go +++ b/function/definition.go @@ -21,7 +21,7 @@ type Definition struct { // VariadicParameter is an optional final parameter which can accept zero or // more arguments when the function is called. The argument data is sent as - // an ordered list of the associated data type. + // a tuple, where all elements are of the same associated data type. VariadicParameter Parameter // Return is the function call response data type. diff --git a/internal/fromproto5/arguments_data.go b/internal/fromproto5/arguments_data.go index 57c43c6b0..180e4739d 100644 --- a/internal/fromproto5/arguments_data.go +++ b/internal/fromproto5/arguments_data.go @@ -51,7 +51,7 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def return function.NewArgumentsData(nil), nil } - // Variadic values are collected as a separate list to ease developer usage. + // Variadic values are collected as a separate tuple to ease developer usage. argumentValues := make([]attr.Value, 0, len(definition.Parameters)) variadicValues := make([]attr.Value, 0, len(arguments)-len(definition.Parameters)) var diags diag.Diagnostics @@ -133,7 +133,19 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def } if definition.VariadicParameter != nil { - variadicValue, variadicValueDiags := basetypes.NewListValue(definition.VariadicParameter.GetType(), variadicValues) + // Variadic parameters are represented as a TupleType that contains all of the same element type. + // + // This is intentional and meant to match how the variadic parameter is handled by Terraform core, + // where the variadic parameter type constraint is applied to each argument individually. + variadicType := definition.VariadicParameter.GetType() + tupleTypes := make([]attr.Type, len(variadicValues)) + tupleValues := make([]attr.Value, len(variadicValues)) + for i, val := range variadicValues { + tupleTypes[i] = variadicType + tupleValues[i] = val + } + + variadicValue, variadicValueDiags := basetypes.NewTupleValue(tupleTypes, tupleValues) diags.Append(variadicValueDiags...) diff --git a/internal/fromproto5/arguments_data_test.go b/internal/fromproto5/arguments_data_test.go index 73b45b7db..5dba9b16f 100644 --- a/internal/fromproto5/arguments_data_test.go +++ b/internal/fromproto5/arguments_data_test.go @@ -183,8 +183,8 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -202,8 +202,10 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), }, @@ -224,8 +226,11 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), basetypes.NewStringValue("varg-arg2"), @@ -264,8 +269,8 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -286,8 +291,10 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg2"), }, @@ -311,8 +318,11 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg2"), basetypes.NewStringValue("varg-arg3"), @@ -326,8 +336,8 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -340,8 +350,10 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg0"), }, @@ -358,8 +370,10 @@ func TestArgumentsData(t *testing.T) { }, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - testtypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + testtypes.StringType{}, + }, []attr.Value{ testtypes.String{ CreatedBy: testtypes.StringType{}, @@ -378,8 +392,11 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg0"), basetypes.NewStringValue("varg-arg1"), diff --git a/internal/fromproto6/arguments_data.go b/internal/fromproto6/arguments_data.go index a41b1ce7d..c19dc9607 100644 --- a/internal/fromproto6/arguments_data.go +++ b/internal/fromproto6/arguments_data.go @@ -51,7 +51,7 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov6.DynamicValue, def return function.NewArgumentsData(nil), nil } - // Variadic values are collected as a separate list to ease developer usage. + // Variadic values are collected as a separate tuple to ease developer usage. argumentValues := make([]attr.Value, 0, len(definition.Parameters)) variadicValues := make([]attr.Value, 0, len(arguments)-len(definition.Parameters)) var diags diag.Diagnostics @@ -133,7 +133,18 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov6.DynamicValue, def } if definition.VariadicParameter != nil { - variadicValue, variadicValueDiags := basetypes.NewListValue(definition.VariadicParameter.GetType(), variadicValues) + // Variadic parameters are represented as a TupleType that contains all of the same element type. + // + // This is intentional and meant to match how the variadic parameter is handled by Terraform core, + // where the variadic parameter type constraint is applied to each argument individually. + variadicType := definition.VariadicParameter.GetType() + tupleTypes := make([]attr.Type, len(variadicValues)) + tupleValues := make([]attr.Value, len(variadicValues)) + for i, val := range variadicValues { + tupleTypes[i] = variadicType + tupleValues[i] = val + } + variadicValue, variadicValueDiags := basetypes.NewTupleValue(tupleTypes, tupleValues) diags.Append(variadicValueDiags...) diff --git a/internal/fromproto6/arguments_data_test.go b/internal/fromproto6/arguments_data_test.go index c88f97b49..a6ff6ed04 100644 --- a/internal/fromproto6/arguments_data_test.go +++ b/internal/fromproto6/arguments_data_test.go @@ -183,8 +183,8 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -202,8 +202,10 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), }, @@ -224,8 +226,11 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), basetypes.NewStringValue("varg-arg2"), @@ -264,8 +269,8 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -286,8 +291,10 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg2"), }, @@ -311,8 +318,11 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg2"), basetypes.NewStringValue("varg-arg3"), @@ -326,8 +336,8 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -340,8 +350,10 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg0"), }, @@ -358,8 +370,10 @@ func TestArgumentsData(t *testing.T) { }, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - testtypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + testtypes.StringType{}, + }, []attr.Value{ testtypes.String{ CreatedBy: testtypes.StringType{}, @@ -378,8 +392,11 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg0"), basetypes.NewStringValue("varg-arg1"), diff --git a/internal/fwserver/server_callfunction_test.go b/internal/fwserver/server_callfunction_test.go index c56d371f1..5564f74cb 100644 --- a/internal/fwserver/server_callfunction_test.go +++ b/internal/fwserver/server_callfunction_test.go @@ -138,8 +138,11 @@ func TestServerCallFunction(t *testing.T) { request: &fwserver.CallFunctionRequest{ Arguments: function.NewArgumentsData([]attr.Value{ basetypes.NewStringValue("arg0"), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("arg1-element0"), basetypes.NewStringValue("arg1-element1"), @@ -149,13 +152,16 @@ func TestServerCallFunction(t *testing.T) { Function: &testprovider.Function{ RunMethod: func(ctx context.Context, req function.RunRequest, resp *function.RunResponse) { var arg0 basetypes.StringValue - var arg1 basetypes.ListValue + var arg1 basetypes.TupleValue resp.Diagnostics.Append(req.Arguments.Get(ctx, &arg0, &arg1)...) expectedArg0 := basetypes.NewStringValue("arg0") - expectedArg1 := basetypes.NewListValueMust( - basetypes.StringType{}, + expectedArg1 := basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("arg1-element0"), basetypes.NewStringValue("arg1-element1"), @@ -297,8 +303,11 @@ func TestServerCallFunction(t *testing.T) { request: &fwserver.CallFunctionRequest{ Arguments: function.NewArgumentsData([]attr.Value{ basetypes.NewStringValue("arg0"), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("arg1-element0"), basetypes.NewStringValue("arg1-element1"), @@ -308,14 +317,17 @@ func TestServerCallFunction(t *testing.T) { Function: &testprovider.Function{ RunMethod: func(ctx context.Context, req function.RunRequest, resp *function.RunResponse) { var arg0 basetypes.StringValue - var arg1 basetypes.ListValue + var arg1 basetypes.TupleValue resp.Diagnostics.Append(req.Arguments.GetArgument(ctx, 0, &arg0)...) resp.Diagnostics.Append(req.Arguments.GetArgument(ctx, 1, &arg1)...) expectedArg0 := basetypes.NewStringValue("arg0") - expectedArg1 := basetypes.NewListValueMust( - basetypes.StringType{}, + expectedArg1 := basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("arg1-element0"), basetypes.NewStringValue("arg1-element1"), diff --git a/internal/proto5server/server_callfunction_test.go b/internal/proto5server/server_callfunction_test.go index 273008754..394dc4ea9 100644 --- a/internal/proto5server/server_callfunction_test.go +++ b/internal/proto5server/server_callfunction_test.go @@ -123,13 +123,16 @@ func TestServerCallFunction(t *testing.T) { }, RunMethod: func(ctx context.Context, req function.RunRequest, resp *function.RunResponse) { var arg0 basetypes.StringValue - var arg1 basetypes.ListValue + var arg1 basetypes.TupleValue resp.Diagnostics.Append(req.Arguments.Get(ctx, &arg0, &arg1)...) expectedArg0 := basetypes.NewStringValue("arg0") - expectedArg1 := basetypes.NewListValueMust( - basetypes.StringType{}, + expectedArg1 := basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), basetypes.NewStringValue("varg-arg2"), diff --git a/internal/proto6server/server_callfunction_test.go b/internal/proto6server/server_callfunction_test.go index aae148006..8d21e1888 100644 --- a/internal/proto6server/server_callfunction_test.go +++ b/internal/proto6server/server_callfunction_test.go @@ -123,13 +123,16 @@ func TestServerCallFunction(t *testing.T) { }, RunMethod: func(ctx context.Context, req function.RunRequest, resp *function.RunResponse) { var arg0 basetypes.StringValue - var arg1 basetypes.ListValue + var arg1 basetypes.TupleValue resp.Diagnostics.Append(req.Arguments.Get(ctx, &arg0, &arg1)...) expectedArg0 := basetypes.NewStringValue("arg0") - expectedArg1 := basetypes.NewListValueMust( - basetypes.StringType{}, + expectedArg1 := basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), basetypes.NewStringValue("varg-arg2"), diff --git a/internal/reflect/into_test.go b/internal/reflect/into_test.go index 023a28705..a0d776347 100644 --- a/internal/reflect/into_test.go +++ b/internal/reflect/into_test.go @@ -50,7 +50,7 @@ func TestInto_Slices(t *testing.T) { target: make([]string, 0), expected: []string{"hello", "world"}, }, - "tuple-to-go-slice-unsupported": { + "tuple-to-go-slice": { typ: types.TupleType{ElemTypes: []attr.Type{types.StringType, types.StringType}}, value: tftypes.NewValue(tftypes.Tuple{ ElementTypes: []tftypes.Type{tftypes.String, tftypes.String}, @@ -59,6 +59,35 @@ func TestInto_Slices(t *testing.T) { tftypes.NewValue(tftypes.String, "world"), }), target: make([]string, 0), + expected: []string{"hello", "world"}, + }, + "tuple-to-go-slice-no-element-types-no-values": { + typ: types.TupleType{ElemTypes: []attr.Type{}}, + value: tftypes.NewValue(tftypes.Tuple{ElementTypes: []tftypes.Type{}}, []tftypes.Value{}), + target: make([]string, 0), + expected: []string{}, + }, + "tuple-to-go-slice-one-element": { + typ: types.TupleType{ElemTypes: []attr.Type{types.StringType}}, + value: tftypes.NewValue(tftypes.Tuple{ + ElementTypes: []tftypes.Type{tftypes.String}, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "hello"), + }), + target: make([]string, 0), + expected: []string{"hello"}, + }, + "tuple-to-go-slice-unsupported-no-element-types-with-values": { + typ: types.TupleType{ElemTypes: []attr.Type{}}, + value: tftypes.NewValue(tftypes.Tuple{ + ElementTypes: []tftypes.Type{tftypes.String, tftypes.String}, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "hello"), + tftypes.NewValue(tftypes.String, "world"), + }), + // Target isn't relevant for this test, as the wrapping reflection logic doesn't attempt to determine the underlying type of an `any` + // The test will successfully reject the reflection attempt based on the element types of the TupleType. + target: make([]string, 0), expected: make([]string, 0), expectedDiags: diag.Diagnostics{ diag.WithPath( @@ -71,7 +100,35 @@ func TestInto_Slices(t *testing.T) { tftypes.NewValue(tftypes.String, "world"), }), TargetType: reflect.TypeOf([]string{}), - Err: errors.New("cannot reflect tftypes.Tuple[tftypes.String, tftypes.String] using type information provided by basetypes.TupleType, reflection support is currently not implemented for tuples"), + Err: errors.New("cannot reflect tftypes.Tuple[tftypes.String, tftypes.String] using type information provided by basetypes.TupleType, tuple type contained no element types but received values"), + }, + ), + }, + }, + "tuple-to-go-slice-unsupported-multiple-element-types": { + typ: types.TupleType{ElemTypes: []attr.Type{types.StringType, types.BoolType}}, + value: tftypes.NewValue(tftypes.Tuple{ + ElementTypes: []tftypes.Type{tftypes.String, tftypes.Bool}, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "hello"), + tftypes.NewValue(tftypes.Bool, true), + }), + // Target isn't relevant for this test, as the wrapping reflection logic doesn't attempt to determine the underlying type of an `any` + // The test will successfully reject the reflection attempt based on the element types of the TupleType. + target: make([]string, 0), + expected: make([]string, 0), + expectedDiags: diag.Diagnostics{ + diag.WithPath( + path.Empty(), + refl.DiagIntoIncompatibleType{ + Val: tftypes.NewValue(tftypes.Tuple{ + ElementTypes: []tftypes.Type{tftypes.String, tftypes.Bool}, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "hello"), + tftypes.NewValue(tftypes.Bool, true), + }), + TargetType: reflect.TypeOf([]string{}), + Err: errors.New("cannot reflect tftypes.Tuple[tftypes.String, tftypes.Bool] using type information provided by basetypes.TupleType, reflection support for tuples is limited to multiple elements of the same element type. Expected all element types to be basetypes.StringType"), }, ), }, diff --git a/internal/reflect/outof_test.go b/internal/reflect/outof_test.go index 33f451218..6661c24c7 100644 --- a/internal/reflect/outof_test.go +++ b/internal/reflect/outof_test.go @@ -61,15 +61,58 @@ func TestFromValue_go_types(t *testing.T) { }, ), }, - "go-slice-to-tuple-value-unsupported": { + "go-slice-to-tuple-value": { typ: types.TupleType{ElemTypes: []attr.Type{types.StringType, types.StringType}}, - value: []any{"hello", "world"}, + value: []string{"hello", "world"}, + expected: types.TupleValueMust( + []attr.Type{ + types.StringType, + types.StringType, + }, + []attr.Value{ + types.StringValue("hello"), + types.StringValue("world"), + }, + ), + }, + "go-slice-to-tuple-value-empty": { + typ: types.TupleType{ElemTypes: []attr.Type{}}, + value: []any{}, + expected: types.TupleValueMust([]attr.Type{}, []attr.Value{}), + }, + "go-slice-to-tuple-value-one-element": { + typ: types.TupleType{ElemTypes: []attr.Type{types.BoolType}}, + value: []bool{true}, + expected: types.TupleValueMust( + []attr.Type{ + types.BoolType, + }, + []attr.Value{ + types.BoolValue(true), + }, + ), + }, + "go-slice-to-tuple-value-unsupported-no-element-types-with-values": { + typ: types.TupleType{ElemTypes: []attr.Type{}}, + value: []string{"hello", "world"}, + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Empty(), + "Value Conversion Error", + "An unexpected error was encountered trying to convert from slice value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "cannot use type []string as schema type basetypes.TupleType; tuple type contained no element types but received values", + ), + }, + }, + "go-slice-to-tuple-value-unsupported-multiple-element-types": { + typ: types.TupleType{ElemTypes: []attr.Type{types.StringType, types.BoolType}}, + value: []any{"hello", true}, expectedDiags: diag.Diagnostics{ diag.NewAttributeErrorDiagnostic( path.Empty(), "Value Conversion Error", "An unexpected error was encountered trying to convert from slice value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ - "cannot use type []interface {} as schema type basetypes.TupleType; reflection support is currently not implemented for tuples", + "cannot use type []interface {} as schema type basetypes.TupleType; reflection support for tuples is limited to multiple elements of the same element type. Expected all element types to be basetypes.StringType", ), }, }, diff --git a/internal/reflect/slice.go b/internal/reflect/slice.go index a160a4a6f..2e2b74ce9 100644 --- a/internal/reflect/slice.go +++ b/internal/reflect/slice.go @@ -90,14 +90,70 @@ func reflectSlice(ctx context.Context, typ attr.Type, val tftypes.Value, target } return slice, diags - // Tuple reflection from slices is currently not supported as usage is limited case attr.TypeWithElementTypes: - diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ - Val: val, - TargetType: target.Type(), - Err: fmt.Errorf("cannot reflect %s using type information provided by %T, reflection support is currently not implemented for tuples", val.Type(), t), - })) - return target, diags + // we need to know the type the slice is wrapping + elemType := target.Type().Elem() + + // we want an empty version of the slice + slice := reflect.MakeSlice(target.Type(), 0, len(values)) + + if len(t.ElementTypes()) <= 0 { + // If the tuple values are empty as well, we can just pass back an empty slice of the type we received. + if len(values) == 0 { + return slice, diags + } + + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ + Val: val, + TargetType: target.Type(), + Err: fmt.Errorf("cannot reflect %s using type information provided by %T, tuple type contained no element types but received values", val.Type(), t), + })) + return target, diags + } + + // Ensure that all tuple element types are the same by comparing each element type to the first + multipleTypes := false + allElemTypes := t.ElementTypes() + elemAttrType := allElemTypes[0] + for _, elemType := range allElemTypes[1:] { + if !elemAttrType.Equal(elemType) { + multipleTypes = true + break + } + } + + if multipleTypes { + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ + Val: val, + TargetType: target.Type(), + Err: fmt.Errorf("cannot reflect %s using type information provided by %T, reflection support for tuples is limited to multiple elements of the same element type. Expected all element types to be %T", val.Type(), t, elemAttrType), + })) + return target, diags + } + + // go over each of the values passed in, create a Go value of the right + // type for them, and add it to our new slice + for pos, value := range values { + // create a new Go value of the type that can go in the slice + targetValue := reflect.Zero(elemType) + + // update our path so we can have nice errors + // utilizing `AtListIndex` because tuple paths are also based on the underlying `PathStepElementKeyInt` + valPath := path.AtListIndex(pos) + + // reflect the value into our new target + val, valDiags := BuildValue(ctx, elemAttrType, value, targetValue, opts, valPath) + diags.Append(valDiags...) + + if diags.HasError() { + return target, diags + } + + // add the new target to our slice + slice = reflect.Append(slice, val) + } + + return slice, diags default: diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, @@ -186,13 +242,67 @@ func FromSlice(ctx context.Context, typ attr.Type, val reflect.Value, path path. } // Tuple reflection from slices is currently not supported as usage is limited case attr.TypeWithElementTypes: - err := fmt.Errorf("cannot use type %s as schema type %T; reflection support is currently not implemented for tuples", val.Type(), t) - diags.AddAttributeError( - path, - "Value Conversion Error", - "An unexpected error was encountered trying to convert from slice value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), - ) - return nil, diags + if len(t.ElementTypes()) <= 0 { + // If the tuple values are empty as well, we can just pass back an empty slice of the type we received. + if val.Len() == 0 { + break + } + + err := fmt.Errorf("cannot use type %s as schema type %T; tuple type contained no element types but received values", val.Type(), t) + diags.AddAttributeError( + path, + "Value Conversion Error", + "An unexpected error was encountered trying to convert from slice value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), + ) + return nil, diags + } + + // Ensure that all tuple element types are the same by comparing each element type to the first + multipleTypes := false + allElemTypes := t.ElementTypes() + elemAttrType := allElemTypes[0] + for _, elemType := range allElemTypes[1:] { + if !elemAttrType.Equal(elemType) { + multipleTypes = true + break + } + } + + if multipleTypes { + err := fmt.Errorf("cannot use type %s as schema type %T; reflection support for tuples is limited to multiple elements of the same element type. Expected all element types to be %T", val.Type(), t, elemAttrType) + diags.AddAttributeError( + path, + "Value Conversion Error", + "An unexpected error was encountered trying to convert from slice value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), + ) + return nil, diags + } + + for i := 0; i < val.Len(); i++ { + // utilizing `AtListIndex` because tuple paths are also based on the underlying `PathStepElementKeyInt` + valPath := path.AtListIndex(i) + + val, valDiags := FromValue(ctx, elemAttrType, val.Index(i).Interface(), valPath) + diags.Append(valDiags...) + + if diags.HasError() { + return nil, diags + } + + tfVal, err := val.ToTerraformValue(ctx) + if err != nil { + return nil, append(diags, toTerraformValueErrorDiag(err, path)) + } + + if typeWithValidate, ok := elemAttrType.(xattr.TypeWithValidate); ok { + diags.Append(typeWithValidate.Validate(ctx, tfVal, valPath)...) + if diags.HasError() { + return nil, diags + } + } + + tfElems = append(tfElems, tfVal) + } default: err := fmt.Errorf("cannot use type %s as schema type %T; %T must be an attr.TypeWithElementType or attr.TypeWithElementTypes", val.Type(), t, t) diags.AddAttributeError( From ce835423211753789c33a99472263e3a0fa9099c Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 15 Feb 2024 20:53:12 -0500 Subject: [PATCH 02/15] update docs --- .../docs/plugin/framework/functions/implementation.mdx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/website/docs/plugin/framework/functions/implementation.mdx b/website/docs/plugin/framework/functions/implementation.mdx index 9fb430a75..9d5e90163 100644 --- a/website/docs/plugin/framework/functions/implementation.mdx +++ b/website/docs/plugin/framework/functions/implementation.mdx @@ -114,7 +114,7 @@ The `Return` field must be defined as all functions must return a result. This i There may be zero or more parameters, which are defined with the `Parameters` field. They are ordered, which influences how practitioners call the function in their configurations and how the [Run method](#run-method) must read the argument data. Refer to the [parameters](/terraform/plugin/framework/functions/parameters) documentation for details about all available types and how to handle data with each type. -An optional `VariadicParameter` field enables a final variadic parameter which accepts zero, one, or more values of the same type. It may be optionally combined with `Parameters`, meaning it represents the any argument data after the final parameter. When reading argument data, a `VariadicParameter` is represented as an ordered list of the parameter type, where the list has zero or more elements to match the given arguments. +An optional `VariadicParameter` field enables a final variadic parameter which accepts zero, one, or more values of the same type. It may be optionally combined with `Parameters`, meaning it represents the any argument data after the final parameter. When reading argument data, a `VariadicParameter` is represented as a tuple, with each element matching the parameter type; the tuple has zero or more elements to match the given arguments. By default, Terraform will not pass null or unknown values to the provider logic when a function is called. Within each parameter, use the `AllowNullValue` and/or `AllowUnknownValues` fields to explicitly allow those kinds of values. Enabling `AllowNullValue` requires using a pointer type or [framework type](/terraform/plugin/framework/handling-data/types) when reading argument data. Enabling `AllowUnknownValues` requires using a [framework type](/terraform/plugin/framework/handling-data/types) when reading argument data. @@ -194,11 +194,11 @@ func (f *ExampleFunction) Run(ctx context.Context, req function.RunRequest, resp #### Reading Variadic Parameter Argument Data -The optional `VariadicParameter` field in a function definition enables a final variadic parameter which accepts zero, one, or more values of the same type. It may be optionally combined with `Parameters`, meaning it represents the argument data after the final parameter. When reading argument data, a `VariadicParameter` is represented as an ordered list of the parameter type, where the list has zero or more elements to match the given arguments. +The optional `VariadicParameter` field in a function definition enables a final variadic parameter which accepts zero, one, or more values of the same type. It may be optionally combined with `Parameters`, meaning it represents the argument data after the final parameter. When reading argument data, a `VariadicParameter` is represented as a tuple, with each element matching the parameter type; the tuple has zero or more elements to match the given arguments. -Use either the [framework list type](/terraform/plugin/framework/handling-data/types/list) or a Go slice of an appropriate type to match the variadic parameter `[]T`. +Use either the [framework tuple type](/terraform/plugin/framework/handling-data/types/tuple) or a Go slice of an appropriate type to match the variadic parameter `[]T`. -In this example, there is a boolean parameter and string variadic parameter, where the variadic parameter argument data is always fetched as a list: +In this example, there is a boolean parameter and string variadic parameter, where the variadic parameter argument data is always fetched as a slice of `string`: ```go func (f *ExampleFunction) Definition(ctx context.Context, req function.DefinitionRequest, resp *function.DefinitionResponse) { @@ -221,7 +221,7 @@ func (f *ExampleFunction) Run(ctx context.Context, req function.RunRequest, resp } ``` -If necessary to return diagnostics for a specific variadic argument, note that Terraform treats each zero-based argument position individually unlike how the framework exposes the argument data. Add the number of non-variadic parameters (if any) to the variadic argument list element index to ensure the diagnostic is aligned to the correct argument in the configuration. +If necessary to return diagnostics for a specific variadic argument, note that Terraform treats each zero-based argument position individually unlike how the framework exposes the argument data. Add the number of non-variadic parameters (if any) to the variadic argument tuple element index to ensure the diagnostic is aligned to the correct argument in the configuration. In this example with two parameters and one variadic parameter, warning diagnostics are returned for all variadic arguments: From 7790febd29807115b674239b0109ddc300a2496c Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 15 Feb 2024 21:02:15 -0500 Subject: [PATCH 03/15] add names to variadic param names --- .../docs/plugin/framework/functions/implementation.mdx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/website/docs/plugin/framework/functions/implementation.mdx b/website/docs/plugin/framework/functions/implementation.mdx index 9d5e90163..9da2c893f 100644 --- a/website/docs/plugin/framework/functions/implementation.mdx +++ b/website/docs/plugin/framework/functions/implementation.mdx @@ -207,7 +207,9 @@ func (f *ExampleFunction) Definition(ctx context.Context, req function.Definitio Parameters: []function.Parameter{ function.BoolParameter{}, }, - VariadicParameter: function.StringParameter{}, + VariadicParameter: function.StringParameter{ + Name: "variadic_param", + }, } } @@ -233,7 +235,9 @@ func (f *ExampleFunction) Definition(ctx context.Context, req function.Definitio function.BoolParameter{}, function.Int64Parameter{}, }, - VariadicParameter: function.StringParameter{}, + VariadicParameter: function.StringParameter{ + Name: "variadic_param", + }, } } From c8649a5122f42c67784000826c4e1dfec19d6a78 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 16 Feb 2024 08:06:19 -0500 Subject: [PATCH 04/15] update doc note --- website/docs/plugin/framework/handling-data/types/tuple.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/plugin/framework/handling-data/types/tuple.mdx b/website/docs/plugin/framework/handling-data/types/tuple.mdx index 88571fd63..85a59dfcf 100644 --- a/website/docs/plugin/framework/handling-data/types/tuple.mdx +++ b/website/docs/plugin/framework/handling-data/types/tuple.mdx @@ -5,7 +5,7 @@ description: >- --- -The tuple type intentionally includes less functionality than other types in the type system as it has limited real world application and therefore is not exposed to provider developers except when working with dynamic values. +The tuple type doesn't have associated schema attributes as it has limited real world application. Provider developers will only encounter tuples when handling provider-defined function variadic parameters or dynamic values. # Tuple Type From c99abc51dd8dd5ca71e1bd4c5a97dbf307d8b130 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 16 Feb 2024 08:08:49 -0500 Subject: [PATCH 05/15] update docs --- internal/fromproto5/arguments_data.go | 4 ++-- internal/fromproto6/arguments_data.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/fromproto5/arguments_data.go b/internal/fromproto5/arguments_data.go index 180e4739d..bcbb17af9 100644 --- a/internal/fromproto5/arguments_data.go +++ b/internal/fromproto5/arguments_data.go @@ -133,9 +133,9 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def } if definition.VariadicParameter != nil { - // Variadic parameters are represented as a TupleType that contains all of the same element type. + // Variadic parameters are represented as a TupleType where each element contains the same element type. // - // This is intentional and meant to match how the variadic parameter is handled by Terraform core, + // This is intentional and meant to match how variadic arguments are handled by Terraform core, // where the variadic parameter type constraint is applied to each argument individually. variadicType := definition.VariadicParameter.GetType() tupleTypes := make([]attr.Type, len(variadicValues)) diff --git a/internal/fromproto6/arguments_data.go b/internal/fromproto6/arguments_data.go index c19dc9607..affb30bcb 100644 --- a/internal/fromproto6/arguments_data.go +++ b/internal/fromproto6/arguments_data.go @@ -133,9 +133,9 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov6.DynamicValue, def } if definition.VariadicParameter != nil { - // Variadic parameters are represented as a TupleType that contains all of the same element type. + // Variadic parameters are represented as a TupleType where each element contains the same element type. // - // This is intentional and meant to match how the variadic parameter is handled by Terraform core, + // This is intentional and meant to match how variadic arguments are handled by Terraform core, // where the variadic parameter type constraint is applied to each argument individually. variadicType := definition.VariadicParameter.GetType() tupleTypes := make([]attr.Type, len(variadicValues)) From 160c1f31c718c9a34af2415020e4dc5fe266dc30 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 16 Feb 2024 08:10:50 -0500 Subject: [PATCH 06/15] update docs --- internal/fromproto5/arguments_data.go | 2 +- internal/fromproto6/arguments_data.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/fromproto5/arguments_data.go b/internal/fromproto5/arguments_data.go index bcbb17af9..92ac70582 100644 --- a/internal/fromproto5/arguments_data.go +++ b/internal/fromproto5/arguments_data.go @@ -133,7 +133,7 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def } if definition.VariadicParameter != nil { - // Variadic parameters are represented as a TupleType where each element contains the same element type. + // Variadic parameters are represented as a TupleType where each element is the same element type. // // This is intentional and meant to match how variadic arguments are handled by Terraform core, // where the variadic parameter type constraint is applied to each argument individually. diff --git a/internal/fromproto6/arguments_data.go b/internal/fromproto6/arguments_data.go index affb30bcb..053360082 100644 --- a/internal/fromproto6/arguments_data.go +++ b/internal/fromproto6/arguments_data.go @@ -133,7 +133,7 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov6.DynamicValue, def } if definition.VariadicParameter != nil { - // Variadic parameters are represented as a TupleType where each element contains the same element type. + // Variadic parameters are represented as a TupleType where each element is the same element type. // // This is intentional and meant to match how variadic arguments are handled by Terraform core, // where the variadic parameter type constraint is applied to each argument individually. From caa6358252976aa596468e227e98d53886c4151f Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 16 Feb 2024 08:43:32 -0500 Subject: [PATCH 07/15] add doc note --- internal/reflect/slice.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/reflect/slice.go b/internal/reflect/slice.go index 2e2b74ce9..b06367497 100644 --- a/internal/reflect/slice.go +++ b/internal/reflect/slice.go @@ -90,6 +90,7 @@ func reflectSlice(ctx context.Context, typ attr.Type, val tftypes.Value, target } return slice, diags + // Tuple reflection into slices is currently limited to use-cases where all tuple element types are the same. case attr.TypeWithElementTypes: // we need to know the type the slice is wrapping elemType := target.Type().Elem() @@ -240,7 +241,7 @@ func FromSlice(ctx context.Context, typ attr.Type, val reflect.Value, path path. tfElems = append(tfElems, tfVal) } - // Tuple reflection from slices is currently not supported as usage is limited + // Tuple reflection from slices is currently limited to use-cases where all tuple element types are the same. case attr.TypeWithElementTypes: if len(t.ElementTypes()) <= 0 { // If the tuple values are empty as well, we can just pass back an empty slice of the type we received. From 4ca51e327a9e3c9bb154a6ef61c9fbf111fb4f66 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 16 Feb 2024 09:01:54 -0500 Subject: [PATCH 08/15] add changelog --- .changes/unreleased/NOTES-20240216-090121.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changes/unreleased/NOTES-20240216-090121.yaml diff --git a/.changes/unreleased/NOTES-20240216-090121.yaml b/.changes/unreleased/NOTES-20240216-090121.yaml new file mode 100644 index 000000000..45e2f2760 --- /dev/null +++ b/.changes/unreleased/NOTES-20240216-090121.yaml @@ -0,0 +1,8 @@ +kind: NOTES +body: 'function: Changed the framework type for variadic parameters to `types.TupleType`, + where each element is the same element type. Provider-defined functions using a + `types.List` for retrieving variadic argument data will need to update their code + to use `types.Tuple`.' +time: 2024-02-16T09:01:21.079352-05:00 +custom: + Issue: "923" From 13be0fc3808f2afa26ea76c7ede355681dd5e321 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 16 Feb 2024 14:25:37 -0500 Subject: [PATCH 09/15] switch changelog to breaking change --- ...0216-090121.yaml => BREAKING CHANGES-20240216-142501.yaml} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename .changes/unreleased/{NOTES-20240216-090121.yaml => BREAKING CHANGES-20240216-142501.yaml} (83%) diff --git a/.changes/unreleased/NOTES-20240216-090121.yaml b/.changes/unreleased/BREAKING CHANGES-20240216-142501.yaml similarity index 83% rename from .changes/unreleased/NOTES-20240216-090121.yaml rename to .changes/unreleased/BREAKING CHANGES-20240216-142501.yaml index 45e2f2760..4fa7c3043 100644 --- a/.changes/unreleased/NOTES-20240216-090121.yaml +++ b/.changes/unreleased/BREAKING CHANGES-20240216-142501.yaml @@ -1,8 +1,8 @@ -kind: NOTES +kind: BREAKING CHANGES body: 'function: Changed the framework type for variadic parameters to `types.TupleType`, where each element is the same element type. Provider-defined functions using a `types.List` for retrieving variadic argument data will need to update their code to use `types.Tuple`.' -time: 2024-02-16T09:01:21.079352-05:00 +time: 2024-02-16T14:25:01.729428-05:00 custom: Issue: "923" From 8c5175a5aaec268d00db7dbe26a149405c0408d5 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 16 Feb 2024 15:12:50 -0500 Subject: [PATCH 10/15] update to tuple index --- internal/reflect/slice.go | 6 ++---- path/path.go | 12 ++++++++++++ path/path_test.go | 40 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/internal/reflect/slice.go b/internal/reflect/slice.go index b06367497..541bd0633 100644 --- a/internal/reflect/slice.go +++ b/internal/reflect/slice.go @@ -139,8 +139,7 @@ func reflectSlice(ctx context.Context, typ attr.Type, val tftypes.Value, target targetValue := reflect.Zero(elemType) // update our path so we can have nice errors - // utilizing `AtListIndex` because tuple paths are also based on the underlying `PathStepElementKeyInt` - valPath := path.AtListIndex(pos) + valPath := path.AtTupleIndex(pos) // reflect the value into our new target val, valDiags := BuildValue(ctx, elemAttrType, value, targetValue, opts, valPath) @@ -280,8 +279,7 @@ func FromSlice(ctx context.Context, typ attr.Type, val reflect.Value, path path. } for i := 0; i < val.Len(); i++ { - // utilizing `AtListIndex` because tuple paths are also based on the underlying `PathStepElementKeyInt` - valPath := path.AtListIndex(i) + valPath := path.AtTupleIndex(i) val, valDiags := FromValue(ctx, elemAttrType, val.Index(i).Interface(), valPath) diags.Append(valDiags...) diff --git a/path/path.go b/path/path.go index e29a46fe0..4e060bd9a 100644 --- a/path/path.go +++ b/path/path.go @@ -52,6 +52,18 @@ func (p Path) AtListIndex(index int) Path { return copiedPath } +// AtTupleIndex returns a copied path with a new tuple index step at the end. +// The returned path is safe to modify without affecting the original. +// +// Tuple indices are 0-based. The first element of a tuple is 0. +func (p Path) AtTupleIndex(index int) Path { + copiedPath := p.Copy() + + copiedPath.steps.Append(PathStepElementKeyInt(index)) + + return copiedPath +} + // AtMapKey returns a copied path with a new map key step at the end. // The returned path is safe to modify without affecting the original. func (p Path) AtMapKey(key string) Path { diff --git a/path/path_test.go b/path/path_test.go index 7e7d78c63..7742e4335 100644 --- a/path/path_test.go +++ b/path/path_test.go @@ -52,6 +52,46 @@ func TestPathAtListIndex(t *testing.T) { } } +func TestPathAtTupleIndex(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + path path.Path + index int + expected path.Path + }{ + "empty": { + path: path.Empty(), + index: 1, + expected: path.Empty().AtTupleIndex(1), + }, + "shallow": { + path: path.Root("test"), + index: 1, + expected: path.Root("test").AtTupleIndex(1), + }, + "deep": { + path: path.Root("test1").AtTupleIndex(0).AtName("test2"), + index: 1, + expected: path.Root("test1").AtTupleIndex(0).AtName("test2").AtTupleIndex(1), + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := testCase.path.AtTupleIndex(testCase.index) + + if diff := cmp.Diff(got, testCase.expected); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} + func TestPathAtMapKey(t *testing.T) { t.Parallel() From 80c2c2b0417e8da3fa304498241ce1e956510270 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 16 Feb 2024 15:22:52 -0500 Subject: [PATCH 11/15] update comment wording --- internal/fromproto5/arguments_data.go | 2 +- internal/fromproto6/arguments_data.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/fromproto5/arguments_data.go b/internal/fromproto5/arguments_data.go index 92ac70582..a7105f9c1 100644 --- a/internal/fromproto5/arguments_data.go +++ b/internal/fromproto5/arguments_data.go @@ -133,7 +133,7 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def } if definition.VariadicParameter != nil { - // Variadic parameters are represented as a TupleType where each element is the same element type. + // Variadic parameters are represented as a TupleType where each element type of the tuple matches the variadic parameter type. // // This is intentional and meant to match how variadic arguments are handled by Terraform core, // where the variadic parameter type constraint is applied to each argument individually. diff --git a/internal/fromproto6/arguments_data.go b/internal/fromproto6/arguments_data.go index 053360082..d03c013e9 100644 --- a/internal/fromproto6/arguments_data.go +++ b/internal/fromproto6/arguments_data.go @@ -133,7 +133,7 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov6.DynamicValue, def } if definition.VariadicParameter != nil { - // Variadic parameters are represented as a TupleType where each element is the same element type. + // Variadic parameters are represented as a TupleType where each element type of the tuple matches the variadic parameter type. // // This is intentional and meant to match how variadic arguments are handled by Terraform core, // where the variadic parameter type constraint is applied to each argument individually. From 2fb35e781427d659047b124535a62749a71b0c12 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 16 Feb 2024 16:06:44 -0500 Subject: [PATCH 12/15] update doc --- internal/reflect/slice.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/reflect/slice.go b/internal/reflect/slice.go index 541bd0633..5ffaddd9c 100644 --- a/internal/reflect/slice.go +++ b/internal/reflect/slice.go @@ -90,7 +90,12 @@ func reflectSlice(ctx context.Context, typ attr.Type, val tftypes.Value, target } return slice, diags + // Tuple reflection into slices is currently limited to use-cases where all tuple element types are the same. + // + // Overall, Tuple support is limited in the framework, but the main path that executes tuple reflection is the provider-defined function variadic + // parameter. All tuple elements in this variadic parameter will have the same element type. For use-cases where the variadic parameter is a dynamic type, + // all elements will have the same type of `DynamicType` and value of `DynamicValue`, with an underlying value that may be different. case attr.TypeWithElementTypes: // we need to know the type the slice is wrapping elemType := target.Type().Elem() @@ -240,7 +245,12 @@ func FromSlice(ctx context.Context, typ attr.Type, val reflect.Value, path path. tfElems = append(tfElems, tfVal) } + // Tuple reflection from slices is currently limited to use-cases where all tuple element types are the same. + // + // Overall, Tuple support is limited in the framework, but the main path that executes tuple reflection is the provider-defined function variadic + // parameter. All tuple elements in this variadic parameter will have the same element type. For use-cases where the variadic parameter is a dynamic type, + // all elements will have the same type of `DynamicType` and value of `DynamicValue`, with an underlying value that may be different. case attr.TypeWithElementTypes: if len(t.ElementTypes()) <= 0 { // If the tuple values are empty as well, we can just pass back an empty slice of the type we received. From 6dfc0320832b750451451f171e163c7236abcc81 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 20 Feb 2024 12:38:06 -0500 Subject: [PATCH 13/15] add new maintainer note --- internal/fromproto5/arguments_data.go | 18 +++++++++++++++--- internal/fromproto6/arguments_data.go | 18 +++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/internal/fromproto5/arguments_data.go b/internal/fromproto5/arguments_data.go index a7105f9c1..1abc9cd02 100644 --- a/internal/fromproto5/arguments_data.go +++ b/internal/fromproto5/arguments_data.go @@ -133,10 +133,22 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def } if definition.VariadicParameter != nil { - // Variadic parameters are represented as a TupleType where each element type of the tuple matches the variadic parameter type. + // Variadic parameters are represented as a framework Tuple where each element type of the tuple matches the variadic parameter type. + // + // MAINTAINER NOTE: + // Variadic parameters are represented as individual arguments in the CallFunction RPC and Terraform core applies the variadic parameter + // type constraint to each argument individually. For developer convenience, the framework logic below, groups the variadic arguments into a + // framework Tuple where each element type of the tuple matches the variadic parameter type. + // + // Previously, this logic utilized a framework List with an element type that matched the variadic parameter type. Using a List presented an issue with dynamic + // variadic parameters, as each argument was allowed to be any type "individually", rather than having a single type constraint applied to all dynamic elements, + // like a cty.List in Terraform. This eventually results in an error attempting to create a tftypes.List with multiple element types (when unwrapping from a framework + // dynamic to a tftypes concrete value). + // + // While a framework List type can handle multiple dynamic values of different types (due to it's wrapping of dynamic values), `terraform-plugin-go` and `tftypes.List` cannot. + // Currently, the logic for retrieving argument data is dependent on the tftypes package to utilize the framework reflection logic, requiring us to apply the a type constraint + // that is valid in Terraform core and `terraform-plugin-go`, which we are doing here with a Tuple. // - // This is intentional and meant to match how variadic arguments are handled by Terraform core, - // where the variadic parameter type constraint is applied to each argument individually. variadicType := definition.VariadicParameter.GetType() tupleTypes := make([]attr.Type, len(variadicValues)) tupleValues := make([]attr.Value, len(variadicValues)) diff --git a/internal/fromproto6/arguments_data.go b/internal/fromproto6/arguments_data.go index d03c013e9..05f56a7dd 100644 --- a/internal/fromproto6/arguments_data.go +++ b/internal/fromproto6/arguments_data.go @@ -133,10 +133,22 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov6.DynamicValue, def } if definition.VariadicParameter != nil { - // Variadic parameters are represented as a TupleType where each element type of the tuple matches the variadic parameter type. + // Variadic parameters are represented as a framework Tuple where each element type of the tuple matches the variadic parameter type. + // + // MAINTAINER NOTE: + // Variadic parameters are represented as individual arguments in the CallFunction RPC and Terraform core applies the variadic parameter + // type constraint to each argument individually. For developer convenience, the framework logic below, groups the variadic arguments into a + // framework Tuple where each element type of the tuple matches the variadic parameter type. + // + // Previously, this logic utilized a framework List with an element type that matched the variadic parameter type. Using a List presented an issue with dynamic + // variadic parameters, as each argument was allowed to be any type "individually", rather than having a single type constraint applied to all dynamic elements, + // like a cty.List in Terraform. This eventually results in an error attempting to create a tftypes.List with multiple element types (when unwrapping from a framework + // dynamic to a tftypes concrete value). + // + // While a framework List type can handle multiple dynamic values of different types (due to it's wrapping of dynamic values), `terraform-plugin-go` and `tftypes.List` cannot. + // Currently, the logic for retrieving argument data is dependent on the tftypes package to utilize the framework reflection logic, requiring us to apply the a type constraint + // that is valid in Terraform core and `terraform-plugin-go`, which we are doing here with a Tuple. // - // This is intentional and meant to match how variadic arguments are handled by Terraform core, - // where the variadic parameter type constraint is applied to each argument individually. variadicType := definition.VariadicParameter.GetType() tupleTypes := make([]attr.Type, len(variadicValues)) tupleValues := make([]attr.Value, len(variadicValues)) From 43aecc97db5f3feab7603ccf54100eec33648afa Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 20 Feb 2024 12:41:57 -0500 Subject: [PATCH 14/15] remove top part of comment --- internal/fromproto5/arguments_data.go | 4 +--- internal/fromproto6/arguments_data.go | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/internal/fromproto5/arguments_data.go b/internal/fromproto5/arguments_data.go index 1abc9cd02..261caee52 100644 --- a/internal/fromproto5/arguments_data.go +++ b/internal/fromproto5/arguments_data.go @@ -133,8 +133,6 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def } if definition.VariadicParameter != nil { - // Variadic parameters are represented as a framework Tuple where each element type of the tuple matches the variadic parameter type. - // // MAINTAINER NOTE: // Variadic parameters are represented as individual arguments in the CallFunction RPC and Terraform core applies the variadic parameter // type constraint to each argument individually. For developer convenience, the framework logic below, groups the variadic arguments into a @@ -146,7 +144,7 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def // dynamic to a tftypes concrete value). // // While a framework List type can handle multiple dynamic values of different types (due to it's wrapping of dynamic values), `terraform-plugin-go` and `tftypes.List` cannot. - // Currently, the logic for retrieving argument data is dependent on the tftypes package to utilize the framework reflection logic, requiring us to apply the a type constraint + // Currently, the logic for retrieving argument data is dependent on the tftypes package to utilize the framework reflection logic, requiring us to apply a type constraint // that is valid in Terraform core and `terraform-plugin-go`, which we are doing here with a Tuple. // variadicType := definition.VariadicParameter.GetType() diff --git a/internal/fromproto6/arguments_data.go b/internal/fromproto6/arguments_data.go index 05f56a7dd..4464ac9ad 100644 --- a/internal/fromproto6/arguments_data.go +++ b/internal/fromproto6/arguments_data.go @@ -133,8 +133,6 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov6.DynamicValue, def } if definition.VariadicParameter != nil { - // Variadic parameters are represented as a framework Tuple where each element type of the tuple matches the variadic parameter type. - // // MAINTAINER NOTE: // Variadic parameters are represented as individual arguments in the CallFunction RPC and Terraform core applies the variadic parameter // type constraint to each argument individually. For developer convenience, the framework logic below, groups the variadic arguments into a @@ -146,7 +144,7 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov6.DynamicValue, def // dynamic to a tftypes concrete value). // // While a framework List type can handle multiple dynamic values of different types (due to it's wrapping of dynamic values), `terraform-plugin-go` and `tftypes.List` cannot. - // Currently, the logic for retrieving argument data is dependent on the tftypes package to utilize the framework reflection logic, requiring us to apply the a type constraint + // Currently, the logic for retrieving argument data is dependent on the tftypes package to utilize the framework reflection logic, requiring us to apply a type constraint // that is valid in Terraform core and `terraform-plugin-go`, which we are doing here with a Tuple. // variadicType := definition.VariadicParameter.GetType() From 3c050ecc1db9b5d777b05365fc2b945dfa1609fd Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Wed, 21 Feb 2024 08:47:52 -0500 Subject: [PATCH 15/15] fixed the odd spacing in the comment :) --- internal/fromproto5/arguments_data.go | 22 ++++++++++------------ internal/fromproto6/arguments_data.go | 22 ++++++++++------------ 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/internal/fromproto5/arguments_data.go b/internal/fromproto5/arguments_data.go index 261caee52..3ce46cffd 100644 --- a/internal/fromproto5/arguments_data.go +++ b/internal/fromproto5/arguments_data.go @@ -133,20 +133,18 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def } if definition.VariadicParameter != nil { - // MAINTAINER NOTE: - // Variadic parameters are represented as individual arguments in the CallFunction RPC and Terraform core applies the variadic parameter - // type constraint to each argument individually. For developer convenience, the framework logic below, groups the variadic arguments into a - // framework Tuple where each element type of the tuple matches the variadic parameter type. + // MAINTAINER NOTE: Variadic parameters are represented as individual arguments in the CallFunction RPC and Terraform core applies the variadic parameter + // type constraint to each argument individually. For developer convenience, the framework logic below, groups the variadic arguments into a + // framework Tuple where each element type of the tuple matches the variadic parameter type. // - // Previously, this logic utilized a framework List with an element type that matched the variadic parameter type. Using a List presented an issue with dynamic - // variadic parameters, as each argument was allowed to be any type "individually", rather than having a single type constraint applied to all dynamic elements, - // like a cty.List in Terraform. This eventually results in an error attempting to create a tftypes.List with multiple element types (when unwrapping from a framework - // dynamic to a tftypes concrete value). - // - // While a framework List type can handle multiple dynamic values of different types (due to it's wrapping of dynamic values), `terraform-plugin-go` and `tftypes.List` cannot. - // Currently, the logic for retrieving argument data is dependent on the tftypes package to utilize the framework reflection logic, requiring us to apply a type constraint - // that is valid in Terraform core and `terraform-plugin-go`, which we are doing here with a Tuple. + // Previously, this logic utilized a framework List with an element type that matched the variadic parameter type. Using a List presented an issue with dynamic + // variadic parameters, as each argument was allowed to be any type "individually", rather than having a single type constraint applied to all dynamic elements, + // like a cty.List in Terraform. This eventually results in an error attempting to create a tftypes.List with multiple element types (when unwrapping from a framework + // dynamic to a tftypes concrete value). // + // While a framework List type can handle multiple dynamic values of different types (due to it's wrapping of dynamic values), `terraform-plugin-go` and `tftypes.List` cannot. + // Currently, the logic for retrieving argument data is dependent on the tftypes package to utilize the framework reflection logic, requiring us to apply a type constraint + // that is valid in Terraform core and `terraform-plugin-go`, which we are doing here with a Tuple. variadicType := definition.VariadicParameter.GetType() tupleTypes := make([]attr.Type, len(variadicValues)) tupleValues := make([]attr.Value, len(variadicValues)) diff --git a/internal/fromproto6/arguments_data.go b/internal/fromproto6/arguments_data.go index 4464ac9ad..4dd6db7bc 100644 --- a/internal/fromproto6/arguments_data.go +++ b/internal/fromproto6/arguments_data.go @@ -133,20 +133,18 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov6.DynamicValue, def } if definition.VariadicParameter != nil { - // MAINTAINER NOTE: - // Variadic parameters are represented as individual arguments in the CallFunction RPC and Terraform core applies the variadic parameter - // type constraint to each argument individually. For developer convenience, the framework logic below, groups the variadic arguments into a - // framework Tuple where each element type of the tuple matches the variadic parameter type. + // MAINTAINER NOTE: Variadic parameters are represented as individual arguments in the CallFunction RPC and Terraform core applies the variadic parameter + // type constraint to each argument individually. For developer convenience, the framework logic below, groups the variadic arguments into a + // framework Tuple where each element type of the tuple matches the variadic parameter type. // - // Previously, this logic utilized a framework List with an element type that matched the variadic parameter type. Using a List presented an issue with dynamic - // variadic parameters, as each argument was allowed to be any type "individually", rather than having a single type constraint applied to all dynamic elements, - // like a cty.List in Terraform. This eventually results in an error attempting to create a tftypes.List with multiple element types (when unwrapping from a framework - // dynamic to a tftypes concrete value). - // - // While a framework List type can handle multiple dynamic values of different types (due to it's wrapping of dynamic values), `terraform-plugin-go` and `tftypes.List` cannot. - // Currently, the logic for retrieving argument data is dependent on the tftypes package to utilize the framework reflection logic, requiring us to apply a type constraint - // that is valid in Terraform core and `terraform-plugin-go`, which we are doing here with a Tuple. + // Previously, this logic utilized a framework List with an element type that matched the variadic parameter type. Using a List presented an issue with dynamic + // variadic parameters, as each argument was allowed to be any type "individually", rather than having a single type constraint applied to all dynamic elements, + // like a cty.List in Terraform. This eventually results in an error attempting to create a tftypes.List with multiple element types (when unwrapping from a framework + // dynamic to a tftypes concrete value). // + // While a framework List type can handle multiple dynamic values of different types (due to it's wrapping of dynamic values), `terraform-plugin-go` and `tftypes.List` cannot. + // Currently, the logic for retrieving argument data is dependent on the tftypes package to utilize the framework reflection logic, requiring us to apply a type constraint + // that is valid in Terraform core and `terraform-plugin-go`, which we are doing here with a Tuple. variadicType := definition.VariadicParameter.GetType() tupleTypes := make([]attr.Type, len(variadicValues)) tupleValues := make([]attr.Value, len(variadicValues))