-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add structured field and rule paths to Violation #265
Conversation
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
I finished adding |
… jchadwick/add-rule-path
JH realized that our idea for handling the unknown map key types was not quite enough so I made a slight tweak: now |
// value inside unknown fields through wire data. | ||
optional google.protobuf.FieldDescriptorProto.Type value_type = 5; | ||
|
||
// `subscript` contains a repeated index or map key, if this path element nests into a repeated or map field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repeated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FieldPathElement
is meant to be logically equivalent to a path segment, e.g. repeated[1]
or map["key"]
. The subscript contains either a repeated field index or a map key. If this wording is confusing, I'm happy to update the documentation.
WDYT about this? (would "list index" be better?)
// `subscript` contains a repeated index or map key, if this path element nests into a repeated or map field. | |
// `subscript` contains a repeated field index or map key, if this path element nests into a repeated or map field. |
Implements the changes necessary to propagate field and rule paths in Violation messages, passing conformance. Depends on bufbuild/protovalidate#265. DO NOT MERGE until the following is done: - [x] bufbuild/protovalidate#265 is merged - [x] Dependencies updated to stable version release
Implements the changes needed to pass conformance after bufbuild/protovalidate#265. DO NOT MERGE until the following is done: - [x] bufbuild/protovalidate#265 is merged - [x] Dependencies updated to stable version release
This PR introduces a new structured field path format, and uses it to provide a structured path to the field and rule of a violation.
buf.validate.FieldPathElement
is added.repeated_field[1]
buf.validate.FieldPath
is added. It just contains a repeated field ofbuf.validate.FieldPathElement
repeated buf.validate.FieldPathElement
anywhere a path is needed to save a level of pointer chasing, but it is inconvenient for certain uses, e.g. comparing paths withproto.Equal
.buf.validate.Violation
fields are added:field
andrule
, both of typebuf.validate.FieldPath
. The oldfield_path
field is left for now, but deprecated.Note that there are a number of very subtle edge cases:
FieldConstraints
message. (In other cases,constraint_id
is always sufficient anyways, but we can change this behavior later.)Implementations: