From 45ce888032dd985962b8b09bf5d5d146387cb7b5 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 29 Oct 2021 12:02:13 -0400 Subject: [PATCH 1/3] diag: Add WithPath function and remove AttributeErrorDiagnostic and AttributeWarningDiagnostic types Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/169 Instead of embedding path information directly in diagnostic types, it can instead be handled as a wrapper. This encourages a separation of concerns and simplifies implementations. Future enhancements, such as type checking wrapped diagnostics in https://github.com/hashicorp/terraform-plugin-framework/issues/218, can enable advanced use cases. --- .changelog/pending.txt | 7 ++++ diag/attribute_error_diagnostic.go | 40 +++------------------ diag/attribute_warning_diagnostic.go | 40 +++------------------ diag/diagnostic.go | 3 ++ diag/with_path.go | 54 ++++++++++++++++++++++++++++ internal/reflect/diags.go | 24 +++---------- internal/reflect/interfaces.go | 5 ++- internal/reflect/map.go | 20 +++++------ internal/reflect/number.go | 5 ++- internal/reflect/pointer.go | 5 ++- internal/reflect/pointer_test.go | 5 ++- internal/reflect/primitive.go | 15 ++++---- internal/reflect/slice.go | 15 ++++---- internal/reflect/struct.go | 35 ++++++++---------- internal/reflect/struct_test.go | 25 ++++++------- internal/testing/types/diags.go | 4 +-- tfsdk/block.go | 41 +++++++++++++++++++++ tfsdk/config.go | 16 +-------- tfsdk/config_test.go | 12 +++---- tfsdk/plan.go | 16 +-------- tfsdk/plan_test.go | 12 +++---- tfsdk/state.go | 16 +-------- tfsdk/state_test.go | 12 +++---- tfsdk/value_as_test.go | 32 ++++++++++------- 24 files changed, 206 insertions(+), 253 deletions(-) create mode 100644 .changelog/pending.txt create mode 100644 diag/with_path.go create mode 100644 tfsdk/block.go diff --git a/.changelog/pending.txt b/.changelog/pending.txt new file mode 100644 index 000000000..899afa897 --- /dev/null +++ b/.changelog/pending.txt @@ -0,0 +1,7 @@ +```release-note:breaking-change +diag: The `AttributeErrorDiagnostic` and `AttributeWarningDiagnostic` types have been removed. Any usage can be replaced with `DiagnosticWithPath`. +``` + +```release-note:enhancement +diag: Added `WithPath()` function to wrap or overwrite diagnostic path information. +``` diff --git a/diag/attribute_error_diagnostic.go b/diag/attribute_error_diagnostic.go index 922f1d14e..c8da17d71 100644 --- a/diag/attribute_error_diagnostic.go +++ b/diag/attribute_error_diagnostic.go @@ -4,42 +4,10 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" ) -var _ DiagnosticWithPath = AttributeErrorDiagnostic{} - -// AttributeErrorDiagnostic is a generic attribute diagnostic with error severity. -type AttributeErrorDiagnostic struct { - ErrorDiagnostic - - path *tftypes.AttributePath -} - -// Equal returns true if the other diagnostic is wholly equivalent. -func (d AttributeErrorDiagnostic) Equal(other Diagnostic) bool { - aed, ok := other.(AttributeErrorDiagnostic) - - if !ok { - return false - } - - if !aed.Path().Equal(d.Path()) { - return false - } - - return aed.ErrorDiagnostic.Equal(d.ErrorDiagnostic) -} - -// Path returns the diagnostic path. -func (d AttributeErrorDiagnostic) Path() *tftypes.AttributePath { - return d.path -} - // NewAttributeErrorDiagnostic returns a new error severity diagnostic with the given summary, detail, and path. -func NewAttributeErrorDiagnostic(path *tftypes.AttributePath, summary string, detail string) AttributeErrorDiagnostic { - return AttributeErrorDiagnostic{ - ErrorDiagnostic: ErrorDiagnostic{ - detail: detail, - summary: summary, - }, - path: path, +func NewAttributeErrorDiagnostic(path *tftypes.AttributePath, summary string, detail string) DiagnosticWithPath { + return withPath{ + Diagnostic: NewErrorDiagnostic(summary, detail), + path: path, } } diff --git a/diag/attribute_warning_diagnostic.go b/diag/attribute_warning_diagnostic.go index d58460a17..9525394fb 100644 --- a/diag/attribute_warning_diagnostic.go +++ b/diag/attribute_warning_diagnostic.go @@ -4,42 +4,10 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" ) -var _ DiagnosticWithPath = AttributeWarningDiagnostic{} - -// AttributeErrorDiagnostic is a generic attribute diagnostic with warning severity. -type AttributeWarningDiagnostic struct { - WarningDiagnostic - - path *tftypes.AttributePath -} - -// Equal returns true if the other diagnostic is wholly equivalent. -func (d AttributeWarningDiagnostic) Equal(other Diagnostic) bool { - awd, ok := other.(AttributeWarningDiagnostic) - - if !ok { - return false - } - - if !awd.Path().Equal(d.Path()) { - return false - } - - return awd.WarningDiagnostic.Equal(d.WarningDiagnostic) -} - -// Path returns the diagnostic path. -func (d AttributeWarningDiagnostic) Path() *tftypes.AttributePath { - return d.path -} - // NewAttributeWarningDiagnostic returns a new warning severity diagnostic with the given summary, detail, and path. -func NewAttributeWarningDiagnostic(path *tftypes.AttributePath, summary string, detail string) AttributeWarningDiagnostic { - return AttributeWarningDiagnostic{ - WarningDiagnostic: WarningDiagnostic{ - detail: detail, - summary: summary, - }, - path: path, +func NewAttributeWarningDiagnostic(path *tftypes.AttributePath, summary string, detail string) DiagnosticWithPath { + return withPath{ + Diagnostic: NewWarningDiagnostic(summary, detail), + path: path, } } diff --git a/diag/diagnostic.go b/diag/diagnostic.go index ded076f8d..fef464a58 100644 --- a/diag/diagnostic.go +++ b/diag/diagnostic.go @@ -12,6 +12,9 @@ import ( // // See the ErrorDiagnostic and WarningDiagnostic concrete types for generic // implementations. +// +// To add path information to an existing diagnostic, see the WithPath() +// function. type Diagnostic interface { // Severity returns the desired level of feedback for the diagnostic. Severity() Severity diff --git a/diag/with_path.go b/diag/with_path.go new file mode 100644 index 000000000..d02ff685c --- /dev/null +++ b/diag/with_path.go @@ -0,0 +1,54 @@ +package diag + +import ( + "github.com/hashicorp/terraform-plugin-go/tftypes" +) + +var _ DiagnosticWithPath = withPath{} + +// withPath wraps a diagnostic with path information. +type withPath struct { + Diagnostic + + path *tftypes.AttributePath +} + +// Equal returns true if the other diagnostic is wholly equivalent. +func (d withPath) Equal(other Diagnostic) bool { + o, ok := other.(withPath) + + if !ok { + return false + } + + if !d.Path().Equal(o.Path()) { + return false + } + + if d.Diagnostic == nil { + return d.Diagnostic == o.Diagnostic + } + + return d.Diagnostic.Equal(o.Diagnostic) +} + +// Path returns the diagnostic path. +func (d withPath) Path() *tftypes.AttributePath { + return d.path +} + +// WithPath wraps a diagnostic with path information or overwrites the path. +func WithPath(path *tftypes.AttributePath, d Diagnostic) DiagnosticWithPath { + wp, ok := d.(withPath) + + if !ok { + return withPath{ + Diagnostic: d, + path: path, + } + } + + wp.path = path + + return wp +} diff --git a/internal/reflect/diags.go b/internal/reflect/diags.go index 23e9136f9..618408ba0 100644 --- a/internal/reflect/diags.go +++ b/internal/reflect/diags.go @@ -9,7 +9,7 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" ) -func toTerraform5ValueErrorDiag(err error, path *tftypes.AttributePath) diag.AttributeErrorDiagnostic { +func toTerraform5ValueErrorDiag(err error, path *tftypes.AttributePath) diag.DiagnosticWithPath { return diag.NewAttributeErrorDiagnostic( path, "Value Conversion Error", @@ -17,7 +17,7 @@ func toTerraform5ValueErrorDiag(err error, path *tftypes.AttributePath) diag.Att ) } -func toTerraformValueErrorDiag(err error, path *tftypes.AttributePath) diag.AttributeErrorDiagnostic { +func toTerraformValueErrorDiag(err error, path *tftypes.AttributePath) diag.DiagnosticWithPath { return diag.NewAttributeErrorDiagnostic( path, "Value Conversion Error", @@ -25,7 +25,7 @@ func toTerraformValueErrorDiag(err error, path *tftypes.AttributePath) diag.Attr ) } -func validateValueErrorDiag(err error, path *tftypes.AttributePath) diag.AttributeErrorDiagnostic { +func validateValueErrorDiag(err error, path *tftypes.AttributePath) diag.DiagnosticWithPath { return diag.NewAttributeErrorDiagnostic( path, "Value Conversion Error", @@ -33,7 +33,7 @@ func validateValueErrorDiag(err error, path *tftypes.AttributePath) diag.Attribu ) } -func valueFromTerraformErrorDiag(err error, path *tftypes.AttributePath) diag.AttributeErrorDiagnostic { +func valueFromTerraformErrorDiag(err error, path *tftypes.AttributePath) diag.DiagnosticWithPath { return diag.NewAttributeErrorDiagnostic( path, "Value Conversion Error", @@ -44,7 +44,6 @@ func valueFromTerraformErrorDiag(err error, path *tftypes.AttributePath) diag.At type DiagIntoIncompatibleType struct { Val tftypes.Value TargetType reflect.Type - AttrPath *tftypes.AttributePath Err error } @@ -71,24 +70,16 @@ func (d DiagIntoIncompatibleType) Equal(o diag.Diagnostic) bool { if d.TargetType != od.TargetType { return false } - if !d.AttrPath.Equal(od.AttrPath) { - return false - } if d.Err.Error() != od.Err.Error() { return false } return true } -func (d DiagIntoIncompatibleType) Path() *tftypes.AttributePath { - return d.AttrPath -} - type DiagNewAttributeValueIntoWrongType struct { ValType reflect.Type TargetType reflect.Type SchemaType attr.Type - AttrPath *tftypes.AttributePath } func (d DiagNewAttributeValueIntoWrongType) Severity() diag.Severity { @@ -117,12 +108,5 @@ func (d DiagNewAttributeValueIntoWrongType) Equal(o diag.Diagnostic) bool { if !d.SchemaType.Equal(od.SchemaType) { return false } - if !d.AttrPath.Equal(od.AttrPath) { - return false - } return true } - -func (d DiagNewAttributeValueIntoWrongType) Path() *tftypes.AttributePath { - return d.AttrPath -} diff --git a/internal/reflect/interfaces.go b/internal/reflect/interfaces.go index 975d23fa9..18f40b29f 100644 --- a/internal/reflect/interfaces.go +++ b/internal/reflect/interfaces.go @@ -291,12 +291,11 @@ func NewAttributeValue(ctx context.Context, typ attr.Type, val tftypes.Value, ta return target, append(diags, valueFromTerraformErrorDiag(err, path)) } if reflect.TypeOf(res) != target.Type() { - diags.Append(DiagNewAttributeValueIntoWrongType{ + diags.Append(diag.WithPath(path, DiagNewAttributeValueIntoWrongType{ ValType: reflect.TypeOf(res), TargetType: target.Type(), SchemaType: typ, - AttrPath: path, - }) + })) return target, diags } return reflect.ValueOf(res), diags diff --git a/internal/reflect/map.go b/internal/reflect/map.go index 62b87a1ea..c780be541 100644 --- a/internal/reflect/map.go +++ b/internal/reflect/map.go @@ -18,31 +18,28 @@ func Map(ctx context.Context, typ attr.Type, val tftypes.Value, target reflect.V // this only works with maps, so check that out first if underlyingValue.Kind() != reflect.Map { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("expected a map type, got %s", target.Type()), - }) + })) return target, diags } if !val.Type().Is(tftypes.Map{}) { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("cannot reflect %s into a map, must be a map", val.Type().String()), - }) + })) return target, diags } elemTyper, ok := typ.(attr.TypeWithElementType) if !ok { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("cannot reflect map using type information provided by %T, %T must be an attr.TypeWithElementType", typ, typ), - }) + })) return target, diags } @@ -51,12 +48,11 @@ func Map(ctx context.Context, typ attr.Type, val tftypes.Value, target reflect.V values := map[string]tftypes.Value{} err := val.As(&values) if err != nil { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, TargetType: target.Type(), - AttrPath: path, Err: err, - }) + })) return target, diags } diff --git a/internal/reflect/number.go b/internal/reflect/number.go index 9f23c2557..1df91935c 100644 --- a/internal/reflect/number.go +++ b/internal/reflect/number.go @@ -29,12 +29,11 @@ func Number(ctx context.Context, typ attr.Type, val tftypes.Value, target reflec result := big.NewFloat(0) err := val.As(&result) if err != nil { - diags.Append(DiagIntoIncompatibleType{ - AttrPath: path, + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Err: err, TargetType: target.Type(), Val: val, - }) + })) return target, diags } roundingError := fmt.Errorf("cannot store %s in %s", result.String(), target.Type()) diff --git a/internal/reflect/pointer.go b/internal/reflect/pointer.go index 07508a1a4..e96234727 100644 --- a/internal/reflect/pointer.go +++ b/internal/reflect/pointer.go @@ -18,12 +18,11 @@ func Pointer(ctx context.Context, typ attr.Type, val tftypes.Value, target refle var diags diag.Diagnostics if target.Kind() != reflect.Ptr { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("cannot dereference pointer, not a pointer, is a %s (%s)", target.Type(), target.Kind()), - }) + })) return target, diags } // we may have gotten a nil pointer, so we need to create our own that diff --git a/internal/reflect/pointer_test.go b/internal/reflect/pointer_test.go index 0ab923208..25ff5eaa8 100644 --- a/internal/reflect/pointer_test.go +++ b/internal/reflect/pointer_test.go @@ -21,12 +21,11 @@ func TestPointer_notAPointer(t *testing.T) { var s string expectedDiags := diag.Diagnostics{ - refl.DiagIntoIncompatibleType{ + diag.WithPath(tftypes.NewAttributePath(), refl.DiagIntoIncompatibleType{ Val: tftypes.NewValue(tftypes.String, "hello"), TargetType: reflect.TypeOf(s), - AttrPath: tftypes.NewAttributePath(), Err: fmt.Errorf("cannot dereference pointer, not a pointer, is a %s (%s)", reflect.TypeOf(s), reflect.TypeOf(s).Kind()), - }, + }), } _, diags := refl.Pointer(context.Background(), types.StringType, tftypes.NewValue(tftypes.String, "hello"), reflect.ValueOf(s), refl.Options{}, tftypes.NewAttributePath()) diff --git a/internal/reflect/primitive.go b/internal/reflect/primitive.go index 63e43b491..7fde1714c 100644 --- a/internal/reflect/primitive.go +++ b/internal/reflect/primitive.go @@ -22,12 +22,11 @@ func Primitive(ctx context.Context, typ attr.Type, val tftypes.Value, target ref var b bool err := val.As(&b) if err != nil { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, TargetType: target.Type(), - AttrPath: path, Err: err, - }) + })) return target, diags } return reflect.ValueOf(b).Convert(target.Type()), nil @@ -35,22 +34,20 @@ func Primitive(ctx context.Context, typ attr.Type, val tftypes.Value, target ref var s string err := val.As(&s) if err != nil { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, TargetType: target.Type(), - AttrPath: path, Err: err, - }) + })) return target, diags } return reflect.ValueOf(s).Convert(target.Type()), nil default: - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, TargetType: target.Type(), - AttrPath: path, Err: errors.New("unknown type"), - }) + })) return target, diags } } diff --git a/internal/reflect/slice.go b/internal/reflect/slice.go index 1f1c9d8f2..d9359a02a 100644 --- a/internal/reflect/slice.go +++ b/internal/reflect/slice.go @@ -17,23 +17,21 @@ func reflectSlice(ctx context.Context, typ attr.Type, val tftypes.Value, target // this only works with slices, so check that out first if target.Kind() != reflect.Slice { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("expected a slice type, got %s", target.Type()), - }) + })) return target, diags } // TODO: check that the val is a list or set or tuple elemTyper, ok := typ.(attr.TypeWithElementType) if !ok { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("cannot reflect %s using type information provided by %T, %T must be an attr.TypeWithElementType", val.Type(), typ, typ), - }) + })) return target, diags } @@ -42,12 +40,11 @@ func reflectSlice(ctx context.Context, typ attr.Type, val tftypes.Value, target var values []tftypes.Value err := val.As(&values) if err != nil { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, TargetType: target.Type(), - AttrPath: path, Err: err, - }) + })) return target, diags } diff --git a/internal/reflect/struct.go b/internal/reflect/struct.go index aea7f60dd..5ea3c206d 100644 --- a/internal/reflect/struct.go +++ b/internal/reflect/struct.go @@ -30,31 +30,28 @@ func Struct(ctx context.Context, typ attr.Type, object tftypes.Value, target ref // this only works with object values, so make sure that constraint is // met if target.Kind() != reflect.Struct { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: object, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("expected a struct type, got %s", target.Type()), - }) + })) return target, diags } if !object.Type().Is(tftypes.Object{}) { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: object, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("cannot reflect %s into a struct, must be an object", object.Type().String()), - }) + })) return target, diags } attrsType, ok := typ.(attr.TypeWithAttributeTypes) if !ok { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: object, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("cannot reflect object using type information provided by %T, %T must be an attr.TypeWithAttributeTypes", typ, typ), - }) + })) return target, diags } @@ -62,12 +59,11 @@ func Struct(ctx context.Context, typ attr.Type, object tftypes.Value, target ref var objectFields map[string]tftypes.Value err := object.As(&objectFields) if err != nil { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: object, TargetType: target.Type(), - AttrPath: path, Err: err, - }) + })) return target, diags } @@ -75,12 +71,11 @@ func Struct(ctx context.Context, typ attr.Type, object tftypes.Value, target ref // passed in targetFields, err := getStructTags(ctx, target, path) if err != nil { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: object, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("error retrieving field names from struct tags: %w", err), - }) + })) return target, diags } @@ -106,12 +101,11 @@ func Struct(ctx context.Context, typ attr.Type, object tftypes.Value, target ref if len(targetMissing) > 0 { missing = append(missing, fmt.Sprintf("Object defines fields not found in struct: %s.", commaSeparatedString(targetMissing))) } - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: object, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("mismatch between struct and object: %s", strings.Join(missing, " ")), - }) + })) return target, diags } @@ -123,12 +117,11 @@ func Struct(ctx context.Context, typ attr.Type, object tftypes.Value, target ref for field, structFieldPos := range targetFields { attrType, ok := attrTypes[field] if !ok { - diags.Append(DiagIntoIncompatibleType{ + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: object, TargetType: target.Type(), - AttrPath: path, Err: fmt.Errorf("could not find type information for attribute in supplied attr.Type %T", typ), - }) + })) return target, diags } structField := result.Field(structFieldPos) diff --git a/internal/reflect/struct_test.go b/internal/reflect/struct_test.go index 679448500..c3376715a 100644 --- a/internal/reflect/struct_test.go +++ b/internal/reflect/struct_test.go @@ -22,12 +22,11 @@ func TestNewStruct_notAnObject(t *testing.T) { var s struct{} expectedDiags := diag.Diagnostics{ - refl.DiagIntoIncompatibleType{ - AttrPath: tftypes.NewAttributePath(), + diag.WithPath(tftypes.NewAttributePath(), refl.DiagIntoIncompatibleType{ Val: tftypes.NewValue(tftypes.String, "hello"), TargetType: reflect.TypeOf(s), Err: fmt.Errorf("cannot reflect %s into a struct, must be an object", tftypes.String), - }, + }), } _, diags := refl.Struct(context.Background(), types.StringType, tftypes.NewValue(tftypes.String, "hello"), reflect.ValueOf(s), refl.Options{}, tftypes.NewAttributePath()) @@ -50,12 +49,11 @@ func TestNewStruct_notAStruct(t *testing.T) { var s string expectedDiags := diag.Diagnostics{ - refl.DiagIntoIncompatibleType{ - AttrPath: tftypes.NewAttributePath(), + diag.WithPath(tftypes.NewAttributePath(), refl.DiagIntoIncompatibleType{ TargetType: reflect.TypeOf(s), Val: val, Err: fmt.Errorf("expected a struct type, got string"), - }, + }), } _, diags := refl.Struct(context.Background(), types.ObjectType{ @@ -80,12 +78,11 @@ func TestNewStruct_objectMissingFields(t *testing.T) { A string `tfsdk:"a"` } expectedDiags := diag.Diagnostics{ - refl.DiagIntoIncompatibleType{ - AttrPath: tftypes.NewAttributePath(), + diag.WithPath(tftypes.NewAttributePath(), refl.DiagIntoIncompatibleType{ Err: errors.New("mismatch between struct and object: Struct defines fields not found in object: a."), Val: val, TargetType: reflect.TypeOf(s), - }, + }), } _, diags := refl.Struct(context.Background(), types.ObjectType{}, val, reflect.ValueOf(s), refl.Options{}, tftypes.NewAttributePath()) @@ -108,12 +105,11 @@ func TestNewStruct_structMissingProperties(t *testing.T) { var s struct{} expectedDiags := diag.Diagnostics{ - refl.DiagIntoIncompatibleType{ - AttrPath: tftypes.NewAttributePath(), + diag.WithPath(tftypes.NewAttributePath(), refl.DiagIntoIncompatibleType{ Err: errors.New("mismatch between struct and object: Object defines fields not found in struct: a."), Val: val, TargetType: reflect.TypeOf(s), - }, + }), } _, diags := refl.Struct(context.Background(), types.ObjectType{ @@ -142,12 +138,11 @@ func TestNewStruct_objectMissingFieldsAndStructMissingProperties(t *testing.T) { A string `tfsdk:"a"` } expectedDiags := diag.Diagnostics{ - refl.DiagIntoIncompatibleType{ - AttrPath: tftypes.NewAttributePath(), + diag.WithPath(tftypes.NewAttributePath(), refl.DiagIntoIncompatibleType{ TargetType: reflect.TypeOf(s), Val: val, Err: errors.New("mismatch between struct and object: Struct defines fields not found in object: a. Object defines fields not found in struct: b."), - }, + }), } _, diags := refl.Struct(context.Background(), types.ObjectType{ diff --git a/internal/testing/types/diags.go b/internal/testing/types/diags.go index a9a28b0a0..811f1d303 100644 --- a/internal/testing/types/diags.go +++ b/internal/testing/types/diags.go @@ -5,7 +5,7 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" ) -func TestErrorDiagnostic(path *tftypes.AttributePath) diag.AttributeErrorDiagnostic { +func TestErrorDiagnostic(path *tftypes.AttributePath) diag.DiagnosticWithPath { return diag.NewAttributeErrorDiagnostic( path, "Error Diagnostic", @@ -13,7 +13,7 @@ func TestErrorDiagnostic(path *tftypes.AttributePath) diag.AttributeErrorDiagnos ) } -func TestWarningDiagnostic(path *tftypes.AttributePath) diag.AttributeWarningDiagnostic { +func TestWarningDiagnostic(path *tftypes.AttributePath) diag.DiagnosticWithPath { return diag.NewAttributeWarningDiagnostic( path, "Warning Diagnostic", diff --git a/tfsdk/block.go b/tfsdk/block.go new file mode 100644 index 000000000..d87d7ea3a --- /dev/null +++ b/tfsdk/block.go @@ -0,0 +1,41 @@ +package tfsdk + +import ( + "github.com/hashicorp/terraform-plugin-go/tftypes" +) + +// Block defines the constraints and behaviors of a single structural field in a +// schema. +type Block struct { + tftypes.AttributePathStepper + + // Attributes + Attributes map[string]Attribute + + // Blocks can have their own nested blocks. This nested map of blocks + // behaves exactly like the map of blocks on the Schema type. + Blocks map[string]Block + + // Description is used in various tooling, like the language server, to + // give practitioners more information about what this attribute is, + // what it's for, and how it should be used. It should be written as + // plain text, with no special formatting. + Description string + + // MarkdownDescription is used in various tooling, like the + // documentation generator, to give practitioners more information + // about what this attribute is, what it's for, and how it should be + // used. It should be formatted using Markdown. + MarkdownDescription string + + // NestingMode indicates the block kind. + NestingMode NestingMode +} + +// ApplyTerraform5AttributePathStep transparently calls +// ApplyTerraform5AttributePathStep on a.Type or a.Attributes, +// whichever is non-nil. It allows Attributes to be walked using tftypes.Walk +// and tftypes.Transform. +// func (b Block) ApplyTerraform5AttributePathStep(step tftypes.AttributePathStep) (interface{}, error) { +// return b.NestingMode.ApplyTerraform5AttributePathStep(step) +// } diff --git a/tfsdk/config.go b/tfsdk/config.go index 962d5e9c8..57451584b 100644 --- a/tfsdk/config.go +++ b/tfsdk/config.go @@ -44,22 +44,8 @@ func (c Config) GetAttribute(ctx context.Context, path *tftypes.AttributePath, t valueAsDiags := ValueAs(ctx, attrValue, target) // ValueAs does not have path information for its Diagnostics. - // TODO: Update to use diagnostic SetPath method. - // Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/169 for idx, valueAsDiag := range valueAsDiags { - if valueAsDiag.Severity() == diag.SeverityError { - valueAsDiags[idx] = diag.NewAttributeErrorDiagnostic( - path, - valueAsDiag.Summary(), - valueAsDiag.Detail(), - ) - } else if valueAsDiag.Severity() == diag.SeverityWarning { - valueAsDiags[idx] = diag.NewAttributeWarningDiagnostic( - path, - valueAsDiag.Summary(), - valueAsDiag.Detail(), - ) - } + valueAsDiags[idx] = diag.WithPath(path, valueAsDiag) } diags.Append(valueAsDiags...) diff --git a/tfsdk/config_test.go b/tfsdk/config_test.go index 8c1811547..29369d05b 100644 --- a/tfsdk/config_test.go +++ b/tfsdk/config_test.go @@ -250,15 +250,13 @@ func TestConfigGetAttribute(t *testing.T) { target: new(testtypes.String), expected: new(testtypes.String), expectedDiags: diag.Diagnostics{ - diag.NewAttributeErrorDiagnostic( + diag.WithPath( tftypes.NewAttributePath().WithAttributeName("name"), - "Value Conversion Error", intreflect.DiagNewAttributeValueIntoWrongType{ ValType: reflect.TypeOf(types.String{Value: "namevalue"}), TargetType: reflect.TypeOf(testtypes.String{}), - AttrPath: tftypes.NewAttributePath().WithAttributeName("name"), SchemaType: types.StringType, - }.Detail(), + }, ), }, }, @@ -283,15 +281,13 @@ func TestConfigGetAttribute(t *testing.T) { target: new(bool), expected: new(bool), expectedDiags: diag.Diagnostics{ - diag.NewAttributeErrorDiagnostic( + diag.WithPath( tftypes.NewAttributePath().WithAttributeName("name"), - "Value Conversion Error", intreflect.DiagIntoIncompatibleType{ Val: tftypes.NewValue(tftypes.String, "namevalue"), TargetType: reflect.TypeOf(false), Err: fmt.Errorf("can't unmarshal %s into *%T, expected boolean", tftypes.String, false), - AttrPath: tftypes.NewAttributePath().WithAttributeName("name"), - }.Detail(), + }, ), }, }, diff --git a/tfsdk/plan.go b/tfsdk/plan.go index 36766818c..cfd00b3b1 100644 --- a/tfsdk/plan.go +++ b/tfsdk/plan.go @@ -44,22 +44,8 @@ func (p Plan) GetAttribute(ctx context.Context, path *tftypes.AttributePath, tar valueAsDiags := ValueAs(ctx, attrValue, target) // ValueAs does not have path information for its Diagnostics. - // TODO: Update to use diagnostic SetPath method. - // Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/169 for idx, valueAsDiag := range valueAsDiags { - if valueAsDiag.Severity() == diag.SeverityError { - valueAsDiags[idx] = diag.NewAttributeErrorDiagnostic( - path, - valueAsDiag.Summary(), - valueAsDiag.Detail(), - ) - } else if valueAsDiag.Severity() == diag.SeverityWarning { - valueAsDiags[idx] = diag.NewAttributeWarningDiagnostic( - path, - valueAsDiag.Summary(), - valueAsDiag.Detail(), - ) - } + valueAsDiags[idx] = diag.WithPath(path, valueAsDiag) } diags.Append(valueAsDiags...) diff --git a/tfsdk/plan_test.go b/tfsdk/plan_test.go index 85d527384..ac436745c 100644 --- a/tfsdk/plan_test.go +++ b/tfsdk/plan_test.go @@ -250,15 +250,13 @@ func TestPlanGetAttribute(t *testing.T) { target: new(testtypes.String), expected: new(testtypes.String), expectedDiags: diag.Diagnostics{ - diag.NewAttributeErrorDiagnostic( + diag.WithPath( tftypes.NewAttributePath().WithAttributeName("name"), - "Value Conversion Error", intreflect.DiagNewAttributeValueIntoWrongType{ ValType: reflect.TypeOf(types.String{Value: "namevalue"}), TargetType: reflect.TypeOf(testtypes.String{}), - AttrPath: tftypes.NewAttributePath().WithAttributeName("name"), SchemaType: types.StringType, - }.Detail(), + }, ), }, }, @@ -283,15 +281,13 @@ func TestPlanGetAttribute(t *testing.T) { target: new(bool), expected: new(bool), expectedDiags: diag.Diagnostics{ - diag.NewAttributeErrorDiagnostic( + diag.WithPath( tftypes.NewAttributePath().WithAttributeName("name"), - "Value Conversion Error", intreflect.DiagIntoIncompatibleType{ Val: tftypes.NewValue(tftypes.String, "namevalue"), TargetType: reflect.TypeOf(false), Err: fmt.Errorf("can't unmarshal %s into *%T, expected boolean", tftypes.String, false), - AttrPath: tftypes.NewAttributePath().WithAttributeName("name"), - }.Detail(), + }, ), }, }, diff --git a/tfsdk/state.go b/tfsdk/state.go index 5c69b648b..92be0294b 100644 --- a/tfsdk/state.go +++ b/tfsdk/state.go @@ -44,22 +44,8 @@ func (s State) GetAttribute(ctx context.Context, path *tftypes.AttributePath, ta valueAsDiags := ValueAs(ctx, attrValue, target) // ValueAs does not have path information for its Diagnostics. - // TODO: Update to use diagnostic SetPath method. - // Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/169 for idx, valueAsDiag := range valueAsDiags { - if valueAsDiag.Severity() == diag.SeverityError { - valueAsDiags[idx] = diag.NewAttributeErrorDiagnostic( - path, - valueAsDiag.Summary(), - valueAsDiag.Detail(), - ) - } else if valueAsDiag.Severity() == diag.SeverityWarning { - valueAsDiags[idx] = diag.NewAttributeWarningDiagnostic( - path, - valueAsDiag.Summary(), - valueAsDiag.Detail(), - ) - } + valueAsDiags[idx] = diag.WithPath(path, valueAsDiag) } diags.Append(valueAsDiags...) diff --git a/tfsdk/state_test.go b/tfsdk/state_test.go index c3ae4d7f6..88a4acdd2 100644 --- a/tfsdk/state_test.go +++ b/tfsdk/state_test.go @@ -947,15 +947,13 @@ func TestStateGetAttribute(t *testing.T) { target: new(testtypes.String), expected: new(testtypes.String), expectedDiags: diag.Diagnostics{ - diag.NewAttributeErrorDiagnostic( + diag.WithPath( tftypes.NewAttributePath().WithAttributeName("name"), - "Value Conversion Error", intreflect.DiagNewAttributeValueIntoWrongType{ ValType: reflect.TypeOf(types.String{Value: "namevalue"}), TargetType: reflect.TypeOf(testtypes.String{}), - AttrPath: tftypes.NewAttributePath().WithAttributeName("name"), SchemaType: types.StringType, - }.Detail(), + }, ), }, }, @@ -980,15 +978,13 @@ func TestStateGetAttribute(t *testing.T) { target: new(bool), expected: new(bool), expectedDiags: diag.Diagnostics{ - diag.NewAttributeErrorDiagnostic( + diag.WithPath( tftypes.NewAttributePath().WithAttributeName("name"), - "Value Conversion Error", intreflect.DiagIntoIncompatibleType{ Val: tftypes.NewValue(tftypes.String, "namevalue"), TargetType: reflect.TypeOf(false), Err: fmt.Errorf("can't unmarshal %s into *%T, expected boolean", tftypes.String, false), - AttrPath: tftypes.NewAttributePath().WithAttributeName("name"), - }.Detail(), + }, ), }, }, diff --git a/tfsdk/value_as_test.go b/tfsdk/value_as_test.go index 1dd5222e9..b59c9317a 100644 --- a/tfsdk/value_as_test.go +++ b/tfsdk/value_as_test.go @@ -48,22 +48,30 @@ func TestValueAs(t *testing.T) { "incompatible-type": { val: types.String{Value: "hello"}, target: newInt64Pointer(0), - expectedDiags: diag.Diagnostics{reflect.DiagIntoIncompatibleType{ - Val: tftypes.NewValue(tftypes.String, "hello"), - TargetType: goreflect.TypeOf(int64(0)), - Err: fmt.Errorf("can't unmarshal %s into %T, expected *big.Float", tftypes.String, big.NewFloat(0)), - AttrPath: tftypes.NewAttributePath(), - }}, + expectedDiags: diag.Diagnostics{ + diag.WithPath( + tftypes.NewAttributePath(), + reflect.DiagIntoIncompatibleType{ + Val: tftypes.NewValue(tftypes.String, "hello"), + TargetType: goreflect.TypeOf(int64(0)), + Err: fmt.Errorf("can't unmarshal %s into %T, expected *big.Float", tftypes.String, big.NewFloat(0)), + }, + ), + }, }, "different-type": { val: types.String{Value: "hello"}, target: &testtypes.String{}, - expectedDiags: diag.Diagnostics{reflect.DiagNewAttributeValueIntoWrongType{ - ValType: goreflect.TypeOf(types.String{Value: "hello"}), - TargetType: goreflect.TypeOf(testtypes.String{}), - AttrPath: tftypes.NewAttributePath(), - SchemaType: types.StringType, - }}, + expectedDiags: diag.Diagnostics{ + diag.WithPath( + tftypes.NewAttributePath(), + reflect.DiagNewAttributeValueIntoWrongType{ + ValType: goreflect.TypeOf(types.String{Value: "hello"}), + TargetType: goreflect.TypeOf(testtypes.String{}), + SchemaType: types.StringType, + }, + ), + }, }, } From f4b2ae5e27fda3826634e715e6e1cb58ae30af43 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 29 Oct 2021 12:04:04 -0400 Subject: [PATCH 2/3] Update CHANGELOG for #219 --- .changelog/{pending.txt => 219.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{pending.txt => 219.txt} (100%) diff --git a/.changelog/pending.txt b/.changelog/219.txt similarity index 100% rename from .changelog/pending.txt rename to .changelog/219.txt From bb3b9b277ee2e27cfb3cf2995ed902eee24dd347 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 29 Oct 2021 12:06:40 -0400 Subject: [PATCH 3/3] Remove errantly included file --- tfsdk/block.go | 41 ----------------------------------------- 1 file changed, 41 deletions(-) delete mode 100644 tfsdk/block.go diff --git a/tfsdk/block.go b/tfsdk/block.go deleted file mode 100644 index d87d7ea3a..000000000 --- a/tfsdk/block.go +++ /dev/null @@ -1,41 +0,0 @@ -package tfsdk - -import ( - "github.com/hashicorp/terraform-plugin-go/tftypes" -) - -// Block defines the constraints and behaviors of a single structural field in a -// schema. -type Block struct { - tftypes.AttributePathStepper - - // Attributes - Attributes map[string]Attribute - - // Blocks can have their own nested blocks. This nested map of blocks - // behaves exactly like the map of blocks on the Schema type. - Blocks map[string]Block - - // Description is used in various tooling, like the language server, to - // give practitioners more information about what this attribute is, - // what it's for, and how it should be used. It should be written as - // plain text, with no special formatting. - Description string - - // MarkdownDescription is used in various tooling, like the - // documentation generator, to give practitioners more information - // about what this attribute is, what it's for, and how it should be - // used. It should be formatted using Markdown. - MarkdownDescription string - - // NestingMode indicates the block kind. - NestingMode NestingMode -} - -// ApplyTerraform5AttributePathStep transparently calls -// ApplyTerraform5AttributePathStep on a.Type or a.Attributes, -// whichever is non-nil. It allows Attributes to be walked using tftypes.Walk -// and tftypes.Transform. -// func (b Block) ApplyTerraform5AttributePathStep(step tftypes.AttributePathStep) (interface{}, error) { -// return b.NestingMode.ApplyTerraform5AttributePathStep(step) -// }