tfprotov5+tfprotov6: Cleanup and unit test fromproto/toproto packages (…

…#367)

Reference: #365

Adjusts the following packages:

* `tfprotov5/internal/fromproto`
* `tfprotov5/internal/tf5server`
* `tfprotov5/internal/toproto`
* `tfprotov6/internal/fromproto`
* `tfprotov6/internal/tf6server`
* `tfprotov6/internal/toproto`

With the following changes:

- Removed unnecessary response types from `fromproto`
- Removed unnecessary request types from `toproto`
- Added unit testing for all functionality
- Added nil panic protection for all functionality and removed extraneous nil checking in callers (e.g. around `DynamicValue` handling)
- Clarified protocol request/response naming in the servers
- Added vertical whitespace for readability and consistency

As noted in some of the new unit testing, many of the response types can also have their error returns removed, but this will be saved for a followup change to somewhat reduce this already large refactoring.

tfprotov5/internal/fromproto/data_source.go

@@ -8,59 +8,29 @@ import (
 	"github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5"
 )
 
-func DataSourceMetadata(in *tfplugin5.GetMetadata_DataSourceMetadata) *tfprotov5.DataSourceMetadata {
+func ValidateDataSourceConfigRequest(in *tfplugin5.ValidateDataSourceConfig_Request) *tfprotov5.ValidateDataSourceConfigRequest {
 	if in == nil {
 		return nil
 	}
 
-	return &tfprotov5.DataSourceMetadata{
-		TypeName: in.TypeName,
-	}
-}
-
-func ValidateDataSourceConfigRequest(in *tfplugin5.ValidateDataSourceConfig_Request) (*tfprotov5.ValidateDataSourceConfigRequest, error) {
 	resp := &tfprotov5.ValidateDataSourceConfigRequest{
+		Config:   DynamicValue(in.Config),
 		TypeName: in.TypeName,
 	}
-	if in.Config != nil {
-		resp.Config = DynamicValue(in.Config)
-	}
-	return resp, nil
+
+	return resp
 }
 
-func ValidateDataSourceConfigResponse(in *tfplugin5.ValidateDataSourceConfig_Response) (*tfprotov5.ValidateDataSourceConfigResponse, error) {
-	diags, err := Diagnostics(in.Diagnostics)
-	if err != nil {
-		return nil, err
+func ReadDataSourceRequest(in *tfplugin5.ReadDataSource_Request) *tfprotov5.ReadDataSourceRequest {
+	if in == nil {
+		return nil
 	}
-	return &tfprotov5.ValidateDataSourceConfigResponse{
-		Diagnostics: diags,
-	}, nil
-}
 
-func ReadDataSourceRequest(in *tfplugin5.ReadDataSource_Request) (*tfprotov5.ReadDataSourceRequest, error) {
 	resp := &tfprotov5.ReadDataSourceRequest{
-		TypeName: in.TypeName,
-	}
-	if in.Config != nil {
-		resp.Config = DynamicValue(in.Config)
-	}
-	if in.ProviderMeta != nil {
-		resp.ProviderMeta = DynamicValue(in.ProviderMeta)
+		Config:       DynamicValue(in.Config),
+		ProviderMeta: DynamicValue(in.ProviderMeta),
+		TypeName:     in.TypeName,
 	}
-	return resp, nil
-}
 
-func ReadDataSourceResponse(in *tfplugin5.ReadDataSource_Response) (*tfprotov5.ReadDataSourceResponse, error) {
-	diags, err := Diagnostics(in.Diagnostics)
-	if err != nil {
-		return nil, err
-	}
-	resp := &tfprotov5.ReadDataSourceResponse{
-		Diagnostics: diags,
-	}
-	if in.State != nil {
-		resp.State = DynamicValue(in.State)
-	}
-	return resp, nil
+	return resp
 }

tfprotov5/internal/fromproto/data_source_test.go

@@ -0,0 +1,117 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package fromproto_test
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/fromproto"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5"
+)
+
+func TestReadDataSourceRequest(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]struct {
+		in       *tfplugin5.ReadDataSource_Request
+		expected *tfprotov5.ReadDataSourceRequest
+	}{
+		"nil": {
+			in:       nil,
+			expected: nil,
+		},
+		"zero": {
+			in:       &tfplugin5.ReadDataSource_Request{},
+			expected: &tfprotov5.ReadDataSourceRequest{},
+		},
+		"Config": {
+			in: &tfplugin5.ReadDataSource_Request{
+				Config: testTfplugin5DynamicValue(),
+			},
+			expected: &tfprotov5.ReadDataSourceRequest{
+				Config: testTfprotov5DynamicValue(),
+			},
+		},
+		"ProviderMeta": {
+			in: &tfplugin5.ReadDataSource_Request{
+				ProviderMeta: testTfplugin5DynamicValue(),
+			},
+			expected: &tfprotov5.ReadDataSourceRequest{
+				ProviderMeta: testTfprotov5DynamicValue(),
+			},
+		},
+		"TypeName": {
+			in: &tfplugin5.ReadDataSource_Request{
+				TypeName: "test",
+			},
+			expected: &tfprotov5.ReadDataSourceRequest{
+				TypeName: "test",
+			},
+		},
+	}
+
+	for name, testCase := range testCases {
+		name, testCase := name, testCase
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			got := fromproto.ReadDataSourceRequest(testCase.in)
+
+			if diff := cmp.Diff(got, testCase.expected); diff != "" {
+				t.Errorf("unexpected difference: %s", diff)
+			}
+		})
+	}
+}
+
+func TestValidateDataSourceConfigRequest(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]struct {
+		in       *tfplugin5.ValidateDataSourceConfig_Request
+		expected *tfprotov5.ValidateDataSourceConfigRequest
+	}{
+		"nil": {
+			in:       nil,
+			expected: nil,
+		},
+		"zero": {
+			in:       &tfplugin5.ValidateDataSourceConfig_Request{},
+			expected: &tfprotov5.ValidateDataSourceConfigRequest{},
+		},
+		"Config": {
+			in: &tfplugin5.ValidateDataSourceConfig_Request{
+				Config: testTfplugin5DynamicValue(),
+			},
+			expected: &tfprotov5.ValidateDataSourceConfigRequest{
+				Config: testTfprotov5DynamicValue(),
+			},
+		},
+		"TypeName": {
+			in: &tfplugin5.ValidateDataSourceConfig_Request{
+				TypeName: "test",
+			},
+			expected: &tfprotov5.ValidateDataSourceConfigRequest{
+				TypeName: "test",
+			},
+		},
+	}
+
+	for name, testCase := range testCases {
+		name, testCase := name, testCase
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			got := fromproto.ValidateDataSourceConfigRequest(testCase.in)
+
+			if diff := cmp.Diff(got, testCase.expected); diff != "" {
+				t.Errorf("unexpected difference: %s", diff)
+			}
+		})
+	}
+}

tfprotov5/internal/fromproto/doc.go

@@ -0,0 +1,6 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+// Package fromproto converts Protocol Buffers generated tfplugin5 types into
+// terraform-plugin-go tfprotov5 types.
+package fromproto

tfprotov5/internal/fromproto/types.go → tfprotov5/internal/fromproto/dynamic_value.go

@@ -9,8 +9,14 @@ import (
 )
 
 func DynamicValue(in *tfplugin5.DynamicValue) *tfprotov5.DynamicValue {
-	return &tfprotov5.DynamicValue{
+	if in == nil {
+		return nil
+	}
+
+	resp := &tfprotov5.DynamicValue{
 		MsgPack: in.Msgpack,
 		JSON:    in.Json,
 	}
+
+	return resp
 }

tfprotov5/internal/fromproto/dynamic_value_test.go

@@ -0,0 +1,28 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package fromproto_test
+
+import (
+	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/toproto"
+	"github.com/hashicorp/terraform-plugin-go/tftypes"
+)
+
+func testTfplugin5DynamicValue() *tfplugin5.DynamicValue {
+	return toproto.DynamicValue(testTfprotov5DynamicValue())
+}
+
+func testTfprotov5DynamicValue() *tfprotov5.DynamicValue {
+	dynamicValue, err := tfprotov5.NewDynamicValue(
+		tftypes.Object{},
+		tftypes.NewValue(tftypes.Object{}, nil),
+	)
+
+	if err != nil {
+		panic("unable to create DynamicValue: " + err.Error())
+	}
+
+	return &dynamicValue
+}

tfprotov5/internal/fromproto/function.go

@@ -8,9 +8,9 @@ import (
 	"github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5"
 )
 
-func CallFunctionRequest(in *tfplugin5.CallFunction_Request) (*tfprotov5.CallFunctionRequest, error) {
+func CallFunctionRequest(in *tfplugin5.CallFunction_Request) *tfprotov5.CallFunctionRequest {
 	if in == nil {
-		return nil, nil
+		return nil
 	}
 
 	resp := &tfprotov5.CallFunctionRequest{
@@ -22,15 +22,15 @@ func CallFunctionRequest(in *tfplugin5.CallFunction_Request) (*tfprotov5.CallFun
 		resp.Arguments = append(resp.Arguments, DynamicValue(argument))
 	}
 
-	return resp, nil
+	return resp
 }
 
-func GetFunctionsRequest(in *tfplugin5.GetFunctions_Request) (*tfprotov5.GetFunctionsRequest, error) {
+func GetFunctionsRequest(in *tfplugin5.GetFunctions_Request) *tfprotov5.GetFunctionsRequest {
 	if in == nil {
-		return nil, nil
+		return nil
 	}
 
 	resp := &tfprotov5.GetFunctionsRequest{}
 
-	return resp, nil
+	return resp
 }