Deprecate ValidateFunc and SchemaValidateFunc

Ensure both ValidateFunc and ValidateDiagFunc can't be set
truncate TypeSet attribute path and explain why

Author: appilon

helper/schema/schema.go

@@ -223,7 +223,17 @@ type Schema struct {
 	//
 	// ValidateFunc is honored only when the schema's Type is set to TypeInt,
 	// TypeFloat, TypeString, TypeBool, or TypeMap. It is ignored for all other types.
-	ValidateFunc     SchemaValidateFunc
+	//
+	// Deprecated: please use ValidateDiagFunc
+	ValidateFunc SchemaValidateFunc
+
+	// ValidateDiagFunc allows individual fields to define arbitrary validation
+	// logic. It is yielded the provided config value as an interface{} that is
+	// guaranteed to be of the proper Schema type, and it can yield diagnostics
+	// based on inspection of that value.
+	//
+	// ValidateDiagFunc is honored only when the schema's Type is set to TypeInt,
+	// TypeFloat, TypeString, TypeBool, or TypeMap. It is ignored for all other types.
 	ValidateDiagFunc SchemaValidateDiagFunc
 
 	// Sensitive ensures that the attribute's value does not get displayed in
@@ -293,7 +303,12 @@ type SchemaStateFunc func(interface{}) string
 
 // SchemaValidateFunc is a function used to validate a single field in the
 // schema.
+//
+// Deprecated: please use SchemaValidateDiagFunc
 type SchemaValidateFunc func(interface{}, string) ([]string, []error)
+
+// SchemaValidateDiagFunc is a function used to validate a single field in the
+// schema and has Diagnostic support.
 type SchemaValidateDiagFunc func(interface{}, string) diag.Diagnostics
 
 func (s *Schema) GoString() string {
@@ -842,6 +857,10 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro
 			}
 		}
 
+		if v.ValidateFunc != nil && v.ValidateDiagFunc != nil {
+			return fmt.Errorf("%s: ValidateFunc and ValidateDiagFunc cannot both be set", k)
+		}
+
 		if v.Deprecated == "" && v.Removed == "" {
 			if !isValidFieldName(k) {
 				return fmt.Errorf("%s: Field name may only contain lowercase alphanumeric characters & underscores.", k)
@@ -1606,8 +1625,7 @@ func (m schemaMap) validateList(
 	raw interface{},
 	schema *Schema,
 	c *terraform.ResourceConfig,
-	path cty.Path,
-	isTypeSet bool) diag.Diagnostics {
+	path cty.Path) diag.Diagnostics {
 
 	var diags diag.Diagnostics
 
@@ -1672,19 +1690,15 @@ func (m schemaMap) validateList(
 	for i, raw := range raws {
 		key := fmt.Sprintf("%s.%d", k, i)
 
-		// TODO: figure out cty.Path for TypeSet
-		// if isTypeSet {
-		// } else {
-		// }
-		p := append(path.Copy(), cty.IndexStep{Key: cty.NumberIntVal(int64(i))})
-
 		// Reify the key value from the ResourceConfig.
 		// If the list was computed we have all raw values, but some of these
 		// may be known in the config, and aren't individually marked as Computed.
 		if r, ok := c.Get(key); ok {
 			raw = r
 		}
 
+		p := append(path.Copy(), cty.IndexStep{Key: cty.NumberIntVal(int64(i))})
+
 		switch t := schema.Elem.(type) {
 		case *Resource:
 			// This is a sub-resource
@@ -1899,7 +1913,7 @@ func (m schemaMap) validateObject(
 		if k != "" {
 			key = fmt.Sprintf("%s.%s", k, subK)
 		}
-		diags = diags.Append(m.validate(key, s, c, append(path, cty.GetAttrStep{Name: subK})))
+		diags = diags.Append(m.validate(key, s, c, append(path.Copy(), cty.GetAttrStep{Name: subK})))
 	}
 
 	// Detect any extra/unknown keys and report those as errors.
@@ -1912,7 +1926,7 @@ func (m schemaMap) validateObject(
 				diags = diags.Append(&diag.Diagnostic{
 					Severity:      diag.Error,
 					Summary:       "Invalid or unknown key",
-					AttributePath: append(path, cty.GetAttrStep{Name: subk}),
+					AttributePath: append(path.Copy(), cty.GetAttrStep{Name: subk}),
 				})
 			}
 		}
@@ -2026,12 +2040,19 @@ func (m schemaMap) validateType(
 	path cty.Path) diag.Diagnostics {
 
 	var diags diag.Diagnostics
-
 	switch schema.Type {
 	case TypeList:
-		diags = m.validateList(k, raw, schema, c, path, false)
+		diags = m.validateList(k, raw, schema, c, path)
 	case TypeSet:
-		diags = m.validateList(k, raw, schema, c, path, true)
+		// indexing into sets is not representable in the current protocol
+		// best we can do is associate the path up to this attribute
+		// we may be able to try and enhance the rendered text with the rest of
+		// the path ie "error in TypeSet element { 'foo' : 'bar' }",
+		// but it will not be a perfect UX
+		diags = m.validateList(k, raw, schema, c, path)
+		for _, d := range diags {
+			d.AttributePath = path
+		}
 	case TypeMap:
 		diags = m.validateMap(k, raw, schema, c, path)
 	default:

helper/schema/schema_test.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/hashcode"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/internal/configs/hcl2shim"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
@@ -3654,6 +3655,22 @@ func TestSchemaMap_InternalValidate(t *testing.T) {
 			},
 			true,
 		},
+
+		"ValidateFunc and ValidateDiagFunc cannot both be set": {
+			map[string]*Schema{
+				"foo": {
+					Type:     TypeInt,
+					Required: true,
+					ValidateFunc: func(interface{}, string) ([]string, []error) {
+						return nil, nil
+					},
+					ValidateDiagFunc: func(interface{}, string) diag.Diagnostics {
+						return nil
+					},
+				},
+			},
+			true,
+		},
 	}
 
 	for tn, tc := range cases {