more follow-up for editions in protoprint: expand test, fix issue unc…

…overed w/ custom sort order (#602)

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)
 }
 

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")

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

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;
+}

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;

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;
+}

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;
+}

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;
+}

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;
+}

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;
+}

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;
+}