-
Notifications
You must be signed in to change notification settings - Fork 20
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
Tolerate missing field tags #246
Conversation
Error: `syntax = "proto2"; | ||
message Foo { | ||
int32 bar = 1 []; | ||
optional int32 bar = 1 []; | ||
}`, | ||
NoError: `syntax = "proto3"; | ||
NoError: `syntax = "proto2"; | ||
message Foo { | ||
int32 bar = 1 [default=1]; | ||
optional int32 bar = 1 [default=1]; |
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.
If you want to preserve these testing a field declaration w/ no label, you could instead change the option from the proto3-disallowed default
to something like json_name="foo"
.
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.
I beleive label doesn't actually change the test coverage, it is testing the compact option rules only
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.
Cool 👍
How do I turn the more lenient parsing off? There should be an option (for this, and anything else that was made more lenient), so that |
@bufdev, you don't need an option for that. All compile operations do the validation -- it's just been moved from the parse stage to the descriptor production/validation stage. |
While still returning an error. See #200
Note that the error occurs during validation instead of parsing to eventually allow algorithmic field tag assignments.