Properly handle repeated enums (#140)

Author: rodaine

templates/cc/enum.go

@@ -6,7 +6,11 @@ const enumTpl = `
 		{{ template "in" . }}
 
 		{{ if $r.GetDefinedOnly }}
-			if (!{{ package $f.Type.Enum }}::{{ (typ $f).Element }}_IsValid({{ accessor . }})) {
+			{{ if $f.Type.IsRepeated }}
+				if (!{{ class $f.Type.Element.Enum }}_IsValid({{ accessor . }})) {
+			{{ else }}
+				if (!{{ class $f.Type.Enum }}_IsValid({{ accessor . }})) {
+			{{ end }}
 				{{ err . "value must be one of the defined enum values" }}
 			}
 		{{ end }}

templates/cc/register.go

@@ -10,8 +10,8 @@ import (
 	"github.com/golang/protobuf/ptypes"
 	"github.com/golang/protobuf/ptypes/duration"
 	"github.com/golang/protobuf/ptypes/timestamp"
-	"github.com/lyft/protoc-gen-star"
-	"github.com/lyft/protoc-gen-star/lang/go"
+	pgs "github.com/lyft/protoc-gen-star"
+	pgsgo "github.com/lyft/protoc-gen-star/lang/go"
 	"github.com/lyft/protoc-gen-validate/templates/shared"
 )
 
@@ -143,15 +143,20 @@ func (fns CCFuncs) hasAccessor(ctx shared.RuleContext) string {
 		fns.methodName(ctx.Field.Name()))
 }
 
-func (fns CCFuncs) classBaseName(msg pgs.Message) string {
-	if m, ok := msg.Parent().(pgs.Message); ok {
-		return fmt.Sprintf("%s_%s", fns.classBaseName(m), msg.Name().String())
+type childEntity interface {
+	pgs.Entity
+	Parent() pgs.ParentEntity
+}
+
+func (fns CCFuncs) classBaseName(ent childEntity) string {
+	if m, ok := ent.Parent().(pgs.Message); ok {
+		return fmt.Sprintf("%s_%s", fns.classBaseName(m), ent.Name().String())
 	}
-	return msg.Name().String()
+	return ent.Name().String()
 }
 
-func (fns CCFuncs) className(msg pgs.Message) string {
-	return fns.packageName(msg) + "::" + fns.classBaseName(msg)
+func (fns CCFuncs) className(ent childEntity) string {
+	return fns.packageName(ent) + "::" + fns.classBaseName(ent)
 }
 
 func (fns CCFuncs) packageName(msg pgs.Entity) string {

templates/cc/repeated.go

@@ -45,7 +45,7 @@ const repTpl = `
 
 	{{ if or $r.GetUnique (ne (.Elem "" "").Typ "none") }}
 		for (int i = 0; i < {{ accessor . }}.size(); i++) {
-			const {{ $typ }}& item = {{ accessor . }}.Get(i);
+			const auto& item = {{ accessor . }}.Get(i);
 			(void)item;
 
 			{{ if $r.GetUnique }}

tests/harness/cases/enums.proto

@@ -39,3 +39,9 @@ message EnumNotIn      { TestEnum val = 1 [(validate.rules).enum = { not_in: [1]
 message EnumAliasNotIn { TestEnumAlias val = 1 [(validate.rules).enum = { not_in: [1]}]; }
 
 message EnumExternal { other_package.Embed.Enumerated val = 1 [(validate.rules).enum.defined_only = true]; }
+
+message RepeatedEnumDefined { repeated TestEnum val = 1 [(validate.rules).repeated.items.enum.defined_only = true]; }
+message RepeatedExternalEnumDefined { repeated other_package.Embed.Enumerated val = 1 [(validate.rules).repeated.items.enum.defined_only = true]; }
+
+message MapEnumDefined { map<string, TestEnum> val = 1 [(validate.rules).map.values.enum.defined_only = true]; }
+message MapExternalEnumDefined { map<string, other_package.Embed.Enumerated> val = 1 [(validate.rules).map.values.enum.defined_only = true]; }

tests/harness/executor/cases.go

@@ -917,6 +917,18 @@ var enumCases = []TestCase{
 
 	{"enum external - defined_only - valid", &cases.EnumExternal{Val: other_package.Embed_VALUE}, true},
 	{"enum external - defined_only - invalid", &cases.EnumExternal{Val: math.MaxInt32}, false},
+
+	{"enum repeated - defined_only - valid", &cases.RepeatedEnumDefined{Val: []cases.TestEnum{cases.TestEnum_ONE, cases.TestEnum_TWO}}, true},
+	{"enum repeated - defined_only - invalid", &cases.RepeatedEnumDefined{Val: []cases.TestEnum{cases.TestEnum_ONE, math.MaxInt32}}, false},
+
+	{"enum repeated (external) - defined_only - valid", &cases.RepeatedExternalEnumDefined{Val: []other_package.Embed_Enumerated{other_package.Embed_VALUE}}, true},
+	{"enum repeated (external) - defined_only - invalid", &cases.RepeatedExternalEnumDefined{Val: []other_package.Embed_Enumerated{math.MaxInt32}}, false},
+
+	{"enum map - defined_only - valid", &cases.MapEnumDefined{Val: map[string]cases.TestEnum{"foo": cases.TestEnum_TWO}}, true},
+	{"enum map - defined_only - invalid", &cases.MapEnumDefined{Val: map[string]cases.TestEnum{"foo": math.MaxInt32}}, false},
+
+	{"enum map (external) - defined_only - valid", &cases.MapExternalEnumDefined{Val: map[string]other_package.Embed_Enumerated{"foo": other_package.Embed_VALUE}}, true},
+	{"enum map (external) - defined_only - invalid", &cases.MapExternalEnumDefined{Val: map[string]other_package.Embed_Enumerated{"foo": math.MaxInt32}}, false},
 }
 
 var messageCases = []TestCase{