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/go.mod b/go.mod index 4791ddedae..917c2b5cff 100644 --- a/go.mod +++ b/go.mod @@ -6,17 +6,17 @@ toolchain go1.23.5 require ( buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go v1.36.4-20250121211742-6d880cc6cc8d.1 - buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.4-20241127180247-a33202765966.1 + buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.5-20250219170025-d39267d9df8f.1 buf.build/gen/go/bufbuild/registry/connectrpc/go v1.18.1-20250116203702-1c024d64352b.1 buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.4-20250116203702-1c024d64352b.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.3-0.20250317160558-38a17488914d github.com/docker/docker v28.0.0+incompatible github.com/go-chi/chi/v5 v5.2.1 github.com/gofrs/flock v0.12.1 @@ -53,7 +53,7 @@ require ( require ( buf.build/gen/go/pluginrpc/pluginrpc/protocolbuffers/go v1.36.4-20241007202033-cf42259fcbfc.1 // indirect - cel.dev/expr v0.19.2 // indirect + cel.dev/expr v0.21.2 // indirect github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 // indirect github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c // indirect github.com/Microsoft/go-winio v0.6.2 // indirect @@ -119,7 +119,7 @@ require ( golang.org/x/exp v0.0.0-20250228200357-dead58393ab7 // indirect golang.org/x/sys v0.31.0 // indirect golang.org/x/text v0.23.0 // 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/genproto/googleapis/api v0.0.0-20250224174004-546df14abb99 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20250224174004-546df14abb99 // indirect google.golang.org/grpc v1.70.0 // indirect ) diff --git a/go.sum b/go.sum index 529d0abe0b..f147b2191e 100644 --- a/go.sum +++ b/go.sum @@ -1,21 +1,21 @@ buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go v1.36.4-20250121211742-6d880cc6cc8d.1 h1:p5SFT60M93aMQhOz81VH3kPg8t1pp/Litae/1eSxie4= buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go v1.36.4-20250121211742-6d880cc6cc8d.1/go.mod h1:umI0o7WWHv8lCbLjYUMzfjHKjyaIt2D89sIj1D9fqy0= -buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.4-20241127180247-a33202765966.1 h1:yeaeyw0RQUe009ebxBQ3TsqBPptiNEGsiS10t+8Htuo= -buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.4-20241127180247-a33202765966.1/go.mod h1:novQBstnxcGpfKf8qGRATqn1anQKwMJIbH5Q581jibU= +buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.5-20250219170025-d39267d9df8f.1 h1:Y0dvIjy09SvloaRbMLTH//k1qDzWcxNm8uBrCwp75ug= +buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.5-20250219170025-d39267d9df8f.1/go.mod h1:eOqrCVUfhh7SLo00urDe/XhJHljj0dWMZirS0aX7cmc= buf.build/gen/go/bufbuild/registry/connectrpc/go v1.18.1-20250116203702-1c024d64352b.1 h1:1SDs5tEGoWWv2vmKLx2B0Bp+yfhlxiU4DaZUII8+Pvs= buf.build/gen/go/bufbuild/registry/connectrpc/go v1.18.1-20250116203702-1c024d64352b.1/go.mod h1:o2AgVM1j3MczvxnMqfZTpiqGwK1VD4JbEagseY0QcjE= buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.4-20250116203702-1c024d64352b.1 h1:uKJgSNHvwQUZ6+0dSnx9MtkZ+h/ORbkKym0rlzIjUSI= buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.4-20250116203702-1c024d64352b.1/go.mod h1:Ua59W2s7uwPS5sGNgW08QewjBaPnUxOdpkWsuDvJ36Q= buf.build/gen/go/pluginrpc/pluginrpc/protocolbuffers/go v1.36.4-20241007202033-cf42259fcbfc.1 h1:XmYgi9W/9oST2ZrfT3ucGWkzD9+Vd0ls9yhyZ8ae0KQ= buf.build/gen/go/pluginrpc/pluginrpc/protocolbuffers/go v1.36.4-20241007202033-cf42259fcbfc.1/go.mod h1:cxFpqWIC80Wm8YNo1038ocBmrF84uQ0IfL0uVdAu9ZY= -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.2 h1:V354PbqIXr9IQdwy4SYA4xa0HXaWq1BUPAGzugBY5V4= -cel.dev/expr v0.19.2/go.mod h1:MrpN08Q+lEBs+bGYdLxxHkZoUSsCp0nSKTs0nTymJgw= +cel.dev/expr v0.21.2 h1:o+Wj235dy4gFYlYin3JsMpp3EEfMrPm/6tdoyjT98S0= +cel.dev/expr v0.21.2/go.mod h1:MrpN08Q+lEBs+bGYdLxxHkZoUSsCp0nSKTs0nTymJgw= connectrpc.com/connect v1.18.1 h1:PAg7CjSAGvscaf6YZKUefjoih5Z/qYkyaTrBW8xvYPw= connectrpc.com/connect v1.18.1/go.mod h1:0292hj1rnx8oFrStN7cB4jjVBeqs+Yx5yDIC2prWDO8= connectrpc.com/otelconnect v0.7.1 h1:scO5pOb0i4yUE66CnNrHeK1x51yq0bE0ehPg6WvzXJY= @@ -32,8 +32,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.3-0.20250317160558-38a17488914d h1:Y6Yp/LwSaRG8gw9GyyQD7jensL9NXqPlkbuulaAvCEE= +github.com/bufbuild/protovalidate-go v0.9.3-0.20250317160558-38a17488914d/go.mod h1:SZN6Qr3lPWuKMoQtIhKdhESkb+3m2vk0lqN9WMuZDDU= 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/chromedp/cdproto v0.0.0-20230802225258-3cf4e6d46a89/go.mod h1:GKljq0VrfU4D5yc+2qA6OVr8pmO/MBbPEWqWQ/oqGEs= @@ -72,8 +72,8 @@ github.com/docker/go-connections v0.5.0 h1:USnMq7hx7gwdVZq1L49hLXaFtUdTADjXGp+uj github.com/docker/go-connections v0.5.0/go.mod h1:ov60Kzw0kKElRwhNs9UlUHAE/F9Fe6GLaXnqyDdmEXc= github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4= github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= -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= @@ -302,10 +302,10 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -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/genproto/googleapis/api v0.0.0-20250224174004-546df14abb99 h1:ilJhrCga0AptpJZXmUYG4MCrx/zf3l1okuYz7YK9PPw= +google.golang.org/genproto/googleapis/api v0.0.0-20250224174004-546df14abb99/go.mod h1:Xsh8gBVxGCcbV8ZeTB9wI5XPyZ5RvC6V3CTeeplHbiA= +google.golang.org/genproto/googleapis/rpc v0.0.0-20250224174004-546df14abb99 h1:ZSlhAUqC4r8TPzqLXQ0m3upBNZeF+Y8jQ3c4CR3Ujms= +google.golang.org/genproto/googleapis/rpc v0.0.0-20250224174004-546df14abb99/go.mod h1:LuRYeWDFV6WOn90g357N17oMCaxpgCnbi/44qJvDn2I= google.golang.org/grpc v1.70.0 h1:pWFv03aZoHzlRKHWicjsZytKAiYCtNS0dHbXnIdq7jQ= google.golang.org/grpc v1.70.0/go.mod h1:ofIJqVKDXx/JiXrwr2IG4/zwdH9txy3IlF40RmcJSQw= google.golang.org/protobuf v1.36.5 h1:tPhr+woSbjfYvY6/GPufUoYizxw1cF/yFoxJ2fmpwlM= diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index f2a5149701..a9baacbf9e 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -691,11 +691,11 @@ func (c *controller) GetMessage( } var validator protoyaml.Validator if functionOptions.messageValidation { - var err error - validator, err = protovalidate.New() + protovalidateValidator, err := protovalidate.New() if err != nil { return nil, 0, err } + validator = yamlValidator{protovalidateValidator} } var unmarshaler protoencoding.Unmarshaler switch messageEncoding { @@ -1287,6 +1287,15 @@ func (c *controller) handleFileAnnotationSetRetError(retErrAddr *error) { } } +// Implements [protoyaml.Validator] using [protovalidate.Validator]. This allows us to pass +// a [protovalidate.Validator] to the [protoencoding.NewYAMLUnmarshaler] and validate types +// when unmarshalling. +type yamlValidator struct{ protovalidateValidator protovalidate.Validator } + +func (v yamlValidator) Validate(msg proto.Message) error { + return v.protovalidateValidator.Validate(msg) +} + func getImageFileInfosForModuleSet(ctx context.Context, moduleSet bufmodule.ModuleSet) ([]bufimage.ImageFileInfo, error) { // Sorted. fileInfos, err := bufmodule.GetFileInfos( 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 8bd1baf0c6..5350bc49dc 100644 --- a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go +++ b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go @@ -26,7 +26,7 @@ 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" @@ -163,7 +163,7 @@ func checkField( if err != nil { return err } - constraints := resolver.DefaultResolver{}.ResolveFieldConstraints(fieldDescriptor) + constraints := resolve.FieldConstraints(fieldDescriptor) return checkConstraintsForField( &adder{ field: field, @@ -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}, @@ -799,62 +770,55 @@ func checkExampleValues( // and validate this message instance with protovalidate and filter the structured // 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, - } - } - // 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, - } - } - } - 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(): // 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": // 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": // 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 { + filterFieldDescriptor := fieldDescriptor messageToValidate := dynamicpb.NewMessage(containingMessageDescriptor) switch { case fieldDescriptor.IsList(): @@ -862,6 +826,10 @@ func checkExampleValues( list.Append(exampleValue) messageToValidate.Set(fieldDescriptor, protoreflect.ValueOfList(list)) case parentMapFieldDescriptor != nil: + // In the case of map fields, we need to filter for the parent field descriptor in the + // Filter function. The violationFilterFunc will filter to approrpriate violation for + // "key" and "value" example values. + filterFieldDescriptor = parentMapFieldDescriptor mapEntryMessageDescriptor := fieldDescriptor.ContainingMessage() // We are being very defensive, because a type mismatch may cause a panic in protoreflect. if !mapEntryMessageDescriptor.IsMapEntry() { @@ -1013,7 +981,16 @@ func checkExampleValues( default: messageToValidate.Set(fieldDescriptor, exampleValue) } - err := validator.Validate(messageToValidate) + err := validator.Validate(messageToValidate, + protovalidate.WithFilter( + protovalidate.FilterFunc(func( + _ protoreflect.Message, + d protoreflect.Descriptor, + ) bool { + return d == filterFieldDescriptor + }), + ), + ) if err == nil { continue } 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 9353f37b12..b8ce9fa698 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -627,11 +627,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]; }