From b032681f2001c441cc8a57bd39676aae6ae441ac Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Mon, 27 Jan 2025 18:45:15 -0500 Subject: [PATCH 1/2] Remove support for deprecated protovalidate options --- .golangci.yml | 8 ----- .../internal/buflintvalidate/field.go | 29 ------------------- 2 files changed, 37 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a25e20c79e..349ec328c3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -183,14 +183,6 @@ issues: - gochecknoinits # we actually want to use this init to create a protovalidate.Validator path: private/bufpkg/bufcas/proto.go - - linters: - - staticcheck - text: "GetIgnoreEmpty is deprecated" - path: private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go - - linters: - - staticcheck - text: "GetSkipped is deprecated" - path: private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go - linters: - gochecknoinits # we actually want to use init here diff --git a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go index 8bd1baf0c6..1e9f5be263 100644 --- a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go +++ b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go @@ -301,15 +301,6 @@ func checkFieldFlags( fieldCount++ return true }) - if fieldConstraints.GetSkipped() && fieldCount > 1 { - adder.addForPathf( - []int32{skippedFieldNumber}, - "Field %q has %s and therefore other rules in %s are not applied and should be removed.", - adder.fieldName(), - adder.getFieldRuleName(skippedFieldNumber), - adder.getFieldRuleName(), - ) - } if fieldConstraints.GetIgnore() == validate.Ignore_IGNORE_ALWAYS && fieldCount > 1 { adder.addForPathf( []int32{ignoreFieldNumber}, @@ -320,18 +311,6 @@ func checkFieldFlags( adder.getFieldRuleName(), ) } - if fieldConstraints.GetRequired() && fieldConstraints.GetIgnoreEmpty() { - adder.addForPathsf( - [][]int32{ - {requiredFieldNumber}, - {ignoreEmptyFieldNumber}, - }, - "Field %q has both %s and %s. A field cannot be empty if it is required.", - adder.fieldName(), - adder.getFieldRuleName(requiredFieldNumber), - adder.getFieldRuleName(ignoreEmptyFieldNumber), - ) - } if fieldConstraints.GetRequired() && fieldConstraints.GetIgnore() == validate.Ignore_IGNORE_IF_UNPOPULATED { adder.addForPathsf( [][]int32{ @@ -395,14 +374,6 @@ func checkConstraintsForExtension( adder.getFieldRuleName(requiredFieldNumber), ) } - if fieldConstraints.GetIgnoreEmpty() { - adder.addForPathf( - []int32{ignoreEmptyFieldNumber}, - "Field %q is an extension field and cannot have %s.", - adder.fieldName(), - adder.getFieldRuleName(ignoreEmptyFieldNumber), - ) - } if fieldConstraints.GetIgnore() == validate.Ignore_IGNORE_IF_UNPOPULATED { adder.addForPathf( []int32{ignoreFieldNumber}, From f535770e1621bd8d9b731495d431b220bcf73966 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 4 Feb 2025 15:31:34 -0500 Subject: [PATCH 2/2] Update protovalidate linting+tests for v0.9.x --- go.mod | 18 +- go.sum | 40 +-- .../buflintvalidate/buflintvalidate.go | 4 +- .../internal/buflintvalidate/cel.go | 14 +- .../internal/buflintvalidate/field.go | 248 +++++++++++++----- .../buflintvalidate/predefined_rules.go | 10 +- .../internal/buflintvalidate/util.go | 23 -- .../buf-plugin-protovalidate-ext/handlers.go | 14 +- private/bufpkg/bufcheck/lint_test.go | 6 +- .../lint/protovalidate/proto/extension.proto | 10 +- .../lint/protovalidate/proto/field.proto | 8 +- 11 files changed, 240 insertions(+), 155 deletions(-) diff --git a/go.mod b/go.mod index 447f9c129c..90442c9139 100644 --- a/go.mod +++ b/go.mod @@ -6,21 +6,21 @@ toolchain go1.23.4 require ( buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go v1.36.3-20250121211742-6d880cc6cc8d.1 - buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.3-20241127180247-a33202765966.1 + buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.4-20250130201111-63bb56e20495.1 buf.build/gen/go/bufbuild/registry/connectrpc/go v1.18.1-20250106231242-56271afbd6ce.1 buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.3-20250106231242-56271afbd6ce.1 - buf.build/go/bufplugin v0.7.0 + buf.build/go/bufplugin v0.8.0 buf.build/go/protoyaml v0.3.1 buf.build/go/spdx v0.2.0 connectrpc.com/connect v1.18.1 connectrpc.com/otelconnect v0.7.1 github.com/bufbuild/protocompile v0.14.1 github.com/bufbuild/protoplugin v0.0.0-20250106231243-3a819552c9d9 - github.com/bufbuild/protovalidate-go v0.8.2 + github.com/bufbuild/protovalidate-go v0.9.1 github.com/docker/docker v27.5.0+incompatible github.com/go-chi/chi/v5 v5.2.0 github.com/gofrs/flock v0.12.1 - github.com/google/cel-go v0.22.1 + github.com/google/cel-go v0.23.1 github.com/google/go-cmp v0.6.0 github.com/google/go-containerregistry v0.20.2 github.com/google/uuid v1.6.0 @@ -41,20 +41,20 @@ require ( go.uber.org/zap v1.27.0 go.uber.org/zap/exp v0.3.0 golang.org/x/crypto v0.32.0 - golang.org/x/exp v0.0.0-20250106191152-7588d65b2ba8 + golang.org/x/exp v0.0.0-20250128182459-e0ece0dbea4c golang.org/x/mod v0.22.0 golang.org/x/net v0.34.0 golang.org/x/sync v0.10.0 golang.org/x/term v0.28.0 golang.org/x/tools v0.29.0 - google.golang.org/protobuf v1.36.4-0.20250116160514-2005adbe0cf6 + google.golang.org/protobuf v1.36.4 gopkg.in/yaml.v3 v3.0.1 pluginrpc.com/pluginrpc v0.5.0 ) require ( buf.build/gen/go/pluginrpc/pluginrpc/protocolbuffers/go v1.36.3-20241007202033-cf42259fcbfc.1 // indirect - cel.dev/expr v0.19.1 // indirect + cel.dev/expr v0.19.2 // indirect github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c // indirect github.com/Microsoft/go-winio v0.6.2 // indirect github.com/Microsoft/hcsshim v0.12.9 // indirect @@ -124,7 +124,7 @@ require ( go.uber.org/multierr v1.11.0 // indirect golang.org/x/sys v0.29.0 // indirect golang.org/x/text v0.21.0 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20250115164207-1a7da9e5054f // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20250115164207-1a7da9e5054f // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20250127172529-29210b9bc287 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20250127172529-29210b9bc287 // indirect google.golang.org/grpc v1.69.4 // indirect ) diff --git a/go.sum b/go.sum index 456f59e2db..d17f29e7c2 100644 --- a/go.sum +++ b/go.sum @@ -1,21 +1,21 @@ buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go v1.36.3-20250121211742-6d880cc6cc8d.1 h1:1v+ez1GRKKKdI1IwDDQqV98lGKo8489+Ekql+prUW6c= buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go v1.36.3-20250121211742-6d880cc6cc8d.1/go.mod h1:MYDFm9IHRP085R5Bis68mLc0mIqp5Q27Uk4o8YXjkAI= -buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.3-20241127180247-a33202765966.1 h1:cQZXKoQ+eB0kykzfJe80RP3nc+3PWbbBrUBm8XNYAQY= -buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.3-20241127180247-a33202765966.1/go.mod h1:6VPKM8zbmgf9qsmkmKeH49a36Vtmidw3rG53B5mTenc= +buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.4-20250130201111-63bb56e20495.1 h1:4erM3WLgEG/HIBrpBDmRbs1puhd7p0z7kNXDuhHthwM= +buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.4-20250130201111-63bb56e20495.1/go.mod h1:novQBstnxcGpfKf8qGRATqn1anQKwMJIbH5Q581jibU= buf.build/gen/go/bufbuild/registry/connectrpc/go v1.18.1-20250106231242-56271afbd6ce.1 h1:4bJ5Sh3FovNqit+k1rYn03YpckFkgpBjeZe52eoiuUY= buf.build/gen/go/bufbuild/registry/connectrpc/go v1.18.1-20250106231242-56271afbd6ce.1/go.mod h1:GI0Fv/enMZ/dJPfDwU5zamn8P3LUEEl2L/1yg0qw4ZQ= buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.3-20250106231242-56271afbd6ce.1 h1:yPzRLpc0SqVzph6J9NcNux3B7vx4hNy8svO36F+mogc= buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.3-20250106231242-56271afbd6ce.1/go.mod h1:UOdD+CmwdvN3oRHXeZ+WDeIthKVXKo7Dm+2E/233xd0= buf.build/gen/go/pluginrpc/pluginrpc/protocolbuffers/go v1.36.3-20241007202033-cf42259fcbfc.1 h1:NOipq02MS20WQCr6rfAG1o0n2AuQnY4Xg9avLl16csA= buf.build/gen/go/pluginrpc/pluginrpc/protocolbuffers/go v1.36.3-20241007202033-cf42259fcbfc.1/go.mod h1:jceo5esD5zSbflHHGad57RXzBpRrcPaiLrLQRA+Mbec= -buf.build/go/bufplugin v0.7.0 h1:Tq8FXBVfpMxhl3QR6P/gMQHROg1Ss7WhpyD4QVV61ds= -buf.build/go/bufplugin v0.7.0/go.mod h1:LuQzv36Ezu2zQIQUtwg4WJJFe58tXn1anL1IosAh6ik= +buf.build/go/bufplugin v0.8.0 h1:YgR1+CNGmzR69jt85oRWTa5FioZoX/tOrHV+JxfNnnk= +buf.build/go/bufplugin v0.8.0/go.mod h1:rcm0Esd3P/GM2rtYTvz3+9Gf8w9zdo7rG8dKSxYHHIE= buf.build/go/protoyaml v0.3.1 h1:ucyzE7DRnjX+mQ6AH4JzN0Kg50ByHHu+yrSKbgQn2D4= buf.build/go/protoyaml v0.3.1/go.mod h1:0TzNpFQDXhwbkXb/ajLvxIijqbve+vMQvWY/b3/Dzxg= buf.build/go/spdx v0.2.0 h1:IItqM0/cMxvFJJumcBuP8NrsIzMs/UYjp/6WSpq8LTw= buf.build/go/spdx v0.2.0/go.mod h1:bXdwQFem9Si3nsbNy8aJKGPoaPi5DKwdeEp5/ArZ6w8= -cel.dev/expr v0.19.1 h1:NciYrtDRIR0lNCnH1LFJegdjspNx9fI59O7TWcua/W4= -cel.dev/expr v0.19.1/go.mod h1:MrpN08Q+lEBs+bGYdLxxHkZoUSsCp0nSKTs0nTymJgw= +cel.dev/expr v0.19.2 h1:V354PbqIXr9IQdwy4SYA4xa0HXaWq1BUPAGzugBY5V4= +cel.dev/expr v0.19.2/go.mod h1:MrpN08Q+lEBs+bGYdLxxHkZoUSsCp0nSKTs0nTymJgw= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= connectrpc.com/connect v1.18.1 h1:PAg7CjSAGvscaf6YZKUefjoih5Z/qYkyaTrBW8xvYPw= connectrpc.com/connect v1.18.1/go.mod h1:0292hj1rnx8oFrStN7cB4jjVBeqs+Yx5yDIC2prWDO8= @@ -36,8 +36,8 @@ github.com/bufbuild/protocompile v0.14.1 h1:iA73zAf/fyljNjQKwYzUHD6AD4R8KMasmwa/ github.com/bufbuild/protocompile v0.14.1/go.mod h1:ppVdAIhbr2H8asPk6k4pY7t9zB1OU5DoEw9xY/FUi1c= github.com/bufbuild/protoplugin v0.0.0-20250106231243-3a819552c9d9 h1:kAWER21DzhzU7ys8LL1WkSfbGkwXv+tM30hyEsYrW2k= github.com/bufbuild/protoplugin v0.0.0-20250106231243-3a819552c9d9/go.mod h1:c5D8gWRIZ2HLWO3gXYTtUfw/hbJyD8xikv2ooPxnklQ= -github.com/bufbuild/protovalidate-go v0.8.2 h1:sgzXHkHYP6HnAsL2Rd3I1JxkYUyEQUv9awU1PduMxbM= -github.com/bufbuild/protovalidate-go v0.8.2/go.mod h1:K6w8iPNAXBoIivVueSELbUeUl+MmeTQfCDSug85pn3M= +github.com/bufbuild/protovalidate-go v0.9.1 h1:cdrIA33994yCcJyEIZRL36ZGTe9UDM/WHs5MBHEimiE= +github.com/bufbuild/protovalidate-go v0.9.1/go.mod h1:5jptBxfvlY51RhX32zR6875JfPBRXUsQjyZjm/NqkLQ= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -98,8 +98,8 @@ github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymF github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= -github.com/envoyproxy/protoc-gen-validate v1.1.0 h1:tntQDh69XqOCOZsDz0lVJQez/2L6Uu2PdjCQwWCJ3bM= -github.com/envoyproxy/protoc-gen-validate v1.1.0/go.mod h1:sXRDRVmzEbkM7CVcM06s9shE/m23dg3wzjl0UWqJ2q4= +github.com/envoyproxy/protoc-gen-validate v1.2.1 h1:DEo3O99U8j4hBFwbJfrz9VtgcDfUKS7KJ7spH3d86P8= +github.com/envoyproxy/protoc-gen-validate v1.2.1/go.mod h1:d/C80l/jxXLdfEIhX1W2TmLfsJ31lvEjwamM4DxlWXU= github.com/felixge/fgprof v0.9.3/go.mod h1:RdbpDgzqYVh/T9fPELJyV7EYJuHB55UTEULNun8eiPw= github.com/felixge/fgprof v0.9.5 h1:8+vR6yu2vvSKn08urWyEuxx75NWPEvybbkBirEpsbVY= github.com/felixge/fgprof v0.9.5/go.mod h1:yKl+ERSa++RYOs32d8K6WEXCB4uXdLls4ZaZPpayhMM= @@ -137,8 +137,8 @@ github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QD github.com/golang/protobuf v1.4.3/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= -github.com/google/cel-go v0.22.1 h1:AfVXx3chM2qwoSbM7Da8g8hX8OVSkBFwX+rz2+PcK40= -github.com/google/cel-go v0.22.1/go.mod h1:BuznPXXfQDpXKWQ9sPW3TzlAJN5zzFe+i9tIs0yC4s8= +github.com/google/cel-go v0.23.1 h1:91ThhEZlBcE5rB2adBVXqvDoqdL8BG2oyhd0bK1I/r4= +github.com/google/cel-go v0.23.1/go.mod h1:52Pb6QsDbC5kvgxvZhiL9QX1oZEkcUF/ZqaPx1J5Wwo= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= @@ -312,8 +312,8 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.32.0 h1:euUpcYgM8WcP71gNpTqQCn6rC2t6ULUPiOzfWaXVVfc= golang.org/x/crypto v0.32.0/go.mod h1:ZnnJkOaASj8g0AjIduWNlq2NRxL0PlBrbKVyZ6V/Ugc= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= -golang.org/x/exp v0.0.0-20250106191152-7588d65b2ba8 h1:yqrTHse8TCMW1M1ZCP+VAR/l0kKxwaAIqN/il7x4voA= -golang.org/x/exp v0.0.0-20250106191152-7588d65b2ba8/go.mod h1:tujkw807nyEEAamNbDrEGzRav+ilXA7PCRAd6xsmwiU= +golang.org/x/exp v0.0.0-20250128182459-e0ece0dbea4c h1:KL/ZBHXgKGVmuZBZ01Lt57yE5ws8ZPSkkihmEyq7FXc= +golang.org/x/exp v0.0.0-20250128182459-e0ece0dbea4c/go.mod h1:tujkw807nyEEAamNbDrEGzRav+ilXA7PCRAd6xsmwiU= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= @@ -379,10 +379,10 @@ google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7 google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98Agz4BDEuKkezgsaosCRResVns1a3J2ZsMNc= google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo= -google.golang.org/genproto/googleapis/api v0.0.0-20250115164207-1a7da9e5054f h1:gap6+3Gk41EItBuyi4XX/bp4oqJ3UwuIMl25yGinuAA= -google.golang.org/genproto/googleapis/api v0.0.0-20250115164207-1a7da9e5054f/go.mod h1:Ic02D47M+zbarjYYUlK57y316f2MoN0gjAwI3f2S95o= -google.golang.org/genproto/googleapis/rpc v0.0.0-20250115164207-1a7da9e5054f h1:OxYkA3wjPsZyBylwymxSHa7ViiW1Sml4ToBrncvFehI= -google.golang.org/genproto/googleapis/rpc v0.0.0-20250115164207-1a7da9e5054f/go.mod h1:+2Yz8+CLJbIfL9z73EW45avw8Lmge3xVElCP9zEKi50= +google.golang.org/genproto/googleapis/api v0.0.0-20250127172529-29210b9bc287 h1:A2ni10G3UlplFrWdCDJTl7D7mJ7GSRm37S+PDimaKRw= +google.golang.org/genproto/googleapis/api v0.0.0-20250127172529-29210b9bc287/go.mod h1:iYONQfRdizDB8JJBybql13nArx91jcUk7zCXEsOofM4= +google.golang.org/genproto/googleapis/rpc v0.0.0-20250127172529-29210b9bc287 h1:J1H9f+LEdWAfHcez/4cvaVBox7cOYT+IU6rgqj5x++8= +google.golang.org/genproto/googleapis/rpc v0.0.0-20250127172529-29210b9bc287/go.mod h1:8BS3B93F/U1juMFq9+EDk+qOT5CO1R9IzXxG3PTqiRk= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY= @@ -399,8 +399,8 @@ google.golang.org/protobuf v1.22.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2 google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c= -google.golang.org/protobuf v1.36.4-0.20250116160514-2005adbe0cf6 h1:ExbOpvwTDdpfXk6InTqz/MevF+sSFWrAZlfZUy5Kw6k= -google.golang.org/protobuf v1.36.4-0.20250116160514-2005adbe0cf6/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE= +google.golang.org/protobuf v1.36.4 h1:6A3ZDJHn/eNqc1i+IdefRzy/9PokBTPvcqMySR7NNIM= +google.golang.org/protobuf v1.36.4/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= diff --git a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/buflintvalidate.go b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/buflintvalidate.go index b1f7fe613e..3be41fcc79 100644 --- a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/buflintvalidate.go +++ b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/buflintvalidate.go @@ -18,7 +18,7 @@ import ( "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "github.com/bufbuild/buf/private/bufpkg/bufprotosource" "github.com/bufbuild/buf/private/pkg/protoencoding" - "github.com/bufbuild/protovalidate-go/resolver" + "github.com/bufbuild/protovalidate-go/resolve" ) // https://buf.build/bufbuild/protovalidate/docs/v0.5.1:buf.validate#buf.validate.MessageConstraints @@ -35,7 +35,7 @@ func CheckMessage( if err != nil { return err } - messageConstraints := resolver.DefaultResolver{}.ResolveMessageConstraints(messageDescriptor) + messageConstraints := resolve.MessageConstraints(messageDescriptor) if messageConstraints.GetDisabled() && len(messageConstraints.GetCel()) > 0 { addAnnotationFunc( message, diff --git a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/cel.go b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/cel.go index 301193b35b..e9f185b88f 100644 --- a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/cel.go +++ b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/cel.go @@ -20,7 +20,7 @@ import ( "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "github.com/bufbuild/buf/private/bufpkg/bufprotosource" - "github.com/bufbuild/protovalidate-go/celext" + celpv "github.com/bufbuild/protovalidate-go/cel" "github.com/google/cel-go/cel" "github.com/google/cel-go/common/types" "google.golang.org/protobuf/reflect/protoreflect" @@ -40,7 +40,9 @@ func checkCELForMessage( messageDescriptor protoreflect.MessageDescriptor, message bufprotosource.Message, ) error { - celEnv, err := celext.DefaultEnv(false) + celEnv, err := cel.NewEnv( + cel.Lib(celpv.NewLibrary()), + ) if err != nil { return err } @@ -79,14 +81,16 @@ func checkCELForField( if len(fieldConstraints.GetCel()) == 0 { return nil } - celEnv, err := celext.DefaultEnv(false) + celEnv, err := cel.NewEnv( + cel.Lib(celpv.NewLibrary()), + ) if err != nil { return err } celEnv, err = celEnv.Extend( append( - celext.RequiredCELEnvOptions(fieldDescriptor), - cel.Variable("this", celext.ProtoFieldToCELType(fieldDescriptor, false, forItems)), + celpv.RequiredEnvOptions(fieldDescriptor), + cel.Variable("this", celpv.ProtoFieldToType(fieldDescriptor, false, forItems)), )..., ) if err != nil { diff --git a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go index 1e9f5be263..431840cf0b 100644 --- a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go +++ b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go @@ -26,10 +26,11 @@ import ( "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/protovalidate-go" - "github.com/bufbuild/protovalidate-go/resolver" + "github.com/bufbuild/protovalidate-go/resolve" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protodesc" "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/types/descriptorpb" "google.golang.org/protobuf/types/dynamicpb" "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/durationpb" @@ -163,7 +164,7 @@ func checkField( if err != nil { return err } - constraints := resolver.DefaultResolver{}.ResolveFieldConstraints(fieldDescriptor) + constraints := resolve.FieldConstraints(fieldDescriptor) return checkConstraintsForField( &adder{ field: field, @@ -260,7 +261,6 @@ func checkConstraintsForField( []int32{typeRulesFieldNumber, exampleFieldNumber}, fieldConstraints, typeRulesMessage, - containingMessageDescriptor, parentMapFieldDescriptor, fieldDescriptor, exampleValues, @@ -731,7 +731,6 @@ func checkExampleValues( pathToExampleValues []int32, fieldConstraints *validate.FieldConstraints, typeRulesMessage protoreflect.Message, - containingMessageDescriptor protoreflect.MessageDescriptor, // Not nil only if fieldDescriptor is a synthetic field for map key/value. parentMapFieldDescriptor protoreflect.FieldDescriptor, fieldDescriptor protoreflect.FieldDescriptor, @@ -771,99 +770,106 @@ func checkExampleValues( // errors by field name to determine whether this example value fails rules defined // on the same field. // - // Pass a constraint resolver interceptor so that constraints on other - // fields are not looked at by the validator. - constraintInterceptor := func(res protovalidate.StandardConstraintResolver) protovalidate.StandardConstraintResolver { - return &constraintsResolverForTargetField{ - StandardConstraintResolver: res, - targetField: fieldDescriptor, - } + // Construct a temporary message containing only the relevant field so that + // constraints on other fields are not evaluated by the validator. + newParentMapFieldDescriptor, newFieldDescriptor, err := createSyntheticFieldDescriptorForChecking( + parentMapFieldDescriptor, + fieldDescriptor, + extensionTypeResolver, + ) + if err != nil { + return err } - // For map fields, we want to resolve the constraints on the parentMapFieldDescriptor rather - // than the MapEntry. - if parentMapFieldDescriptor != nil { - constraintInterceptor = func(res protovalidate.StandardConstraintResolver) protovalidate.StandardConstraintResolver { - // Pass a constraint resolver interceptor so that constraints on other - // fields are not looked at by the validator. - return &constraintsResolverForTargetField{ - StandardConstraintResolver: res, - targetField: parentMapFieldDescriptor, - } - } + newContainingMessageDescriptor := newFieldDescriptor.ContainingMessage() + if newParentMapFieldDescriptor != nil { + newContainingMessageDescriptor = newParentMapFieldDescriptor.ContainingMessage() } - validator, err := protovalidate.New(protovalidate.WithStandardConstraintInterceptor(constraintInterceptor)) + validator, err := protovalidate.New() if err != nil { return err } // The shape of field path in a protovalidate.Violation depends on the type of the field descriptor. violationFilterFunc := func(violation *validate.Violation) bool { - return protovalidate.FieldPathString(violation.GetField()) == string(fieldDescriptor.Name()) + return len(violation.GetField().GetElements()) == 1 && + violation.GetField().GetElements()[0].GetFieldNumber() == int32(fieldDescriptor.Number()) && + violation.GetField().GetElements()[0].GetSubscript() == nil } switch { - case fieldDescriptor.IsList(): + case newFieldDescriptor.IsList(): // Field path looks like repeated_field[10] violationFilterFunc = func(violation *validate.Violation) bool { - prefix := fieldDescriptor.Name() + "[" - suffix := "]" - fieldPath := protovalidate.FieldPathString(violation.GetField()) - return strings.HasPrefix(fieldPath, string(prefix)) && strings.HasSuffix(fieldPath, suffix) + if len(violation.GetField().GetElements()) != 1 || + violation.GetField().GetElements()[0].GetFieldNumber() != int32(fieldDescriptor.Number()) || + violation.GetField().GetElements()[0].GetSubscript() == nil { + return false + } + _, ok := violation.GetField().GetElements()[0].Subscript.(*validate.FieldPathElement_Index) + return ok } - case parentMapFieldDescriptor != nil && fieldDescriptor.Name() == "key": + case parentMapFieldDescriptor != nil && newFieldDescriptor.Name() == "key": // Field path looks like map_field["the key value that failed"] and ForKey is set to true. violationFilterFunc = func(violation *validate.Violation) bool { - prefix := parentMapFieldDescriptor.Name() + "[" - suffix := "]" - fieldPath := protovalidate.FieldPathString(violation.GetField()) - return strings.HasPrefix(fieldPath, string(prefix)) && strings.HasSuffix(fieldPath, suffix) && violation.GetForKey() + if !violation.GetForKey() || + len(violation.GetField().GetElements()) != 1 || + violation.GetField().GetElements()[0].GetFieldNumber() != int32(parentMapFieldDescriptor.Number()) || + violation.GetField().GetElements()[0].GetSubscript() == nil { + return false + } + _, ok := violation.GetField().GetElements()[0].Subscript.(*validate.FieldPathElement_Index) + return !ok } - case parentMapFieldDescriptor != nil && fieldDescriptor.Name() == "value": + case newParentMapFieldDescriptor != nil && newFieldDescriptor.Name() == "value": // Field path looks like map_field["the key value that failed"], but ForKey is set to false. violationFilterFunc = func(violation *validate.Violation) bool { - prefix := parentMapFieldDescriptor.Name() + "[" - suffix := "]" - fieldPath := protovalidate.FieldPathString(violation.GetField()) - return strings.HasPrefix(fieldPath, string(prefix)) && strings.HasSuffix(fieldPath, suffix) && !violation.GetForKey() + if violation.GetForKey() || + len(violation.GetField().GetElements()) != 1 || + violation.GetField().GetElements()[0].GetFieldNumber() != int32(parentMapFieldDescriptor.Number()) || + violation.GetField().GetElements()[0].GetSubscript() == nil { + return false + } + _, ok := violation.GetField().GetElements()[0].Subscript.(*validate.FieldPathElement_Index) + return !ok } } for exampleValueIndex, exampleValue := range exampleValues { - messageToValidate := dynamicpb.NewMessage(containingMessageDescriptor) + messageToValidate := dynamicpb.NewMessage(newContainingMessageDescriptor) switch { - case fieldDescriptor.IsList(): - list := messageToValidate.NewField(fieldDescriptor).List() + case newFieldDescriptor.IsList(): + list := messageToValidate.NewField(newFieldDescriptor).List() list.Append(exampleValue) - messageToValidate.Set(fieldDescriptor, protoreflect.ValueOfList(list)) - case parentMapFieldDescriptor != nil: - mapEntryMessageDescriptor := fieldDescriptor.ContainingMessage() + messageToValidate.Set(newFieldDescriptor, protoreflect.ValueOfList(list)) + case newParentMapFieldDescriptor != nil: + mapEntryMessageDescriptor := newFieldDescriptor.ContainingMessage() // We are being very defensive, because a type mismatch may cause a panic in protoreflect. if !mapEntryMessageDescriptor.IsMapEntry() { return syserror.Newf("containing message %q is not a map", mapEntryMessageDescriptor.Name()) } - if !parentMapFieldDescriptor.IsMap() { - return syserror.Newf("parent field descriptor %q is passed but is not a map field", parentMapFieldDescriptor.Name()) + if !newParentMapFieldDescriptor.IsMap() { + return syserror.Newf("parent field descriptor %q is passed but is not a map field", newParentMapFieldDescriptor.Name()) } - if containingMessageDescriptor.Fields().ByName(parentMapFieldDescriptor.Name()) == nil { - return syserror.Newf("containing message %q does not have field named %q", containingMessageDescriptor.Name(), parentMapFieldDescriptor.Name()) + if newContainingMessageDescriptor.Fields().ByName(newParentMapFieldDescriptor.Name()) == nil { + return syserror.Newf("containing message %q does not have field named %q", newContainingMessageDescriptor.Name(), newParentMapFieldDescriptor.Name()) } - if parentMapFieldDescriptor.Message().Name() != mapEntryMessageDescriptor.Name() { - return syserror.Newf("field %q should have parent of type %q but has type %q", fieldDescriptor.Name(), parentMapFieldDescriptor.Message().Name(), containingMessageDescriptor.Name()) + if newParentMapFieldDescriptor.Message().Name() != mapEntryMessageDescriptor.Name() { + return syserror.Newf("field %q should have parent of type %q but has type %q", newFieldDescriptor.Name(), parentMapFieldDescriptor.Message().Name(), newContainingMessageDescriptor.Name()) } - mapEntry := messageToValidate.NewField(parentMapFieldDescriptor).Map() - switch fieldDescriptor.Name() { + mapEntry := messageToValidate.NewField(newParentMapFieldDescriptor).Map() + switch newFieldDescriptor.Name() { case "key": mapEntry.Set( exampleValue.MapKey(), - dynamicpb.NewMessage(parentMapFieldDescriptor.Message()).NewField(parentMapFieldDescriptor.MapValue()), + dynamicpb.NewMessage(newParentMapFieldDescriptor.Message()).NewField(newParentMapFieldDescriptor.MapValue()), ) case "value": mapEntry.Set( - dynamicpb.NewMessage(parentMapFieldDescriptor.Message()).NewField(parentMapFieldDescriptor.MapKey()).MapKey(), + dynamicpb.NewMessage(newParentMapFieldDescriptor.Message()).NewField(newParentMapFieldDescriptor.MapKey()).MapKey(), exampleValue, ) default: - return syserror.Newf("expected key or value as synthetic field name for map entry's field name, got %q", fieldDescriptor.Name()) + return syserror.Newf("expected key or value as synthetic field name for map entry's field name, got %q", newFieldDescriptor.Name()) } - messageToValidate.Set(parentMapFieldDescriptor, protoreflect.ValueOfMap(mapEntry)) - case fieldDescriptor.Enum() != nil: + messageToValidate.Set(newParentMapFieldDescriptor, protoreflect.ValueOfMap(mapEntry)) + case newFieldDescriptor.Enum() != nil: // We need to handle enum examples in a special way, since enum examples are set as // int32, but we need to set it to the enum value to the field. // So we cast exampleValue to an int32 and check that cast first before attempting @@ -873,17 +879,17 @@ func checkExampleValues( if !ok { return syserror.Newf("expected enum example value to be int32 for field %q, got %T type instead", fieldDescriptor.FullName(), exampleValue) } - messageToValidate.Set(fieldDescriptor, protoreflect.ValueOf(protoreflect.EnumNumber(exampleInt32))) - case fieldDescriptor.Message() != nil: + messageToValidate.Set(newFieldDescriptor, protoreflect.ValueOf(protoreflect.EnumNumber(exampleInt32))) + case newFieldDescriptor.Message() != nil: // We need to handle the case where the field is a wrapper type. We set the value directly base on the wrapper type. - switch string(fieldDescriptor.Message().FullName()) { + switch string(newFieldDescriptor.Message().FullName()) { case string((&wrapperspb.FloatValue{}).ProtoReflect().Descriptor().FullName()): exampleValueFloat, ok := exampleValue.Interface().(float32) if !ok { return syserror.Newf("unexpected type found for float wrapper type %T", exampleValue.Interface()) } messageToValidate.Set( - fieldDescriptor, + newFieldDescriptor, protoreflect.ValueOf( (&wrapperspb.FloatValue{Value: exampleValueFloat}).ProtoReflect(), ), @@ -894,7 +900,7 @@ func checkExampleValues( return syserror.Newf("unexpected type found for double wrapper type %T", exampleValue.Interface()) } messageToValidate.Set( - fieldDescriptor, + newFieldDescriptor, protoreflect.ValueOf( (&wrapperspb.DoubleValue{Value: exampleValueDouble}).ProtoReflect(), ), @@ -905,7 +911,7 @@ func checkExampleValues( return syserror.Newf("unexpected type found for int32 wrapper type %T", exampleValue.Interface()) } messageToValidate.Set( - fieldDescriptor, + newFieldDescriptor, protoreflect.ValueOf( (&wrapperspb.Int32Value{Value: exampleValueInt32}).ProtoReflect(), ), @@ -916,7 +922,7 @@ func checkExampleValues( return syserror.Newf("unexpected type found for int64 wrapper type %T", exampleValue.Interface()) } messageToValidate.Set( - fieldDescriptor, + newFieldDescriptor, protoreflect.ValueOf( (&wrapperspb.Int64Value{Value: exampleValueInt64}).ProtoReflect(), ), @@ -927,7 +933,7 @@ func checkExampleValues( return syserror.Newf("unexpected type found for uint32 wrapper type %T", exampleValue.Interface()) } messageToValidate.Set( - fieldDescriptor, + newFieldDescriptor, protoreflect.ValueOf( (&wrapperspb.UInt32Value{Value: exampleValueUInt32}).ProtoReflect(), ), @@ -938,7 +944,7 @@ func checkExampleValues( return syserror.Newf("unexpected type found for uint32 wrapper type %T", exampleValue.Interface()) } messageToValidate.Set( - fieldDescriptor, + newFieldDescriptor, protoreflect.ValueOf( (&wrapperspb.UInt64Value{Value: exampleValueUInt64}).ProtoReflect(), ), @@ -949,7 +955,7 @@ func checkExampleValues( return syserror.Newf("unexpected type found for bool wrapper type %T", exampleValue.Interface()) } messageToValidate.Set( - fieldDescriptor, + newFieldDescriptor, protoreflect.ValueOf( (&wrapperspb.BoolValue{Value: exampleValueBool}).ProtoReflect(), ), @@ -960,7 +966,7 @@ func checkExampleValues( return syserror.Newf("unexpected type found for string wrapper type %T", exampleValue.Interface()) } messageToValidate.Set( - fieldDescriptor, + newFieldDescriptor, protoreflect.ValueOf( (&wrapperspb.StringValue{Value: exampleValueString}).ProtoReflect(), ), @@ -971,7 +977,7 @@ func checkExampleValues( return syserror.Newf("unexpected type found for bytes wrapper type %T", exampleValue.Interface()) } messageToValidate.Set( - fieldDescriptor, + newFieldDescriptor, protoreflect.ValueOf( (&wrapperspb.BytesValue{Value: exampleValueBytes}).ProtoReflect(), ), @@ -979,10 +985,10 @@ func checkExampleValues( default: // In the case where it is not a wrapper type (e.g. google.protobuf.Timestamp), we just set the example // value directly. - messageToValidate.Set(fieldDescriptor, exampleValue) + messageToValidate.Set(newFieldDescriptor, exampleValue) } default: - messageToValidate.Set(fieldDescriptor, exampleValue) + messageToValidate.Set(newFieldDescriptor, exampleValue) } err := validator.Validate(messageToValidate) if err == nil { @@ -1008,6 +1014,104 @@ func checkExampleValues( return nil } +// createSyntheticFieldDescriptorForChecking creates a clone of a specific field +// into its own message so that it can be checked independently. For map fields, +// this includes creating a synthetic MapEntry as well. +func createSyntheticFieldDescriptorForChecking( + // If specified, parentMapFieldDescriptor is the map field to be cloned, + // e.g. it is a field on a message that points to a map type. + parentMapFieldDescriptor protoreflect.FieldDescriptor, + // If parentMapFieldDescriptor is specified, this is a MapKey or MapValue. + // Otherwise, it is the non-map field to be cloned. + fieldDescriptor protoreflect.FieldDescriptor, + resolver protodesc.Resolver, +) ( + newParentMapFieldDescriptor protoreflect.FieldDescriptor, + newFieldDescriptor protoreflect.FieldDescriptor, + err error, +) { + // We need to determine the "root" descriptor: if parentMapFieldDescriptor + // is set, then fieldDescriptor points to a MapKey or MapValue field, but we + // want to create a synthetic clone of the map field itself. + rootFieldDescriptor := fieldDescriptor + if parentMapFieldDescriptor != nil { + rootFieldDescriptor = parentMapFieldDescriptor + } + originalFile := rootFieldDescriptor.ContainingMessage().ParentFile() + newFileDescriptorProto := &descriptorpb.FileDescriptorProto{ + Name: proto.String(":synthetic:"), + Package: proto.String("__SYNTHETIC__"), + Syntax: proto.String("proto3"), + Dependency: []string{ + originalFile.Path(), + }, + } + + // Ensure that enum and message types are available to be resolved. It's a + // bit tricky to get this right, so we just import everything the original + // file containing the root message imports (and that file, too.) + for i := 0; i < originalFile.Imports().Len(); i++ { + newFileDescriptorProto.Dependency = append( + newFileDescriptorProto.Dependency, + originalFile.Imports().Get(i).Path(), + ) + } + + // Create a clone of the root field descriptor. Since our target message is + // going to be empty aside from this field, we also clear the OneofIndex. + newFieldDescriptorProto := protodesc.ToFieldDescriptorProto(rootFieldDescriptor) + newFieldDescriptorProto.OneofIndex = nil + + newMessageDescriptorProto := &descriptorpb.DescriptorProto{ + Name: proto.String(string(rootFieldDescriptor.ContainingMessage().Name())), + Field: []*descriptorpb.FieldDescriptorProto{newFieldDescriptorProto}, + } + + // The map field and entry message have to be declared in the same scope, so + // we need to make a clone of the map entry type itself. This needs to be + // nested under the synthetic message. + if parentMapFieldDescriptor != nil { + // Shouldn't happen, but we don't want to panic if it does. + if !parentMapFieldDescriptor.IsMap() { + return nil, nil, errors.New("parentMapFieldDescriptor is not a map") + } + newMapEntryMessageDescriptor := &descriptorpb.DescriptorProto{ + Name: proto.String(string(parentMapFieldDescriptor.Message().Name())), + Field: []*descriptorpb.FieldDescriptorProto{ + protodesc.ToFieldDescriptorProto(parentMapFieldDescriptor.MapKey()), + protodesc.ToFieldDescriptorProto(parentMapFieldDescriptor.MapValue()), + }, + Options: &descriptorpb.MessageOptions{ + MapEntry: proto.Bool(true), + }, + } + newMessageDescriptorProto.NestedType = append( + newFileDescriptorProto.MessageType, + newMapEntryMessageDescriptor, + ) + newFieldDescriptorProto.Type = descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum() + newFieldDescriptorProto.TypeName = proto.String(newMapEntryMessageDescriptor.GetName()) + } + newFileDescriptorProto.MessageType = append(newFileDescriptorProto.MessageType, newMessageDescriptorProto) + + newFileDescriptor, err := protodesc.NewFile(newFileDescriptorProto, resolver) + if err != nil { + return nil, nil, err + } + newRootDescriptor := newFileDescriptor.Messages().ByName( + rootFieldDescriptor.ContainingMessage().Name(), + ).Fields().ByNumber( + rootFieldDescriptor.Number(), + ) + if parentMapFieldDescriptor != nil { + return newRootDescriptor, newRootDescriptor.Message().Fields().ByNumber( + fieldDescriptor.Number(), + ), nil + } else { + return nil, newRootDescriptor, nil + } +} + func checkConst(adder *adder, rule proto.Message, ruleFieldNumber int32) { var ( fieldCount int diff --git a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/predefined_rules.go b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/predefined_rules.go index 49204c4438..fd7df88607 100644 --- a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/predefined_rules.go +++ b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/predefined_rules.go @@ -21,7 +21,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufprotosource" "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/syserror" - "github.com/bufbuild/protovalidate-go/celext" + celpv "github.com/bufbuild/protovalidate-go/cel" "github.com/google/cel-go/cel" ) @@ -59,7 +59,9 @@ func checkPredefinedRuleExtension( if predefinedConstraints == nil { return nil } - celEnv, err := celext.DefaultEnv(false) + celEnv, err := cel.NewEnv( + cel.Lib(celpv.NewLibrary()), + ) if err != nil { return err } @@ -80,7 +82,7 @@ func checkPredefinedRuleExtension( // }]; // } // - ruleType := celext.ProtoFieldToCELType(extensionDescriptor, false, false) + ruleType := celpv.ProtoFieldToType(extensionDescriptor, false, false) // To check for the type of "this", we check the descriptor for the rule type we are extending. thisType := celTypeForStandardRuleMessageDescriptor(extendedStandardRuleDescriptor) if thisType == nil { @@ -88,7 +90,7 @@ func checkPredefinedRuleExtension( } celEnv, err = celEnv.Extend( append( - celext.RequiredCELEnvOptions(extensionDescriptor), + celpv.RequiredEnvOptions(extensionDescriptor), cel.Variable("rule", ruleType), cel.Variable("this", thisType), )..., diff --git a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/util.go b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/util.go index 015227de15..2e873051cc 100644 --- a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/util.go +++ b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/util.go @@ -19,34 +19,11 @@ import ( "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "github.com/bufbuild/buf/private/pkg/protoencoding" - "github.com/bufbuild/protovalidate-go" "github.com/google/cel-go/cel" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" ) -// This implements protovalidate.StandardConstraintResolver, see checkExampleValues' comment -// for why this is needed. -type constraintsResolverForTargetField struct { - protovalidate.StandardConstraintResolver - targetField protoreflect.FieldDescriptor -} - -func (r *constraintsResolverForTargetField) ResolveFieldConstraints(desc protoreflect.FieldDescriptor) *validate.FieldConstraints { - if desc.FullName() != r.targetField.FullName() { - return nil - } - return r.StandardConstraintResolver.ResolveFieldConstraints(desc) -} - -func (*constraintsResolverForTargetField) ResolveMessageConstraints(protoreflect.MessageDescriptor) *validate.MessageConstraints { - return nil -} - -func (*constraintsResolverForTargetField) ResolveOneofConstraints(protoreflect.OneofDescriptor) *validate.OneofConstraints { - return nil -} - // This function is copied directly from protovalidate-go, except refactored to use protoencoding // for marshalling and unmarshalling. We also added error handling for marshal/unmarshal. // diff --git a/private/bufpkg/bufcheck/internal/cmd/buf-plugin-protovalidate-ext/handlers.go b/private/bufpkg/bufcheck/internal/cmd/buf-plugin-protovalidate-ext/handlers.go index aec4153f07..b1617840a0 100644 --- a/private/bufpkg/bufcheck/internal/cmd/buf-plugin-protovalidate-ext/handlers.go +++ b/private/bufpkg/bufcheck/internal/cmd/buf-plugin-protovalidate-ext/handlers.go @@ -20,7 +20,7 @@ import ( "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "buf.build/go/bufplugin/check" "buf.build/go/bufplugin/option" - "github.com/bufbuild/protovalidate-go/resolver" + "github.com/bufbuild/protovalidate-go/resolve" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -35,7 +35,7 @@ func checkFieldNotSkippedNoImport( request check.Request, fieldDescriptor protoreflect.FieldDescriptor, ) error { - constraints := resolver.DefaultResolver{}.ResolveFieldConstraints(fieldDescriptor) + constraints := resolve.FieldConstraints(fieldDescriptor) if constraints.GetIgnore() == validate.Ignore_IGNORE_ALWAYS { skippedRuleName := "(buf.validate.field).skipped" if fieldDescriptor.Cardinality() == protoreflect.Repeated { @@ -59,7 +59,7 @@ func checkFieldNotSkipped( request check.Request, fieldDescriptor protoreflect.FieldDescriptor, ) error { - constraints := resolver.DefaultResolver{}.ResolveFieldConstraints(fieldDescriptor) + constraints := resolve.FieldConstraints(fieldDescriptor) if constraints.GetIgnore() == validate.Ignore_IGNORE_ALWAYS { skippedRuleName := "(buf.validate.field).skipped" if fieldDescriptor.Cardinality() == protoreflect.Repeated { @@ -84,9 +84,9 @@ func checkStringLenRangeDontShrink( field protoreflect.FieldDescriptor, againstField protoreflect.FieldDescriptor, ) error { - againstConstraints := resolver.DefaultResolver{}.ResolveFieldConstraints(againstField) + againstConstraints := resolve.FieldConstraints(againstField) if againstStringRules := againstConstraints.GetString(); againstStringRules != nil { - constraints := resolver.DefaultResolver{}.ResolveFieldConstraints(field) + constraints := resolve.FieldConstraints(field) if stringRules := constraints.GetString(); stringRules != nil { if againstStringRules.MinLen != nil && stringRules.MinLen != nil && stringRules.GetMinLen() > againstStringRules.GetMinLen() { responseWriter.AddAnnotation( @@ -127,7 +127,7 @@ func checkValidateIDDashless( if fieldDescriptor.Kind() != protoreflect.StringKind { return nil } - constraints := resolver.DefaultResolver{}.ResolveFieldConstraints(fieldDescriptor) + constraints := resolve.FieldConstraints(fieldDescriptor) if stringConstraints := constraints.GetString(); stringConstraints == nil || !stringConstraints.GetTuuid() { missingRuleName := "(buf.validate.field).string.tuuid" if fieldDescriptor.Cardinality() == protoreflect.Repeated { @@ -151,7 +151,7 @@ func checkMessageNotDisabled( request check.Request, messageDescriptor protoreflect.MessageDescriptor, ) error { - constraints := resolver.DefaultResolver{}.ResolveMessageConstraints(messageDescriptor) + constraints := resolve.MessageConstraints(messageDescriptor) if constraints.GetDisabled() { responseWriter.AddAnnotation( check.WithMessagef("%s has (buf.validate.message).disabled set to true", string(messageDescriptor.Name())), diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index a80062ef63..3f1863697a 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -638,11 +638,11 @@ func TestRunProtovalidate(t *testing.T) { bufanalysistesting.NewFileAnnotation(t, "enum.proto", 28, 5, 28, 40, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "enum.proto", 36, 5, 36, 42, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "extension.proto", 25, 7, 25, 43, "PROTOVALIDATE"), - bufanalysistesting.NewFileAnnotation(t, "extension.proto", 30, 7, 30, 47, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "extension.proto", 30, 7, 30, 58, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "extension.proto", 40, 5, 40, 41, "PROTOVALIDATE"), - bufanalysistesting.NewFileAnnotation(t, "extension.proto", 45, 5, 45, 45, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "extension.proto", 45, 5, 45, 56, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "field.proto", 18, 5, 18, 41, "PROTOVALIDATE"), - bufanalysistesting.NewFileAnnotation(t, "field.proto", 19, 5, 19, 45, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "field.proto", 19, 5, 19, 56, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "map.proto", 24, 38, 24, 76, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "map.proto", 27, 5, 27, 43, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "map.proto", 29, 5, 29, 43, "PROTOVALIDATE"), diff --git a/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/extension.proto b/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/extension.proto index 1474bbebb9..fe738fd734 100644 --- a/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/extension.proto +++ b/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/extension.proto @@ -11,7 +11,7 @@ message ExtensionTest { ]; optional string valid_ignore_empty = 2 [ (buf.validate.field).string.min_len = 3, - (buf.validate.field).ignore_empty = true + (buf.validate.field).ignore = IGNORE_IF_UNPOPULATED ]; extensions 10 to 10000; } @@ -26,8 +26,8 @@ message Extending { ]; optional string invalid_ignore_empty = 14 [ (buf.validate.field).string.min_len = 3, - // ignore empty cannot be specified for extension - (buf.validate.field).ignore_empty = true + // ignore if unpopulated cannot be specified for extension + (buf.validate.field).ignore = IGNORE_IF_UNPOPULATED ]; } } @@ -41,7 +41,7 @@ extend ExtensionTest { ]; optional string invalid_ignore_empty = 24 [ (buf.validate.field).string.min_len = 3, - // ignore empty cannot be specified for extension - (buf.validate.field).ignore_empty = true + // ignore if unpopulated cannot be specified for extension + (buf.validate.field).ignore = IGNORE_IF_UNPOPULATED ]; } diff --git a/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/field.proto b/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/field.proto index 30e5650d1b..db3d193f40 100644 --- a/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/field.proto +++ b/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/field.proto @@ -10,16 +10,14 @@ message FieldTest { (buf.validate.field).int32.lt = 1 ]; string valid_ignore_empty = 2 [ - (buf.validate.field).ignore_empty = true, + (buf.validate.field).ignore = IGNORE_IF_UNPOPULATED, (buf.validate.field).string.min_len = 5 ]; string invalid_required_and_ignore_empty = 3 [ (buf.validate.field).string.min_len = 5, (buf.validate.field).required = true, - (buf.validate.field).ignore_empty = true + (buf.validate.field).ignore = IGNORE_IF_UNPOPULATED ]; // buf:lint:ignore PROTOVALIDATE - int32 int32_validated_with_int64 = 4 [ - (buf.validate.field).int64.gt = 5 - ]; + int32 int32_validated_with_int64 = 4 [(buf.validate.field).int64.gt = 5]; }