helper/schema: move InternalValidate to schemaMap

helper/schema/resource.go

@@ -2,7 +2,6 @@ package schema
 
 import (
 	"errors"
-	"fmt"
 
 	"github.com/hashicorp/terraform/terraform"
 )
@@ -53,42 +52,5 @@ func (r *Resource) InternalValidate() error {
 		return errors.New("resource is nil")
 	}
 
-	for k, v := range r.Schema {
+	return schemaMap(r.Schema).InternalValidate()
-		if v.Type == TypeInvalid {
-			return fmt.Errorf("%s: Type must be specified", k)
-		}
-
-		if v.Optional && v.Required {
-			return fmt.Errorf("%s: Optional or Required must be set, not both", k)
-		}
-
-		if v.Required && v.Computed {
-			return fmt.Errorf("%s: Cannot be both Required and Computed", k)
-		}
-
-		if len(v.ComputedWhen) > 0 && !v.Computed {
-			return fmt.Errorf("%s: ComputedWhen can only be set with Computed", k)
-		}
-
-		if v.Type == TypeList {
-			if v.Elem == nil {
-				return fmt.Errorf("%s: Elem must be set for lists", k)
-			}
-
-			switch t := v.Elem.(type) {
-			case *Resource:
-				if err := t.InternalValidate(); err != nil {
-					return err
-				}
-			case *Schema:
-				bad := t.Computed || t.Optional || t.Required
-				if bad {
-					return fmt.Errorf(
-						"%s: Elem must have only Type set", k)
-				}
-			}
-		}
-	}
-
-	return nil
 }

helper/schema/resource_test.go

@@ -27,123 +27,6 @@ func TestResourceInternalValidate(t *testing.T) {
 			},
 			true,
 		},
-
-		// Missing Type
-		{
-			&Resource{
-				Schema: map[string]*Schema{
-					"foo": &Schema{
-						Required: true,
-					},
-				},
-			},
-			true,
-		},
-
-		// Required but computed
-		{
-			&Resource{
-				Schema: map[string]*Schema{
-					"foo": &Schema{
-						Type:     TypeInt,
-						Required: true,
-						Computed: true,
-					},
-				},
-			},
-			true,
-		},
-
-		// Looks good
-		{
-			&Resource{
-				Schema: map[string]*Schema{
-					"foo": &Schema{
-						Type:     TypeString,
-						Required: true,
-					},
-				},
-			},
-			false,
-		},
-
-		// List element not set
-		{
-			&Resource{
-				Schema: map[string]*Schema{
-					"foo": &Schema{
-						Type: TypeList,
-					},
-				},
-			},
-			true,
-		},
-
-		// List element computed
-		{
-			&Resource{
-				Schema: map[string]*Schema{
-					"foo": &Schema{
-						Type: TypeList,
-						Elem: &Schema{
-							Type:     TypeInt,
-							Computed: true,
-						},
-					},
-				},
-			},
-			true,
-		},
-
-		// Required but computed
-		{
-			&Resource{
-				Schema: map[string]*Schema{
-					"foo": &Schema{
-						Type:         TypeInt,
-						Required:     true,
-						ComputedWhen: []string{"foo"},
-					},
-				},
-			},
-			true,
-		},
-
-		// Sub-resource invalid
-		{
-			&Resource{
-				Schema: map[string]*Schema{
-					"foo": &Schema{
-						Type: TypeList,
-						Elem: &Resource{
-							Schema: map[string]*Schema{
-								"foo": new(Schema),
-							},
-						},
-					},
-				},
-			},
-			true,
-		},
-
-		// Sub-resource valid
-		{
-			&Resource{
-				Schema: map[string]*Schema{
-					"foo": &Schema{
-						Type: TypeList,
-						Elem: &Resource{
-							Schema: map[string]*Schema{
-								"foo": &Schema{
-									Type: TypeInt,
-								},
-							},
-						},
-					},
-				},
-			},
-			false,
-		},
 	}
 
 	for i, tc := range cases {

helper/schema/schema.go

@@ -136,6 +136,50 @@ func (m schemaMap) Validate(c *terraform.ResourceConfig) ([]string, []error) {
 	return m.validateObject("", m, c)
 }
 
+// InternalValidate validates the format of this schema. This should be called
+// from a unit test (and not in user-path code) to verify that a schema
+// is properly built.
+func (m schemaMap) InternalValidate() error {
+	for k, v := range m {
+		if v.Type == TypeInvalid {
+			return fmt.Errorf("%s: Type must be specified", k)
+		}
+
+		if v.Optional && v.Required {
+			return fmt.Errorf("%s: Optional or Required must be set, not both", k)
+		}
+
+		if v.Required && v.Computed {
+			return fmt.Errorf("%s: Cannot be both Required and Computed", k)
+		}
+
+		if len(v.ComputedWhen) > 0 && !v.Computed {
+			return fmt.Errorf("%s: ComputedWhen can only be set with Computed", k)
+		}
+
+		if v.Type == TypeList {
+			if v.Elem == nil {
+				return fmt.Errorf("%s: Elem must be set for lists", k)
+			}
+
+			switch t := v.Elem.(type) {
+			case *Resource:
+				if err := t.InternalValidate(); err != nil {
+					return err
+				}
+			case *Schema:
+				bad := t.Computed || t.Optional || t.Required
+				if bad {
+					return fmt.Errorf(
+						"%s: Elem must have only Type set", k)
+				}
+			}
+		}
+	}
+
+	return nil
+}
+
 func (m schemaMap) diff(
 	k string,
 	schema *Schema,

helper/schema/schema_test.go

@@ -568,6 +568,139 @@ func TestSchemaMap_Diff(t *testing.T) {
 	}
 }
 
+func TestSchemaMap_InternalValidate(t *testing.T) {
+	cases := []struct {
+		In  map[string]*Schema
+		Err bool
+	}{
+		{
+			nil,
+			false,
+		},
+
+		// No optional and no required
+		{
+			map[string]*Schema{
+				"foo": &Schema{
+					Type:     TypeInt,
+					Optional: true,
+					Required: true,
+				},
+			},
+			true,
+		},
+
+		// Missing Type
+		{
+			map[string]*Schema{
+				"foo": &Schema{
+					Required: true,
+				},
+			},
+			true,
+		},
+
+		// Required but computed
+		{
+			map[string]*Schema{
+				"foo": &Schema{
+					Type:     TypeInt,
+					Required: true,
+					Computed: true,
+				},
+			},
+			true,
+		},
+
+		// Looks good
+		{
+			map[string]*Schema{
+				"foo": &Schema{
+					Type:     TypeString,
+					Required: true,
+				},
+			},
+			false,
+		},
+
+		// List element not set
+		{
+			map[string]*Schema{
+				"foo": &Schema{
+					Type: TypeList,
+				},
+			},
+			true,
+		},
+
+		// List element computed
+		{
+			map[string]*Schema{
+				"foo": &Schema{
+					Type: TypeList,
+					Elem: &Schema{
+						Type:     TypeInt,
+						Computed: true,
+					},
+				},
+			},
+			true,
+		},
+
+		// Required but computed
+		{
+			map[string]*Schema{
+				"foo": &Schema{
+					Type:         TypeInt,
+					Required:     true,
+					ComputedWhen: []string{"foo"},
+				},
+			},
+			true,
+		},
+
+		// Sub-resource invalid
+		{
+			map[string]*Schema{
+				"foo": &Schema{
+					Type: TypeList,
+					Elem: &Resource{
+						Schema: map[string]*Schema{
+							"foo": new(Schema),
+						},
+					},
+				},
+			},
+			true,
+		},
+
+		// Sub-resource valid
+		{
+			map[string]*Schema{
+				"foo": &Schema{
+					Type: TypeList,
+					Elem: &Resource{
+						Schema: map[string]*Schema{
+							"foo": &Schema{
+								Type: TypeInt,
+							},
+						},
+					},
+				},
+			},
+			false,
+		},
+	}
+
+	for i, tc := range cases {
+		err := schemaMap(tc.In).InternalValidate()
+		if (err != nil) != tc.Err {
+			t.Fatalf("%d: bad: %s", i, err)
+		}
+	}
+
+}
+
 func TestSchemaMap_Validate(t *testing.T) {
 	cases := []struct {
 		Schema map[string]*Schema