Update to match Editions behavior of latest protoc (v25.0) #191
Conversation
…dition and prevent use of packed field in editions
…tures are disallowed unless using editions
…s removed in 528a61d because now we need it again for validation
| @@ -6,7 +6,7 @@ require ( | |||
| github.com/google/go-cmp v0.5.9 | |||
| github.com/stretchr/testify v1.8.4 | |||
| golang.org/x/sync v0.3.0 | |||
| google.golang.org/protobuf v1.31.1-0.20230727123859-6d0a5dbd9500 | |||
| google.golang.org/protobuf v1.31.1-0.20231027082548-f4a6c1f6e5c1 | |||
There was a problem hiding this comment.
This latest version of protobuf-go pulls in the latest descriptor.proto changes.
| @@ -2041,44 +2038,6 @@ func TestLinkerValidation(t *testing.T) { | |||
| }, | |||
| expectedErr: `test.proto:1:11: edition value "2022" not recognized; should be one of ["2023"]`, | |||
| }, | |||
| "failure_use_of_features_without_editions": { | |||
There was a problem hiding this comment.
This moved into the parser's descriptor validation, so is done in parser tests instead of in linker tests.
| }, | ||
| expectedErr: `test.proto:3:25: field Foo.foo: option 'features' may only be used with editions but file uses "proto3" syntax`, | ||
| }, | ||
| "failure_direct_use_of_raw_features": { |
There was a problem hiding this comment.
There is no longer a raw_features field in google.protobuf.FeatureSet.
| md.Options.UninterpretedOption = internal.RemoveOption(md.Options.UninterpretedOption, index) | ||
| valid := false | ||
| if opt.IdentifierValue != nil { | ||
| if opt.GetIdentifierValue() == "true" { |
There was a problem hiding this comment.
Previously, this would allow an explicit map_entry option as long as the value was false. But the latest version of protoc doesn't allow this at all, so we mirror its behavior.
| @@ -272,9 +272,8 @@ func TestBasicValidation(t *testing.T) { | |||
| contents: `message Foo { reserved 1, 2 to 10, 11 to 20; extensions 21 to 22; }`, | |||
| }, | |||
| "failure_message_reserved_start_after_end": { | |||
| contents: `message Foo { reserved 10 to 1; }`, | |||
| expectedErr: `test.proto:1:24: range, 10 to 1, is invalid: start must be <= end`, | |||
| expectedDiffWithProtoc: true, // bug in protoc: https://github.com/protocolbuffers/protobuf/issues/13442 | |||
There was a problem hiding this comment.
Numerous bugs have been fixed in protoc v25. Yay! 💥
| return err | ||
| } | ||
| if featuresDefaults != nil && nmd.GetOptions().GetMapEntry() { |
There was a problem hiding this comment.
This was moved to a post-process below. It was a little simpler that way.
jhump
changed the title
Between v24 and v25, the implementation or editions inside of
protocchanged quite rather drastically. Most of the complexity having to do with computation of feature defaults and feature set resolution has been completely removed 🎉.There are some changes to exported symbols made in this branch, like removal of the exported
options.FeaturesResolverCache. But this is technically not a breaking change because those symbols were never included in a released version. So this branch is fully backwards-compatible with the previous release (though not the previous commit).It's a large-ish change, so it might be easier to digest if reviewed commit-by-commit.
This still does not perform some of the checks for field features that
protocperforms, so that still needs to come in a subsequent branch. But this does bring this repo up to the same level ofprotoc-compatibility it had before, but this time withprotocv25 (instead of v24). It also does add one add'l check: that thepackedfield option is not used with editions.