diff --git a/templates/cc/register.go b/templates/cc/register.go index e92798577..050efda90 100644 --- a/templates/cc/register.go +++ b/templates/cc/register.go @@ -315,7 +315,7 @@ func (fns CCFuncs) inType(f pgs.Field, x interface{}) string { return fns.cTypeOfString(fns.Type(f).Value().String()) } - return fns.PackageName(fldEn).String() + "::" + fns.Type(f).Value().String() + return fns.packageName(fldEn) + "::" + fns.Type(f).Value().String() default: return fns.cType(f.Type()) } diff --git a/templates/cc/repeated.go b/templates/cc/repeated.go index 53b2a35f1..e7124f6f7 100644 --- a/templates/cc/repeated.go +++ b/templates/cc/repeated.go @@ -26,6 +26,9 @@ const repTpl = ` {{ end }} {{ if $r.GetUnique }} + {{ if isRepeatedEnum $f }} + std::unordered_set {{ lookup $f "Unique" }}; + {{ else }} // Implement comparison for wrapped reference types struct cmp { bool operator() (const std::reference_wrapper<{{ $typ }}> lhs, const std::reference_wrapper<{{ $typ }}> rhs) const { @@ -44,6 +47,7 @@ const repTpl = ` // Save a set of references to avoid copying overhead std::unordered_set, hash, cmp> {{ lookup $f "Unique" }}; {{ end }} + {{ end }} {{ if or $r.GetUnique (ne (.Elem "" "").Typ "none") }} for (int i = 0; i < {{ accessor . }}.size(); i++) { @@ -51,7 +55,11 @@ const repTpl = ` (void)item; {{ if $r.GetUnique }} + {{ if isRepeatedEnum $f }} + auto p = {{ lookup $f "Unique" }}.emplace(item); + {{ else }} auto p = {{ lookup $f "Unique" }}.emplace(const_cast<{{ $typ }}&>(item)); + {{ end }} if (p.second == false) { {{ errIdx . "i" "repeated value must contain unique items" }} } diff --git a/templates/goshared/register.go b/templates/goshared/register.go index 19a2a00ae..6a8fac73d 100644 --- a/templates/goshared/register.go +++ b/templates/goshared/register.go @@ -415,3 +415,29 @@ func (fns goSharedFuncs) enumPackages(enums []pgs.Enum) map[pgs.Name]NormalizedE func (fns goSharedFuncs) snakeCase(name string) string { return strcase.ToSnake(name) } + +func (fns goSharedFuncs) Type(f pgs.Field) pgsgo.TypeName { + if f.Type().ProtoType() == pgs.EnumT { + ens := fns.enumPackages(fns.externalEnums(f.File())) + // Check if the imported name of the enum has collided and been renamed + if len(ens) != 0 { + enType := f.Type().Enum() + if f.Type().IsRepeated() { + enType = f.Type().Element().Enum() + } + + enImportPath := fns.ImportPath(enType) + for pkg, en := range ens { + if en.FilePath == enImportPath { + name := fns.enumName(enType) + if f.Type().IsRepeated() { + return pgsgo.TypeName("[]" + pkg.String() + "." + name) + } + return pgsgo.TypeName(pkg.String() + "." + name) + } + } + } + } + + return fns.Context.Type(f) +} diff --git a/templates/shared/enums.go b/templates/shared/enums.go index bb5f053e7..79fcf39f4 100644 --- a/templates/shared/enums.go +++ b/templates/shared/enums.go @@ -11,6 +11,10 @@ func isEnum(f pgs.Field) bool { return f.Type().IsEnum() } +func isRepeatedEnum(f pgs.Field) bool { + return f.Type().IsRepeated() && f.Type().Element().IsEnum() +} + func enumNamesMap(values []pgs.EnumValue) (m map[int32]string) { m = make(map[int32]string) for _, v := range values { diff --git a/templates/shared/functions.go b/templates/shared/functions.go index eb10045d8..8ed5cd68b 100644 --- a/templates/shared/functions.go +++ b/templates/shared/functions.go @@ -8,16 +8,17 @@ import ( func RegisterFunctions(tpl *template.Template, params pgs.Parameters) { tpl.Funcs(map[string]interface{}{ - "disabled": Disabled, - "ignored": Ignored, - "required": RequiredOneOf, - "context": rulesContext, - "render": Render(tpl), - "has": Has, - "needs": Needs, - "fileneeds": FileNeeds, - "isEnum": isEnum, - "enumList": enumList, - "enumVal": enumVal, + "disabled": Disabled, + "ignored": Ignored, + "required": RequiredOneOf, + "context": rulesContext, + "render": Render(tpl), + "has": Has, + "needs": Needs, + "fileneeds": FileNeeds, + "isEnum": isEnum, + "isRepeatedEnum": isRepeatedEnum, + "enumList": enumList, + "enumVal": enumVal, }) } diff --git a/tests/harness/cases/BUILD b/tests/harness/cases/BUILD index 514d293c0..d557a5e03 100644 --- a/tests/harness/cases/BUILD +++ b/tests/harness/cases/BUILD @@ -34,6 +34,7 @@ proto_library( visibility = ["//visibility:public"], deps = [ "//tests/harness/cases/other_package:embed_proto", + "//tests/harness/cases/other_package/nested:enum_proto", "//tests/harness/cases/sort:sort_proto", "//tests/harness/cases/yet_another_package:embed_proto", "//validate:validate_proto", @@ -50,6 +51,7 @@ pgv_go_proto_library( proto = ":cases_proto", deps = [ "//tests/harness/cases/other_package:go", + "//tests/harness/cases/other_package/nested:go", "//tests/harness/cases/sort:go", "//tests/harness/cases/yet_another_package:go", "@org_golang_google_protobuf//types/known/anypb:go_default_library", @@ -63,6 +65,7 @@ pgv_cc_proto_library( name = "cc", cc_deps = [ "//tests/harness/cases/other_package:cc", + "//tests/harness/cases/other_package/nested:cc", "//tests/harness/cases/sort:cc", "//tests/harness/cases/yet_another_package:cc", ], @@ -81,6 +84,7 @@ pgv_java_proto_library( java_deps = [ ":cases_java_proto", "//tests/harness/cases/other_package:java", + "//tests/harness/cases/other_package/nested:java", "//tests/harness/cases/yet_another_package:java", "//tests/harness/cases/sort:java", ], diff --git a/tests/harness/cases/enums.proto b/tests/harness/cases/enums.proto index 97c1ed0fd..14f404365 100644 --- a/tests/harness/cases/enums.proto +++ b/tests/harness/cases/enums.proto @@ -4,6 +4,7 @@ package tests.harness.cases; option go_package = "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/go;cases"; import "validate/validate.proto"; import "tests/harness/cases/other_package/embed.proto"; +import "tests/harness/cases/other_package/nested/enum.proto"; import "tests/harness/cases/yet_another_package/embed.proto"; import "tests/harness/cases/sort/sort.proto"; @@ -51,11 +52,16 @@ message EnumExternal4 { 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 RepeatedExternalEnumMultiLevelDefined { repeated other_package.nested.DeepEnum val = 1 [(validate.rules).repeated.items.enum.defined_only = true]; } message RepeatedYetAnotherExternalEnumDefined { repeated yet_another_package.Embed.Enumerated val = 1 [(validate.rules).repeated.items.enum.defined_only = true]; } message RepeatedEnumExternal { repeated other_package.Embed.FooNumber foo = 1 [(validate.rules).repeated.items.enum = { in: [0, 2] }]; repeated yet_another_package.Embed.BarNumber bar = 2 [(validate.rules).repeated.items.enum = { not_in: [1] }]; } +message RepeatedExternalEnumMultiLevel { + repeated other_package.nested.DeepEnum foo = 1 [(validate.rules).repeated.items.enum = { in: [1] }]; + repeated other_package.nested.DeepEnum bar = 2 [(validate.rules).repeated.items.enum = { not_in: [1] }]; +} message MapEnumDefined { map val = 1 [(validate.rules).map.values.enum.defined_only = true]; } message MapExternalEnumDefined { map val = 1 [(validate.rules).map.values.enum.defined_only = true]; } diff --git a/tests/harness/cases/other_package/nested/BUILD b/tests/harness/cases/other_package/nested/BUILD new file mode 100644 index 000000000..de40dd2d5 --- /dev/null +++ b/tests/harness/cases/other_package/nested/BUILD @@ -0,0 +1,48 @@ +load("@com_google_protobuf//bazel:proto_library.bzl", "proto_library") +load("@com_google_protobuf//bazel:py_proto_library.bzl", "py_proto_library") +load("@rules_java//java:defs.bzl", "java_proto_library") +load( + "//bazel:pgv_proto_library.bzl", + "pgv_cc_proto_library", + "pgv_go_proto_library", + "pgv_java_proto_library", +) + +proto_library( + name = "enum_proto", + srcs = [ + "enum.proto", + ], + visibility = ["//visibility:public"], +) + +pgv_go_proto_library( + name = "go", + importpath = "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/other_package/nested/go", + proto = ":enum_proto", +) + +pgv_cc_proto_library( + name = "cc", + visibility = ["//tests:__subpackages__"], + deps = [":enum_proto"], +) + +java_proto_library( + name = "enum_java_proto", + visibility = ["//visibility:public"], + deps = [":enum_proto"], +) + +pgv_java_proto_library( + name = "java", + java_deps = [":enum_java_proto"], + visibility = ["//visibility:public"], + deps = [":enum_proto"], +) + +py_proto_library( + name = "enum_python_proto", + visibility = ["//visibility:public"], + deps = [":enum_proto"], +) diff --git a/tests/harness/cases/other_package/nested/enum.proto b/tests/harness/cases/other_package/nested/enum.proto new file mode 100644 index 000000000..6a5fc1bdd --- /dev/null +++ b/tests/harness/cases/other_package/nested/enum.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +package tests.harness.cases.other_package.nested; +option go_package = "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/other_package/nested/go;nested"; + +enum DeepEnum { + DEEP_UNSPECIFIED = 0; + DEEP_ONE = 1; +} diff --git a/tests/harness/cases/repeated.proto b/tests/harness/cases/repeated.proto index c3ac792f9..7c6e6a0a7 100644 --- a/tests/harness/cases/repeated.proto +++ b/tests/harness/cases/repeated.proto @@ -22,6 +22,7 @@ message RepeatedMax { repeated double val = 1 [(validate.rules).repeated.m message RepeatedMinMax { repeated sfixed32 val = 1 [(validate.rules).repeated = {min_items: 2, max_items: 4}]; } message RepeatedExact { repeated uint32 val = 1 [(validate.rules).repeated = {min_items: 3, max_items: 3}]; } message RepeatedUnique { repeated string val = 1 [(validate.rules).repeated.unique = true]; } +message RepeatedUniqueEnum { repeated AnEnum val = 1 [(validate.rules).repeated.unique = true]; } message RepeatedItemRule { repeated float val = 1 [(validate.rules).repeated.items.float.gt = 0]; } message RepeatedItemPattern { repeated string val = 1 [(validate.rules).repeated.items.string.pattern = "(?i)^[a-z0-9]+$"]; } message RepeatedEmbedSkip { repeated Embed val = 1 [(validate.rules).repeated.items.message.skip = true]; } diff --git a/tests/harness/executor/BUILD b/tests/harness/executor/BUILD index 9922814c4..a10709a72 100644 --- a/tests/harness/executor/BUILD +++ b/tests/harness/executor/BUILD @@ -19,6 +19,7 @@ go_library( "//tests/harness:harness_go_proto", "//tests/harness/cases:go", "//tests/harness/cases/other_package:go", + "//tests/harness/cases/other_package/nested:go", "//tests/harness/cases/sort:go", "//tests/harness/cases/yet_another_package:go", "@org_golang_google_protobuf//proto:go_default_library", diff --git a/tests/harness/executor/cases.go b/tests/harness/executor/cases.go index c8eb26583..f22c65dcc 100644 --- a/tests/harness/executor/cases.go +++ b/tests/harness/executor/cases.go @@ -6,6 +6,7 @@ import ( cases "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/go" other_package "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/other_package/go" + nested "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/other_package/nested/go" sort "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/sort/go" yet_another_package "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/yet_another_package/go" "google.golang.org/protobuf/proto" @@ -1035,12 +1036,19 @@ var enumCases = []TestCase{ {"enum repeated (external) - defined_only - valid", &cases.RepeatedExternalEnumDefined{Val: []other_package.Embed_Enumerated{other_package.Embed_VALUE}}, 0}, {"enum repeated (external) - defined_only - invalid", &cases.RepeatedExternalEnumDefined{Val: []other_package.Embed_Enumerated{math.MaxInt32}}, 1}, + {"enum repeated (external multilevel) - defined_only - valid", &cases.RepeatedExternalEnumMultiLevelDefined{Val: []nested.DeepEnum{nested.DeepEnum_DEEP_ONE}}, 0}, + {"enum repeated (external multilevel) - defined_only - invalid", &cases.RepeatedExternalEnumMultiLevelDefined{Val: []nested.DeepEnum{math.MaxInt32}}, 1}, + {"enum repeated (another external) - defined_only - valid", &cases.RepeatedYetAnotherExternalEnumDefined{Val: []yet_another_package.Embed_Enumerated{yet_another_package.Embed_VALUE}}, 0}, {"enum repeated (external) - in - valid", &cases.RepeatedEnumExternal{Foo: []other_package.Embed_FooNumber{other_package.Embed_ZERO, other_package.Embed_TWO}}, 0}, {"enum repeated (external) - in - invalid", &cases.RepeatedEnumExternal{Foo: []other_package.Embed_FooNumber{other_package.Embed_ONE}}, 1}, {"enum repeated (external) - not in - valid", &cases.RepeatedEnumExternal{Bar: []yet_another_package.Embed_BarNumber{yet_another_package.Embed_ZERO, yet_another_package.Embed_TWO}}, 0}, {"enum repeated (external) - not in - invalid", &cases.RepeatedEnumExternal{Bar: []yet_another_package.Embed_BarNumber{yet_another_package.Embed_ONE}}, 1}, + {"enum repeated (external multilevel) - in - valid", &cases.RepeatedExternalEnumMultiLevel{Foo: []nested.DeepEnum{nested.DeepEnum_DEEP_ONE}}, 0}, + {"enum repeated (external multilevel) - in - invalid", &cases.RepeatedExternalEnumMultiLevel{Foo: []nested.DeepEnum{nested.DeepEnum_DEEP_UNSPECIFIED}}, 1}, + {"enum repeated (external multilevel) - not in - valid", &cases.RepeatedExternalEnumMultiLevel{Bar: []nested.DeepEnum{nested.DeepEnum_DEEP_UNSPECIFIED}}, 0}, + {"enum repeated (external multilevel) - not in - invalid", &cases.RepeatedExternalEnumMultiLevel{Bar: []nested.DeepEnum{nested.DeepEnum_DEEP_ONE}}, 1}, {"enum map - defined_only - valid", &cases.MapEnumDefined{Val: map[string]cases.TestEnum{"foo": cases.TestEnum_TWO}}, 0}, {"enum map - defined_only - invalid", &cases.MapEnumDefined{Val: map[string]cases.TestEnum{"foo": math.MaxInt32}}, 1}, @@ -1116,6 +1124,9 @@ var repeatedCases = []TestCase{ {"repeated - unique - valid (empty)", &cases.RepeatedUnique{}, 0}, {"repeated - unique - valid (case sensitivity)", &cases.RepeatedUnique{Val: []string{"foo", "Foo"}}, 0}, {"repeated - unique - invalid", &cases.RepeatedUnique{Val: []string{"foo", "bar", "foo", "baz"}}, 1}, + {"repeated - unique enum - valid", &cases.RepeatedUniqueEnum{Val: []cases.AnEnum{cases.AnEnum_X, cases.AnEnum_Y}}, 0}, + {"repeated - unique enum - valid (empty)", &cases.RepeatedUniqueEnum{}, 0}, + {"repeated - unique enum - invalid", &cases.RepeatedUniqueEnum{Val: []cases.AnEnum{cases.AnEnum_X, cases.AnEnum_X}}, 1}, {"repeated - items - valid", &cases.RepeatedItemRule{Val: []float32{1, 2, 3}}, 0}, {"repeated - items - valid (empty)", &cases.RepeatedItemRule{Val: []float32{}}, 0},