tfsdk: Properly set attribute path in (Plan).SetAttribute() and (State).SetAttribute() reflection diagnostics (#133)

Reference: #128

Author: bflad

.changelog/133.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+tfsdk: Diagnostics returned from `(Plan).SetAttribute()` and `(State).SetAttribute()` reflection will now properly include attribute path
+```

internal/reflect/interfaces.go

@@ -62,7 +62,7 @@ func NewUnknownable(ctx context.Context, typ attr.Type, val tftypes.Value, targe
 
 // FromUnknownable creates an attr.Value from the data in an Unknownable.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromUnknownable(ctx context.Context, typ attr.Type, val Unknownable, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 
@@ -156,7 +156,7 @@ func NewNullable(ctx context.Context, typ attr.Type, val tftypes.Value, target r
 
 // FromNullable creates an attr.Value from the data in a Nullable.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromNullable(ctx context.Context, typ attr.Type, val Nullable, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 
@@ -242,7 +242,7 @@ func NewValueConverter(ctx context.Context, typ attr.Type, val tftypes.Value, ta
 // tftypes.ValueCreator, calling its ToTerraform5Value method and converting
 // the result to an attr.Value using `typ`.
 //
-// It is meant to be called from OutOf, not directly.
+// It is meant to be called from FromValue, not directly.
 func FromValueCreator(ctx context.Context, typ attr.Type, val tftypes.ValueCreator, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 	raw, err := val.ToTerraform5Value()
@@ -307,7 +307,7 @@ func NewAttributeValue(ctx context.Context, typ attr.Type, val tftypes.Value, ta
 // validation on that attr.Value to make sure it matches the type produced by
 // `typ`.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromAttributeValue(ctx context.Context, typ attr.Type, val attr.Value, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 

internal/reflect/map.go

@@ -93,7 +93,7 @@ func Map(ctx context.Context, typ attr.Type, val tftypes.Value, target reflect.V
 // `val` must be a map type with keys that are a string type. The attr.Value
 // will be of the type produced by `typ`.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromMap(ctx context.Context, typ attr.TypeWithElementType, val reflect.Value, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 	tfType := typ.TerraformType(ctx)

internal/reflect/number.go

@@ -235,7 +235,7 @@ func Number(ctx context.Context, typ attr.Type, val tftypes.Value, target reflec
 
 // FromInt creates an attr.Value using `typ` from an int64.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromInt(ctx context.Context, typ attr.Type, val int64, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 	err := tftypes.ValidateValue(tftypes.Number, val)
@@ -262,7 +262,7 @@ func FromInt(ctx context.Context, typ attr.Type, val int64, path *tftypes.Attrib
 
 // FromUint creates an attr.Value using `typ` from a uint64.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromUint(ctx context.Context, typ attr.Type, val uint64, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 	err := tftypes.ValidateValue(tftypes.Number, val)
@@ -289,7 +289,7 @@ func FromUint(ctx context.Context, typ attr.Type, val uint64, path *tftypes.Attr
 
 // FromFloat creates an attr.Value using `typ` from a float64.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromFloat(ctx context.Context, typ attr.Type, val float64, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 	err := tftypes.ValidateValue(tftypes.Number, val)
@@ -316,7 +316,7 @@ func FromFloat(ctx context.Context, typ attr.Type, val float64, path *tftypes.At
 
 // FromBigFloat creates an attr.Value using `typ` from a *big.Float.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromBigFloat(ctx context.Context, typ attr.Type, val *big.Float, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 	err := tftypes.ValidateValue(tftypes.Number, val)
@@ -343,7 +343,7 @@ func FromBigFloat(ctx context.Context, typ attr.Type, val *big.Float, path *tfty
 
 // FromBigInt creates an attr.Value using `typ` from a *big.Int.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromBigInt(ctx context.Context, typ attr.Type, val *big.Int, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 	fl := big.NewFloat(0).SetInt(val)

internal/reflect/outof.go

@@ -11,18 +11,10 @@ import (
 	"github.com/hashicorp/terraform-plugin-go/tftypes"
 )
 
-// OutOf is the inverse of Into, taking a Go value (`val`) and transforming it
+// FromValue is the inverse of Into, taking a Go value (`val`) and transforming it
 // into an attr.Value using the attr.Type supplied. `val` will first be
 // transformed into a tftypes.Value, then passed to `typ`'s ValueFromTerraform
 // method.
-func OutOf(ctx context.Context, typ attr.Type, val interface{}) (attr.Value, diag.Diagnostics) {
-	return FromValue(ctx, typ, val, tftypes.NewAttributePath())
-}
-
-// FromValue is recursively called to turn `val` into an `attr.Value` using
-// `typ`.
-//
-// It is meant to be called through OutOf, not directly.
 func FromValue(ctx context.Context, typ attr.Type, val interface{}, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 

internal/reflect/pointer.go

@@ -73,7 +73,7 @@ func pointerSafeZeroValue(ctx context.Context, target reflect.Value) reflect.Val
 // it will recurse into FromValue to find the attr.Value of the type the value
 // the pointer is referencing.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromPointer(ctx context.Context, typ attr.Type, value reflect.Value, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 

internal/reflect/primitive.go

@@ -55,7 +55,7 @@ func Primitive(ctx context.Context, typ attr.Type, val tftypes.Value, target ref
 
 // FromString returns an attr.Value as produced by `typ` from a string.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromString(ctx context.Context, typ attr.Type, val string, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 	err := tftypes.ValidateValue(tftypes.String, val)
@@ -82,7 +82,7 @@ func FromString(ctx context.Context, typ attr.Type, val string, path *tftypes.At
 
 // FromBool returns an attr.Value as produced by `typ` from a bool.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromBool(ctx context.Context, typ attr.Type, val bool, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 	err := tftypes.ValidateValue(tftypes.Bool, val)

internal/reflect/slice.go

@@ -88,7 +88,7 @@ func reflectSlice(ctx context.Context, typ attr.Type, val tftypes.Value, target
 // for each element in the slice, using the element type or types defined on
 // `typ` to construct values for them.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromSlice(ctx context.Context, typ attr.Type, val reflect.Value, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 

internal/reflect/struct.go

@@ -148,7 +148,7 @@ func Struct(ctx context.Context, typ attr.Type, object tftypes.Value, target ref
 // into FromValue for each attribute, using the type of the attribute as
 // reported by `typ`.
 //
-// It is meant to be called through OutOf, not directly.
+// It is meant to be called through FromValue, not directly.
 func FromStruct(ctx context.Context, typ attr.TypeWithAttributeTypes, val reflect.Value, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
 	var diags diag.Diagnostics
 	objTypes := map[string]tftypes.Type{}

tfsdk/plan.go

@@ -79,7 +79,7 @@ func (p Plan) GetAttribute(ctx context.Context, path *tftypes.AttributePath) (at
 // should be a struct whose values have one of the attr.Value types. Each field
 // must be tagged with the corresponding schema field.
 func (p *Plan) Set(ctx context.Context, val interface{}) diag.Diagnostics {
-	newPlanAttrValue, diags := reflect.OutOf(ctx, p.Schema.AttributeType(), val)
+	newPlanAttrValue, diags := reflect.FromValue(ctx, p.Schema.AttributeType(), val, tftypes.NewAttributePath())
 	if diags.HasError() {
 		return diags
 	}
@@ -115,7 +115,7 @@ func (p *Plan) SetAttribute(ctx context.Context, path *tftypes.AttributePath, va
 		return diags
 	}
 
-	newVal, newValDiags := reflect.OutOf(ctx, attrType, val)
+	newVal, newValDiags := reflect.FromValue(ctx, attrType, val, path)
 	diags.Append(newValDiags...)
 
 	if diags.HasError() {

tfsdk/plan_test.go

@@ -408,9 +408,7 @@ func TestPlanSetAttribute(t *testing.T) {
 			}, map[string]tftypes.Value{
 				"name": tftypes.NewValue(tftypes.String, "originalname"),
 			}),
-			// TODO: Diagnostic should include WithAttributeName("name")
-			// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/128
-			expectedDiags: diag.Diagnostics{testtypes.TestErrorDiagnostic(tftypes.NewAttributePath())},
+			expectedDiags: diag.Diagnostics{testtypes.TestErrorDiagnostic(tftypes.NewAttributePath().WithAttributeName("name"))},
 		},
 		"AttrTypeWithValidateWarning": {
 			plan: Plan{
@@ -440,10 +438,6 @@ func TestPlanSetAttribute(t *testing.T) {
 				"name": tftypes.NewValue(tftypes.String, "newname"),
 			}),
 			expectedDiags: diag.Diagnostics{
-				// TODO: Diagnostic should include WithAttributeName("name")
-				// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/128
-				testtypes.TestWarningDiagnostic(tftypes.NewAttributePath()),
-				// This duplicate should go away with the above fix.
 				testtypes.TestWarningDiagnostic(tftypes.NewAttributePath().WithAttributeName("name")),
 			},
 		},

tfsdk/state.go

@@ -88,7 +88,7 @@ func (s *State) Set(ctx context.Context, val interface{}) diag.Diagnostics {
 			),
 		}
 	}
-	newStateAttrValue, diags := reflect.OutOf(ctx, s.Schema.AttributeType(), val)
+	newStateAttrValue, diags := reflect.FromValue(ctx, s.Schema.AttributeType(), val, tftypes.NewAttributePath())
 	if diags.HasError() {
 		return diags
 	}
@@ -124,7 +124,7 @@ func (s *State) SetAttribute(ctx context.Context, path *tftypes.AttributePath, v
 		return diags
 	}
 
-	newVal, newValDiags := reflect.OutOf(ctx, attrType, val)
+	newVal, newValDiags := reflect.FromValue(ctx, attrType, val, path)
 	diags.Append(newValDiags...)
 
 	if diags.HasError() {

tfsdk/state_test.go

@@ -1539,9 +1539,7 @@ func TestStateSetAttribute(t *testing.T) {
 			}, map[string]tftypes.Value{
 				"name": tftypes.NewValue(tftypes.String, "originalname"),
 			}),
-			// TODO: Diagnostic should include WithAttributeName("name")
-			// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/128
-			expectedDiags: diag.Diagnostics{testtypes.TestErrorDiagnostic(tftypes.NewAttributePath())},
+			expectedDiags: diag.Diagnostics{testtypes.TestErrorDiagnostic(tftypes.NewAttributePath().WithAttributeName("name"))},
 		},
 		"AttrTypeWithValidateWarning": {
 			state: State{
@@ -1571,10 +1569,6 @@ func TestStateSetAttribute(t *testing.T) {
 				"name": tftypes.NewValue(tftypes.String, "newname"),
 			}),
 			expectedDiags: diag.Diagnostics{
-				// TODO: Diagnostic should include WithAttributeName("name")
-				// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/128
-				testtypes.TestWarningDiagnostic(tftypes.NewAttributePath()),
-				// This duplicate should go away with the above fix.
 				testtypes.TestWarningDiagnostic(tftypes.NewAttributePath().WithAttributeName("name")),
 			},
 		},