Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

function: Switch the representation of variadic arguments to types.Tuple #923

Merged
merged 16 commits into from
Feb 21, 2024
Merged
8 changes: 8 additions & 0 deletions .changes/unreleased/BREAKING CHANGES-20240216-142501.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
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-16T14:25:01.729428-05:00
custom:
Issue: "923"
12 changes: 6 additions & 6 deletions function/arguments_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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),
)

Expand Down
127 changes: 125 additions & 2 deletions function/arguments_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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(),
Expand All @@ -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 {
Expand All @@ -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 != "" {
Expand Down Expand Up @@ -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",
),
},
Expand Down
2 changes: 1 addition & 1 deletion function/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
16 changes: 14 additions & 2 deletions internal/fromproto5/arguments_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 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.
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...)

Expand Down
57 changes: 37 additions & 20 deletions internal/fromproto5/arguments_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
),
}),
Expand All @@ -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"),
},
Expand All @@ -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"),
Expand Down Expand Up @@ -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{},
),
}),
Expand All @@ -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"),
},
Expand All @@ -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"),
Expand All @@ -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{},
),
}),
Expand All @@ -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"),
},
Expand All @@ -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{},
Expand All @@ -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"),
Expand Down
Loading
Loading