From 8d1f661155e028a739edc575316e2e53160e32c8 Mon Sep 17 00:00:00 2001 From: Joshua Humphries <2035234+jhump@users.noreply.github.com> Date: Sat, 6 Apr 2024 08:44:03 -0400 Subject: [PATCH] more follow-up for editions in protoprint: expand test, fix issue uncovered w/ custom sort order (#602) --- desc/protoprint/print.go | 26 +++++++++++- desc/protoprint/print_test.go | 8 +--- desc/protoprint/testfiles/check-protos.sh | 4 +- .../desc_test_editions-compact.proto | 25 +++++++++++ .../desc_test_editions-custom-sort.proto | 41 +++++++++++++++++++ ...st_editions-multiline-style-comments.proto | 41 +++++++++++++++++++ ...c_test_editions-no-trailing-comments.proto | 41 +++++++++++++++++++ ...desc_test_editions-only-doc-comments.proto | 41 +++++++++++++++++++ ...-sorted-AND-multiline-style-comments.proto | 41 +++++++++++++++++++ .../testfiles/desc_test_editions-sorted.proto | 41 +++++++++++++++++++ ..._test_editions-trailing-on-next-line.proto | 41 +++++++++++++++++++ 11 files changed, 338 insertions(+), 12 deletions(-) create mode 100644 desc/protoprint/testfiles/desc_test_editions-compact.proto create mode 100644 desc/protoprint/testfiles/desc_test_editions-custom-sort.proto create mode 100644 desc/protoprint/testfiles/desc_test_editions-multiline-style-comments.proto create mode 100644 desc/protoprint/testfiles/desc_test_editions-no-trailing-comments.proto create mode 100644 desc/protoprint/testfiles/desc_test_editions-only-doc-comments.proto create mode 100644 desc/protoprint/testfiles/desc_test_editions-sorted-AND-multiline-style-comments.proto create mode 100644 desc/protoprint/testfiles/desc_test_editions-sorted.proto create mode 100644 desc/protoprint/testfiles/desc_test_editions-trailing-on-next-line.proto diff --git a/desc/protoprint/print.go b/desc/protoprint/print.go index 1b570a9b..01979640 100644 --- a/desc/protoprint/print.go +++ b/desc/protoprint/print.go @@ -2419,8 +2419,30 @@ type customSortOrder struct { } func (cso customSortOrder) Less(i, j int) bool { - ei := asElement(cso.at(cso.addrs[i])) - ej := asElement(cso.at(cso.addrs[j])) + // Regardless of the custom sort order, for proto3 files, + // the enum value zero MUST be first. So we override the + // custom sort order to make sure the file will be valid + // and can compile. + addri := cso.addrs[i] + addrj := cso.addrs[j] + di := cso.at(addri) + dj := cso.at(addrj) + if addri.elementType == addrj.elementType { + if vi, ok := di.(*desc.EnumValueDescriptor); ok { + vj := dj.(*desc.EnumValueDescriptor) + if !vi.GetEnum().UnwrapEnum().IsClosed() { + if vi.GetNumber() == 0 { + return true + } + if vj.GetNumber() == 0 { + return false + } + } + } + } + + ei := asElement(di) + ej := asElement(dj) return cso.less(ei, ej) } diff --git a/desc/protoprint/print_test.go b/desc/protoprint/print_test.go index 010bdbe1..a73124b5 100644 --- a/desc/protoprint/print_test.go +++ b/desc/protoprint/print_test.go @@ -77,6 +77,7 @@ func TestPrinter(t *testing.T) { files := []string{ "../../internal/testprotos/desc_test_comments.protoset", "../../internal/testprotos/desc_test_complex_source_info.protoset", + "../../internal/testprotos/desc_test_editions.protoset", "../../internal/testprotos/descriptor.protoset", "../../internal/testprotos/desc_test1.protoset", "../../internal/testprotos/proto3_optional/desc_test_proto3_optional.protoset", @@ -227,13 +228,6 @@ message SomeMessage { checkFile(t, &Printer{}, fd, "test-uninterpreted-options.proto") } -func TestPrintEditions(t *testing.T) { - fd, err := loadProtoset("../../internal/testprotos/desc_test_editions.protoset") - testutil.Ok(t, err) - - checkFile(t, &Printer{}, fd, "desc_test_editions-default.proto") -} - func TestPrintNonFileDescriptors(t *testing.T) { pa := protoparse.Parser{ImportPaths: []string{"../../internal/testprotos"}, IncludeSourceCodeInfo: true} fds, err := pa.ParseFiles("desc_test_comments.proto") diff --git a/desc/protoprint/testfiles/check-protos.sh b/desc/protoprint/testfiles/check-protos.sh index 27348d87..88a437cd 100755 --- a/desc/protoprint/testfiles/check-protos.sh +++ b/desc/protoprint/testfiles/check-protos.sh @@ -6,8 +6,6 @@ cd $(dirname $0) for f in *.proto; do echo -n "Checking $f..." - ../../../internal/testprotos/protoc/bin/protoc $f --experimental_editions -o tmp.protoset -I ../../../internal/testprotos -I . + ../../../internal/testprotos/protoc/bin/protoc $f --experimental_editions -o /dev/null -I ../../../internal/testprotos -I . echo " good" done - -rm tmp.protoset diff --git a/desc/protoprint/testfiles/desc_test_editions-compact.proto b/desc/protoprint/testfiles/desc_test_editions-compact.proto new file mode 100644 index 00000000..c353710f --- /dev/null +++ b/desc/protoprint/testfiles/desc_test_editions-compact.proto @@ -0,0 +1,25 @@ +edition = "2023"; +package testprotos; +option features = { enum_type: CLOSED }; +option go_package = "github.com/jhump/protoreflect/internal/testprotos"; +message Foo { + reserved reserved_field; + int32 a = 1; + int32 required_field = 2 [features = { field_presence: LEGACY_REQUIRED }]; + int32 default_field = 3 [default = 99]; + DelimitedField delimitedfield = 4 [features = { message_encoding: DELIMITED }]; + message DelimitedField { + int32 b = 1; + } +} +enum Closed { + CLOSED_C = 1; + CLOSED_A = 2; + reserved CLOSED_E, CLOSED_F; +} +enum Open { + option features = { enum_type: OPEN }; + OPEN_B = 0; + OPEN_C = -1; + OPEN_A = 2; +} diff --git a/desc/protoprint/testfiles/desc_test_editions-custom-sort.proto b/desc/protoprint/testfiles/desc_test_editions-custom-sort.proto new file mode 100644 index 00000000..b34a3dc9 --- /dev/null +++ b/desc/protoprint/testfiles/desc_test_editions-custom-sort.proto @@ -0,0 +1,41 @@ +edition = "2023"; + +enum Open { + OPEN_B = 0; + + OPEN_C = -1; + + OPEN_A = 2; + + option features = { enum_type: OPEN }; +} + +enum Closed { + CLOSED_C = 1; + + CLOSED_A = 2; + + reserved CLOSED_F, CLOSED_E; +} + +message Foo { + reserved reserved_field; + + message DelimitedField { + int32 b = 1; + } + + int32 required_field = 2 [features = { field_presence: LEGACY_REQUIRED }]; + + DelimitedField delimitedfield = 4 [features = { message_encoding: DELIMITED }]; + + int32 default_field = 3 [default = 99]; + + int32 a = 1; +} + +option go_package = "github.com/jhump/protoreflect/internal/testprotos"; + +option features = { enum_type: CLOSED }; + +package testprotos; diff --git a/desc/protoprint/testfiles/desc_test_editions-multiline-style-comments.proto b/desc/protoprint/testfiles/desc_test_editions-multiline-style-comments.proto new file mode 100644 index 00000000..30e1cf75 --- /dev/null +++ b/desc/protoprint/testfiles/desc_test_editions-multiline-style-comments.proto @@ -0,0 +1,41 @@ +edition = "2023"; + +package testprotos; + +option features = { enum_type: CLOSED }; + +option go_package = "github.com/jhump/protoreflect/internal/testprotos"; + +message Foo { + reserved reserved_field; + + int32 a = 1; + + int32 required_field = 2 [features = { field_presence: LEGACY_REQUIRED }]; + + int32 default_field = 3 [default = 99]; + + DelimitedField delimitedfield = 4 [features = { message_encoding: DELIMITED }]; + + message DelimitedField { + int32 b = 1; + } +} + +enum Closed { + CLOSED_C = 1; + + CLOSED_A = 2; + + reserved CLOSED_E, CLOSED_F; +} + +enum Open { + option features = { enum_type: OPEN }; + + OPEN_B = 0; + + OPEN_C = -1; + + OPEN_A = 2; +} diff --git a/desc/protoprint/testfiles/desc_test_editions-no-trailing-comments.proto b/desc/protoprint/testfiles/desc_test_editions-no-trailing-comments.proto new file mode 100644 index 00000000..f746ce25 --- /dev/null +++ b/desc/protoprint/testfiles/desc_test_editions-no-trailing-comments.proto @@ -0,0 +1,41 @@ +edition = "2023"; + +package testprotos; + +option features = { enum_type: CLOSED }; + +option go_package = "github.com/jhump/protoreflect/internal/testprotos"; + +message Foo { + reserved reserved_field; + + int32 a = 1; + + int32 required_field = 2 [features = { field_presence: LEGACY_REQUIRED }]; + + int32 default_field = 3 [default = 99]; + + DelimitedField delimitedfield = 4 [features = { message_encoding: DELIMITED }]; + + message DelimitedField { + int32 b = 1; + } +} + +enum Closed { + CLOSED_C = 1; + + CLOSED_A = 2; + + reserved CLOSED_E, CLOSED_F; +} + +enum Open { + option features = { enum_type: OPEN }; + + OPEN_B = 0; + + OPEN_C = -1; + + OPEN_A = 2; +} diff --git a/desc/protoprint/testfiles/desc_test_editions-only-doc-comments.proto b/desc/protoprint/testfiles/desc_test_editions-only-doc-comments.proto new file mode 100644 index 00000000..f746ce25 --- /dev/null +++ b/desc/protoprint/testfiles/desc_test_editions-only-doc-comments.proto @@ -0,0 +1,41 @@ +edition = "2023"; + +package testprotos; + +option features = { enum_type: CLOSED }; + +option go_package = "github.com/jhump/protoreflect/internal/testprotos"; + +message Foo { + reserved reserved_field; + + int32 a = 1; + + int32 required_field = 2 [features = { field_presence: LEGACY_REQUIRED }]; + + int32 default_field = 3 [default = 99]; + + DelimitedField delimitedfield = 4 [features = { message_encoding: DELIMITED }]; + + message DelimitedField { + int32 b = 1; + } +} + +enum Closed { + CLOSED_C = 1; + + CLOSED_A = 2; + + reserved CLOSED_E, CLOSED_F; +} + +enum Open { + option features = { enum_type: OPEN }; + + OPEN_B = 0; + + OPEN_C = -1; + + OPEN_A = 2; +} diff --git a/desc/protoprint/testfiles/desc_test_editions-sorted-AND-multiline-style-comments.proto b/desc/protoprint/testfiles/desc_test_editions-sorted-AND-multiline-style-comments.proto new file mode 100644 index 00000000..93b08bee --- /dev/null +++ b/desc/protoprint/testfiles/desc_test_editions-sorted-AND-multiline-style-comments.proto @@ -0,0 +1,41 @@ +edition = "2023"; + +package testprotos; + +option features = { enum_type: CLOSED }; + +option go_package = "github.com/jhump/protoreflect/internal/testprotos"; + +message Foo { + int32 a = 1; + + int32 required_field = 2 [features = { field_presence: LEGACY_REQUIRED }]; + + int32 default_field = 3 [default = 99]; + + DelimitedField delimitedfield = 4 [features = { message_encoding: DELIMITED }]; + + message DelimitedField { + int32 b = 1; + } + + reserved reserved_field; +} + +enum Closed { + CLOSED_C = 1; + + CLOSED_A = 2; + + reserved CLOSED_E, CLOSED_F; +} + +enum Open { + option features = { enum_type: OPEN }; + + OPEN_B = 0; + + OPEN_C = -1; + + OPEN_A = 2; +} diff --git a/desc/protoprint/testfiles/desc_test_editions-sorted.proto b/desc/protoprint/testfiles/desc_test_editions-sorted.proto new file mode 100644 index 00000000..8347d006 --- /dev/null +++ b/desc/protoprint/testfiles/desc_test_editions-sorted.proto @@ -0,0 +1,41 @@ +edition = "2023"; + +package testprotos; + +option features = { enum_type: CLOSED }; + +option go_package = "github.com/jhump/protoreflect/internal/testprotos"; + +message Foo { + int32 a = 1; + + int32 required_field = 2 [features = { field_presence: LEGACY_REQUIRED }]; + + int32 default_field = 3 [default = 99]; + + DelimitedField delimitedfield = 4 [features = { message_encoding: DELIMITED }]; + + message DelimitedField { + int32 b = 1; + } + + reserved reserved_field; +} + +enum Closed { + CLOSED_C = 1; + + CLOSED_A = 2; + + reserved CLOSED_E, CLOSED_F; +} + +enum Open { + option features = { enum_type: OPEN }; + + OPEN_B = 0; + + OPEN_C = -1; + + OPEN_A = 2; +} diff --git a/desc/protoprint/testfiles/desc_test_editions-trailing-on-next-line.proto b/desc/protoprint/testfiles/desc_test_editions-trailing-on-next-line.proto new file mode 100644 index 00000000..f746ce25 --- /dev/null +++ b/desc/protoprint/testfiles/desc_test_editions-trailing-on-next-line.proto @@ -0,0 +1,41 @@ +edition = "2023"; + +package testprotos; + +option features = { enum_type: CLOSED }; + +option go_package = "github.com/jhump/protoreflect/internal/testprotos"; + +message Foo { + reserved reserved_field; + + int32 a = 1; + + int32 required_field = 2 [features = { field_presence: LEGACY_REQUIRED }]; + + int32 default_field = 3 [default = 99]; + + DelimitedField delimitedfield = 4 [features = { message_encoding: DELIMITED }]; + + message DelimitedField { + int32 b = 1; + } +} + +enum Closed { + CLOSED_C = 1; + + CLOSED_A = 2; + + reserved CLOSED_E, CLOSED_F; +} + +enum Open { + option features = { enum_type: OPEN }; + + OPEN_B = 0; + + OPEN_C = -1; + + OPEN_A = 2; +}