prost-reflect
prost-reflect copied to clipboard
andrewhickman
Validation of FileDescriptorSets
Currently FileDescriptor::new performs some sanity checking on the FileDescriptorSet, for example checking that all referenced types can be resolved. However there are many checks that protoc does and we do not.
Known error cases which we could check for but currently don't:
- [ ] Invalid message/enum reserved ranges (invalid field number, start greater than end, overlapping ranges)
- [ ] Use of reserved message field/enum value names
- [ ] Re-use of the same field number for multiple extensions to the same message
- [ ] Extending a non-options type in proto3
- [ ] Using a proto2 enum in a proto3 message (#44)
- [x] Use of message/enum names which are defined but not imported into the currently file (either directly or transitively through
import public
I found that some of the work you've done on this item has caused the 0.10.x revisions to break for my use case. The validation fails on DuplicateFieldJsonName being the empty string.
DuplicateFieldJsonName { name: "", first: Label { file: "google/protobuf/descriptor.proto", path: [4, 1, 2, 0, 1], span: Some([60, 18, 60, 22]) }, second: Label { file: "google/protobuf/descriptor.proto", path: [4, 1, 2, 1, 1], span: Some([61, 18, 61, 25]) } }
It's a huge spam of seemingly every field in the proto. My understanding of the spec is that if a JSON field name is not specified it should default to the proto field name lowerCamelCase.
@mrpopogod How are you generating the FileDescriptorSet? With protoc 3.19.1, it sets the json_name field in the descriptor even if the name isn't set explicitly in the .proto file
In my case it's being generated by a schema registry owned by another team; you pass in a plaintext file and it will generate the FileDescriptorSet itself. I want to say it might still be proto2, but with no support for required fields (those get validated out).
@mrpopogod I've published 0.10.2 which should resolve this by setting the json_name to the camel-cased field name if it isn't set
Awesome, no longer getting the build error and my generated .rs files look good. Appreciate the fix.