stringEncoder fromPropertyValue will now format numbers (#1366)

This is necessary for backwards compatibility with SDKv{1,2} based
provider. It is
dangerous; there are many ways to represent bools and numbers so we are
unable to round trip.

This is the bridge side fix for
pulumi/pulumi-aws#2806.

pf/internal/convert/convert_test.go

@@ -18,6 +18,7 @@ import (
 	"fmt"
 	"math/big"
 	"reflect"
+	"strconv"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-go/tftypes"
@@ -27,17 +28,28 @@ import (
 )
 
 type convertTurnaroundTestCase struct {
-	name    string
-	ty      tftypes.Type
-	val     tftypes.Value
-	prop    resource.PropertyValue
-	normVal func(tftypes.Value) interface{}
+	name     string
+	ty       tftypes.Type
+	val      tftypes.Value
+	prop     resource.PropertyValue
+	normVal  func(tftypes.Value) any
+	normProp func(resource.PropertyValue) any
 }
 
 func TestConvertTurnaround(t *testing.T) {
 	t.Parallel()
 
 	cases := convertTurnaroundTestCases(tftypes.String, resource.NewStringProperty, "", "test-string")
+	cases = append(cases, convertTurnaroundTestCases(tftypes.String, func(x string) resource.PropertyValue {
+		if x == "" {
+			return resource.NewNullProperty()
+		}
+		v, err := strconv.ParseFloat(x, 64)
+		if err != nil {
+			panic(err)
+		}
+		return resource.NewNumberProperty(v)
+	}, "0", "8.3").withNormProp(func(p resource.PropertyValue) any { return fmt.Sprintf("%v", p.V) })...)
 	cases = append(cases, convertTurnaroundTestCases(tftypes.Bool, resource.NewBoolProperty, false, true)...)
 	cases = append(cases, convertTurnaroundTestCases(tftypes.Number, resource.NewNumberProperty, float64(0), 42, 3.12)...)
 
@@ -162,7 +174,11 @@ func TestConvertTurnaround(t *testing.T) {
 			actual, err := decoder.toPropertyValue(testcase.val)
 			require.NoError(t, err)
 
-			assert.Equal(t, testcase.prop, actual)
+			if f := testcase.normProp; f != nil {
+				assert.Equal(t, f(testcase.prop), f(actual))
+			} else {
+				assert.Equal(t, testcase.prop, actual)
+			}
 		})
 
 		t.Run(testcase.name+"/pu2tf", func(t *testing.T) {
@@ -171,8 +187,8 @@ func TestConvertTurnaround(t *testing.T) {
 			actual, err := encoder.fromPropertyValue(testcase.prop)
 			require.NoError(t, err)
 
-			if testcase.normVal != nil {
-				assert.Equal(t, testcase.normVal(testcase.val), testcase.normVal(actual))
+			if f := testcase.normVal; f != nil {
+				assert.Equal(t, f(testcase.val), f(actual))
 			} else {
 				assert.Equal(t, testcase.val, actual)
 			}
@@ -198,9 +214,20 @@ func convertTurnaroundNilTestCase(ty tftypes.Type) convertTurnaroundTestCase {
 	}
 }
 
+type convertTurnaroundTestCaseSet []convertTurnaroundTestCase
+
+func (c convertTurnaroundTestCaseSet) withNormProp(norm func(resource.PropertyValue) any) convertTurnaroundTestCaseSet {
+	n := make([]convertTurnaroundTestCase, len(c))
+	for i, v := range c {
+		v.normProp = norm
+		n[i] = v
+	}
+	return n
+}
+
 func convertTurnaroundTestCases[T any](
 	ty tftypes.Type, topv func(x T) resource.PropertyValue, vals ...T,
-) []convertTurnaroundTestCase {
+) convertTurnaroundTestCaseSet {
 	var zero T
 	zeroValue := topv(zero)
 	cases := []convertTurnaroundTestCase{

pf/internal/convert/string.go

@@ -37,20 +37,52 @@ func (*stringEncoder) fromPropertyValue(p resource.PropertyValue) (tftypes.Value
 	if propertyValueIsUnkonwn(p) {
 		return tftypes.NewValue(tftypes.String, tftypes.UnknownValue), nil
 	}
-	if p.IsNull() {
+
+	switch {
+	case p.IsNull():
 		return tftypes.NewValue(tftypes.String, nil), nil
-	}
 
-	// Special-case to tolerate booleans.
-	if p.IsBool() {
+	// Special-case values that can be converted into strings for backward
+	// comparability with SDKv{1,2} based resources.
+	//
+	// Unfortunately, it is not possible to round trip values that are not string
+	// typed with full fidelity. For example, consider this simple YAML program:
+	//
+	//	resources:
+	//	  r:
+	//	    type: some:simple:Resource
+	//	    properties:
+	//	      stringType: 0x1234
+	//
+	// In YAML, 0x1234 is parsed as the number 4660, so our recourse will receive
+	// `4660` as its output. This problem appears even for simple numbers:
+	//
+	//
+	//	resources:
+	//	  r:
+	//	    type: some:simple:Resource
+	//	    properties:
+	//	      s1: 1
+	//	      s2: 1.0
+	//
+	// Go formats float64(1) as "1", so we get the expected result. float64(1) is
+	// equal to float64(1.0), so we are unable to distinguish between s1 and s2.
+	//
+	// We have the same problems with bools: YAML parses "YES" as true, so we are
+	// unable to distinguish between the two.
+
+	case p.IsBool():
 		return tftypes.NewValue(tftypes.String, fmt.Sprintf("%v", p.BoolValue())), nil
-	}
+	case p.IsNumber():
+		return tftypes.NewValue(tftypes.String, fmt.Sprintf("%v", p.NumberValue())), nil
+
+	case p.IsString():
+		return tftypes.NewValue(tftypes.String, p.StringValue()), nil
 
-	if !p.IsString() {
+	default:
 		return tftypes.NewValue(tftypes.String, nil),
 			fmt.Errorf("Expected a string, got: %v", p)
 	}
-	return tftypes.NewValue(tftypes.String, p.StringValue()), nil
 }
 
 func (*stringDecoder) toPropertyValue(v tftypes.Value) (resource.PropertyValue, error) {

pkg/tfgen/generate_test.go

@@ -98,6 +98,7 @@ func Test_ForceNew(t *testing.T) {
 	}
 
 	for _, test := range cases {
+		test := test
 		t.Run(test.Name, func(t *testing.T) {
 			v := &test.Var
 			actuallyForcesNew := v.forceNew()