diff --git a/BUILD b/BUILD index 1354b5d3f..4d9ed6c82 100644 --- a/BUILD +++ b/BUILD @@ -22,6 +22,7 @@ go_library( visibility = ["//visibility:private"], deps = [ "//module", + "@org_golang_google_protobuf//types/pluginpb", "@com_github_lyft_protoc_gen_star//:protoc-gen-star", "@com_github_lyft_protoc_gen_star//lang/go", ], diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 68acc1a00..d571c9c66 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -28,9 +28,9 @@ def pgv_dependencies(maven_repos = _DEFAULT_REPOSITORIES): if not native.existing_rule("com_google_protobuf"): http_archive( name = "com_google_protobuf", - url = "https://github.com/protocolbuffers/protobuf/archive/v3.14.0.tar.gz", - sha256 = "d0f5f605d0d656007ce6c8b5a82df3037e1d8fe8b121ed42e536f569dec16113", - strip_prefix = "protobuf-3.14.0", + url = "https://github.com/protocolbuffers/protobuf/archive/v3.15.3.tar.gz", + sha256 = "b10bf4e2d1a7586f54e64a5d9e7837e5188fc75ae69e36f215eb01def4f9721b", + strip_prefix = "protobuf-3.15.3", ) # TODO(akonradi): This shouldn't be necesary since the same http_archive block is imported by diff --git a/dependencies.bzl b/dependencies.bzl index ec46a298c..5b2443dd6 100644 --- a/dependencies.bzl +++ b/dependencies.bzl @@ -35,8 +35,8 @@ def go_third_party(): go_repository( name = "com_github_lyft_protoc_gen_star", importpath = "github.com/lyft/protoc-gen-star", - sum = "h1:zSGLzsUew8RT+ZKPHc3jnf8XLaVyHzTcAFBzHtCNR20=", - version = "v0.5.3", + sum = "h1:xOpFu4vwmIoUeUrRuAtdCrZZymT/6AkW/bsUWA506Fo=", + version = "v0.6.0", ) go_repository( name = "com_github_pkg_errors", @@ -105,7 +105,7 @@ def go_third_party(): name = "org_golang_google_protobuf", importpath = "google.golang.org/protobuf", sum = "h1:bxAC2xTBsZGibn2RTntX0oH50xLsqy1OxA9tTL3p/lk=", - version = "v1.26.0", + version = "v1.27.1", ) go_repository( name = "org_golang_x_crypto", diff --git a/go.mod b/go.mod index 504d1099f..53e9f1141 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.12 require ( github.com/iancoleman/strcase v0.2.0 - github.com/lyft/protoc-gen-star v0.5.3 + github.com/lyft/protoc-gen-star v0.6.0 github.com/spf13/afero v1.6.0 // indirect golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 golang.org/x/mod v0.5.0 // indirect diff --git a/go.sum b/go.sum index 2058f261e..f601b0d1d 100644 --- a/go.sum +++ b/go.sum @@ -8,8 +8,8 @@ github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/iancoleman/strcase v0.2.0 h1:05I4QRnGpI0m37iZQRuskXh+w77mr6Z41lwQzuHLwW0= github.com/iancoleman/strcase v0.2.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho= github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= -github.com/lyft/protoc-gen-star v0.5.3 h1:zSGLzsUew8RT+ZKPHc3jnf8XLaVyHzTcAFBzHtCNR20= -github.com/lyft/protoc-gen-star v0.5.3/go.mod h1:V0xaHgaf5oCCqmcxYcWiDfTiKsZsRc87/1qhoTACD8w= +github.com/lyft/protoc-gen-star v0.6.0 h1:xOpFu4vwmIoUeUrRuAtdCrZZymT/6AkW/bsUWA506Fo= +github.com/lyft/protoc-gen-star v0.6.0/go.mod h1:TGAoBVkt8w7MPG72TrKIu85MIdXwDuzJYeZuUPFPNwA= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/sftp v1.10.1/go.mod h1:lYOWFsE0bwd1+KfKJaKeuokY15vzFx25BLbzYYoAxZI= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= diff --git a/main.go b/main.go index 5483be6fa..27f4ce0e1 100644 --- a/main.go +++ b/main.go @@ -1,14 +1,16 @@ package main import ( + "github.com/envoyproxy/protoc-gen-validate/module" "github.com/lyft/protoc-gen-star" "github.com/lyft/protoc-gen-star/lang/go" - "github.com/envoyproxy/protoc-gen-validate/module" + "google.golang.org/protobuf/types/pluginpb" ) func main() { + optional := uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL) pgs. - Init(pgs.DebugEnv("DEBUG_PGV")). + Init(pgs.DebugEnv("DEBUG_PGV"), pgs.SupportedFeatures(&optional)). RegisterModule(module.Validator()). RegisterPostProcessor(pgsgo.GoFmt()). Render() diff --git a/templates/cc/msg.go b/templates/cc/msg.go index a54450ea5..bd8ad2d86 100644 --- a/templates/cc/msg.go +++ b/templates/cc/msg.go @@ -102,7 +102,7 @@ bool Validate(const {{ class . }}& m, pgv::ValidationMsg* err) { {{ range .NonOneOfFields }} {{- render (context .) -}} {{ end -}} - {{ range .OneOfs }} + {{ range .RealOneOfs }} switch (m.{{ .Name }}_case()) { {{ range .Fields -}} case {{ oneof . }}: diff --git a/templates/goshared/msg.go b/templates/goshared/msg.go index 4e09f7770..6ecd991c7 100644 --- a/templates/goshared/msg.go +++ b/templates/goshared/msg.go @@ -33,7 +33,7 @@ func (m {{ (msgTyp .).Pointer }}) validate(all bool) error { {{ render (context .) }} {{ end }} - {{ range .OneOfs }} + {{ range .RealOneOfs }} switch m.{{ name . }}.(type) { {{ range .Fields }} case {{ oneof . }}: @@ -51,9 +51,16 @@ func (m {{ (msgTyp .).Pointer }}) validate(all bool) error { } {{ end }} + {{ range .SyntheticOneOfFields }} + if m.{{ name . }} != nil { + {{ render (context .) }} + } + {{ end }} + if len(errors) > 0 { return {{ multierrname . }}(errors) } + return nil {{ end -}} } diff --git a/templates/java/msg.go b/templates/java/msg.go index 26ba40ad8..0c58760da 100644 --- a/templates/java/msg.go +++ b/templates/java/msg.go @@ -15,7 +15,7 @@ const msgInnerTpl = ` {{- range .NonOneOfFields }} {{ renderConstants (context .) }} {{ end }} - {{ range .OneOfs }} + {{ range .RealOneOfs }} {{ template "oneOfConst" . }} {{ end }} @@ -27,7 +27,7 @@ const msgInnerTpl = ` {{ range .NonOneOfFields -}} {{ render (context .) }} {{ end -}} - {{ range .OneOfs }} + {{ range .RealOneOfs }} {{ template "oneOf" . }} {{- end -}} {{- end }} diff --git a/tests/harness/cases/messages.proto b/tests/harness/cases/messages.proto index b748b150a..c56dfb1f6 100644 --- a/tests/harness/cases/messages.proto +++ b/tests/harness/cases/messages.proto @@ -30,6 +30,7 @@ message Message { TestMsg val = 1; } message MessageCrossPackage { tests.harness.cases.other_package.Embed val = 1; } message MessageSkip { TestMsg val = 1 [(validate.rules).message.skip = true];} message MessageRequired { TestMsg val = 1 [(validate.rules).message.required = true]; } +message MessageRequiredButOptional { optional TestMsg val = 1 [(validate.rules).message.required = true]; } message MessageRequiredOneof { oneof one { diff --git a/tests/harness/cases/numbers.proto b/tests/harness/cases/numbers.proto index ccbb57ae0..c74f34f9c 100644 --- a/tests/harness/cases/numbers.proto +++ b/tests/harness/cases/numbers.proto @@ -171,3 +171,5 @@ message SFixed64ExLTGT { sfixed64 val = 1 [(validate.rules).sfixed64 = {lt: 0, message SFixed64GTELTE { sfixed64 val = 1 [(validate.rules).sfixed64 = {gte: 128, lte: 256}]; } message SFixed64ExGTELTE { sfixed64 val = 1 [(validate.rules).sfixed64 = {lte: 128, gte: 256}]; } message SFixed64Ignore { sfixed64 val = 1 [(validate.rules).sfixed64 = {lte: 128, gte: 256, ignore_empty: true}]; } + +message Int64LTEOptional { optional int64 val = 1 [(validate.rules).int64.lte = 64]; } diff --git a/tests/harness/executor/cases.go b/tests/harness/executor/cases.go index 0931a01a4..8f98f3b13 100644 --- a/tests/harness/executor/cases.go +++ b/tests/harness/executor/cases.go @@ -279,6 +279,10 @@ var int64Cases = []TestCase{ {"int64 - exclusive gte & lte - invalid", &cases.Int64ExGTELTE{Val: 200}, 1}, {"int64 - ignore_empty gte & lte - valid", &cases.Int64Ignore{Val: 0}, 0}, + + {"int64 optional - lte - valid", &cases.Int64LTEOptional{Val: &wrapperspb.Int64(63).Value}, 0}, + {"int64 optional - lte - valid (equal)", &cases.Int64LTEOptional{Val: &wrapperspb.Int64(64).Value}, 0}, + {"int64 optional - lte - valid (unset)", &cases.Int64LTEOptional{}, 0}, } var uint32Cases = []TestCase{ @@ -1059,6 +1063,9 @@ var messageCases = []TestCase{ {"message - cross-package embed none - valid (nil)", &cases.MessageCrossPackage{}, 0}, {"message - cross-package embed none - valid (empty)", &cases.MessageCrossPackage{Val: &other_package.Embed{}}, 1}, {"message - cross-package embed none - invalid", &cases.MessageCrossPackage{Val: &other_package.Embed{Val: -1}}, 1}, + + {"message - required - valid", &cases.MessageRequiredButOptional{Val: &cases.TestMsg{Const: "foo"}}, 0}, + {"message - required - valid (unset)", &cases.MessageRequiredButOptional{}, 0}, } var repeatedCases = []TestCase{