internal/framework/types: convert regexp to value validation

hashicorp · Apr 24, 2024 · fa504f7 · fa504f7
1 parent e1be78d
commit fa504f7
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 61 deletions.
diff --git a/internal/framework/types/regexp.go b/internal/framework/types/regexp.go
@@ -8,20 +8,16 @@ import (
 	"fmt"
 	"regexp"
 
-	"github.com/YakDriver/regexache"
 	"github.com/hashicorp/terraform-plugin-framework/attr"
 	"github.com/hashicorp/terraform-plugin-framework/attr/xattr"
 	"github.com/hashicorp/terraform-plugin-framework/diag"
-	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
 	"github.com/hashicorp/terraform-plugin-go/tftypes"
 )
 
 var (
-	_ xattr.TypeWithValidate   = (*regexpType)(nil)
-	_ basetypes.StringTypable  = (*regexpType)(nil)
-	_ basetypes.StringValuable = (*Regexp)(nil)
+	_ basetypes.StringTypable = (*regexpType)(nil)
 )
 
 type regexpType struct {
@@ -61,7 +57,7 @@ func (t regexpType) ValueFromString(_ context.Context, in types.String) (basetyp
 		return RegexpUnknown(), diags // Must not return validation errors.
 	}
 
-	return RegexpValueMust(valueString), diags
+	return RegexpValue(valueString), diags
 }
 
 func (t regexpType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) {
@@ -90,35 +86,10 @@ func (regexpType) ValueType(context.Context) attr.Value {
 	return Regexp{}
 }
 
-func (t regexpType) Validate(_ context.Context, in tftypes.Value, path path.Path) diag.Diagnostics {
-	var diags diag.Diagnostics
-
-	if !in.IsKnown() || in.IsNull() {
-		return diags
-	}
-
-	var value string
-	err := in.As(&value)
-	if err != nil {
-		diags.AddAttributeError(
-			path,
-			"Regexp Type Validation Error",
-			ProviderErrorDetailPrefix+fmt.Sprintf("Cannot convert value to string: %s", err),
-		)
-		return diags
-	}
-
-	if _, err := regexp.Compile(value); err != nil {
-		diags.AddAttributeError(
-			path,
-			"Regexp Type Validation Error",
-			fmt.Sprintf("Value %q cannot be parsed as a regular expression: %s", value, err),
-		)
-		return diags
-	}
-
-	return diags
-}
+var (
+	_ basetypes.StringValuable    = (*Regexp)(nil)
+	_ xattr.ValidateableAttribute = (*Regexp)(nil)
+)
 
 func RegexpNull() Regexp {
 	return Regexp{StringValue: basetypes.NewStringNull()}
@@ -128,10 +99,21 @@ func RegexpUnknown() Regexp {
 	return Regexp{StringValue: basetypes.NewStringUnknown()}
 }
 
-func RegexpValueMust(value string) Regexp {
+// RegexpValue initializes a new Regexp type with the provided value
+//
+// This function does not return diagnostics, and therefore invalid regular expression values
+// are not handled during construction. Invalid values will be detected by the
+// ValidateAttribute method, called by the ValidateResourceConfig RPC during
+// operations like `terraform validate`, `plan`, or `apply`.
+func RegexpValue(value string) Regexp {
+	// swallow any regex parsing errors here and just pass along the
+	// zero value regexp.Regexp. Invalid values will be handled downstream
+	// by the ValidateAttribute method.
+	v, _ := regexp.Compile(value)
+
 	return Regexp{
 		StringValue: basetypes.NewStringValue(value),
-		value:       regexache.MustCompile(value),
+		value:       v,
 	}
 }
 
@@ -157,3 +139,21 @@ func (Regexp) Type(context.Context) attr.Type {
 func (v Regexp) ValueRegexp() *regexp.Regexp {
 	return v.value
 }
+
+func (v Regexp) ValidateAttribute(ctx context.Context, req xattr.ValidateAttributeRequest, resp *xattr.ValidateAttributeResponse) {
+	if v.IsNull() || v.IsUnknown() {
+		return
+	}
+
+	vs := v.ValueString()
+	if _, err := regexp.Compile(vs); err != nil {
+		resp.Diagnostics.AddAttributeError(
+			req.Path,
+			"Invalid Regexp Value",
+			"The provided value cannot be parsed as a regular expression.\n\n"+
+				"Path: "+req.Path.String()+"\n"+
+				"Value: "+vs+"\n"+
+				"Error: "+err.Error(),
+		)
+	}
+}
diff --git a/internal/framework/types/regexp_test.go b/internal/framework/types/regexp_test.go
@@ -7,7 +7,7 @@ import (
 	"context"
 	"testing"
 
-	"github.com/hashicorp/terraform-plugin-framework/path"
+	"github.com/hashicorp/terraform-plugin-framework/attr/xattr"
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-go/tftypes"
 	fwtypes "github.com/hashicorp/terraform-provider-aws/internal/framework/types"
@@ -30,7 +30,7 @@ func TestRegexpTypeValueFromTerraform(t *testing.T) {
 		},
 		"valid Regexp": {
 			val:      tftypes.NewValue(tftypes.String, `\w+`),
-			expected: fwtypes.RegexpValueMust(`\w+`),
+			expected: fwtypes.RegexpValue(`\w+`),
 		},
 		"invalid Regexp": {
 			val:      tftypes.NewValue(tftypes.String, `(`),
@@ -57,29 +57,25 @@ func TestRegexpTypeValueFromTerraform(t *testing.T) {
 	}
 }
 
-func TestRegexpTypeValidate(t *testing.T) {
+func TestRegexpValidateAttribute(t *testing.T) {
 	t.Parallel()
 
 	type testCase struct {
-		val         tftypes.Value
+		val         fwtypes.Regexp
 		expectError bool
 	}
 	tests := map[string]testCase{
-		"not a string": {
-			val:         tftypes.NewValue(tftypes.Bool, true),
-			expectError: true,
-		},
-		"unknown string": {
-			val: tftypes.NewValue(tftypes.String, tftypes.UnknownValue),
+		"unknown": {
+			val: fwtypes.RegexpUnknown(),
 		},
-		"null string": {
-			val: tftypes.NewValue(tftypes.String, nil),
+		"null": {
+			val: fwtypes.RegexpNull(),
 		},
-		"valid string": {
-			val: tftypes.NewValue(tftypes.String, `\w+`),
+		"valid": {
+			val: fwtypes.RegexpValue(`\w+`),
 		},
-		"invalid string": {
-			val:         tftypes.NewValue(tftypes.String, `(`),
+		"invalid": {
+			val:         fwtypes.RegexpValue(`(`),
 			expectError: true,
 		},
 	}
@@ -91,14 +87,12 @@ func TestRegexpTypeValidate(t *testing.T) {
 
 			ctx := context.Background()
 
-			diags := fwtypes.RegexpType.Validate(ctx, test.val, path.Root("test"))
-
-			if !diags.HasError() && test.expectError {
-				t.Fatal("expected error, got no error")
-			}
+			req := xattr.ValidateAttributeRequest{}
+			resp := xattr.ValidateAttributeResponse{}
 
-			if diags.HasError() && !test.expectError {
-				t.Fatalf("got unexpected error: %#v", diags)
+			test.val.ValidateAttribute(ctx, req, &resp)
+			if resp.Diagnostics.HasError() != test.expectError {
+				t.Errorf("resp.Diagnostics.HasError() = %t, want = %t", resp.Diagnostics.HasError(), test.expectError)
 			}
 		})
 	}
@@ -112,7 +106,7 @@ func TestRegexpToStringValue(t *testing.T) {
 		expected types.String
 	}{
 		"value": {
-			regexp:   fwtypes.RegexpValueMust(`\w+`),
+			regexp:   fwtypes.RegexpValue(`\w+`),
 			expected: types.StringValue(`\w+`),
 		},
 		"null": {