Add editions support to protoprint (#600)

The protoprint package doesn't support editions yet, but protoprint now does.

desc/protoparse/ast_roundtrip_test.go

@@ -16,6 +16,10 @@ func TestASTRoundTrips(t *testing.T) {
 		if err != nil {
 			return err
 		}
+		if filepath.Base(path) == "desc_test_editions.proto" {
+			// The AST doesn't support editions yet
+			return nil
+		}
 		if filepath.Ext(path) == ".proto" {
 			t.Run(path, func(t *testing.T) {
 				b, err := ioutil.ReadFile(path)

desc/protoprint/print.go

@@ -532,6 +532,10 @@ func (p *Printer) printFile(fd *desc.FileDescriptor, reg *protoregistry.Types, w
 	si := sourceInfo.Get(path)
 	p.printElement(false, si, w, 0, func(w *writer) {
 		syn := fdp.GetSyntax()
+		if syn == "editions" {
+			_, _ = fmt.Fprintf(w, "edition = %q;", strings.TrimPrefix(fdp.GetEdition().String(), "EDITION_"))
+			return
+		}
 		if syn == "" {
 			syn = "proto2"
 		}
@@ -984,7 +988,7 @@ func (p *Printer) printMessageBody(md *desc.MessageDescriptor, reg *protoregistr
 				addrs = append(addrs, elnext)
 				skip[rn] = true
 			}
-			p.printReservedNames(names, addrs, w, sourceInfo, path, indent)
+			p.printReservedNames(names, addrs, w, sourceInfo, path, indent, useQuotedReserved(md.GetFile()))
 		}
 	}
 }
@@ -1063,7 +1067,7 @@ func (p *Printer) printField(fld *desc.FieldDescriptor, reg *protoregistry.Types
 		// we use negative values for "extras" keys so they can't collide
 		// with legit option tags
 
-		if !fld.GetFile().IsProto3() && fld.AsFieldDescriptorProto().DefaultValue != nil {
+		if fld.UnwrapField().HasPresence() && fld.AsFieldDescriptorProto().DefaultValue != nil {
 			defVal := fld.GetDefaultValue()
 			if fld.GetEnumType() != nil {
 				defVal = ident(fld.GetEnumType().FindValueByNumber(defVal.(int32)).GetName())
@@ -1097,7 +1101,8 @@ func (p *Printer) printField(fld *desc.FieldDescriptor, reg *protoregistry.Types
 func shouldEmitLabel(fld *desc.FieldDescriptor) bool {
 	return fld.IsProto3Optional() ||
 		(!fld.IsMap() && fld.GetOneOf() == nil &&
-			(fld.GetLabel() != descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL || !fld.GetFile().IsProto3()))
+			(fld.GetLabel() != descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL ||
+				fld.GetFile().UnwrapFile().Syntax() == protoreflect.Proto2))
 }
 
 func labelString(lbl descriptorpb.FieldDescriptorProto_Label) string {
@@ -1276,7 +1281,11 @@ func (p *Printer) printReservedRanges(ranges []reservedRange, maxVal int32, addr
 	_, _ = fmt.Fprintln(w, ";")
 }
 
-func (p *Printer) printReservedNames(names []string, addrs []elementAddr, w *writer, sourceInfo internal.SourceInfoMap, parentPath []int32, indent int) {
+func useQuotedReserved(fd *desc.FileDescriptor) bool {
+	return fd.AsFileDescriptorProto().GetEdition() < descriptorpb.Edition_EDITION_2023
+}
+
+func (p *Printer) printReservedNames(names []string, addrs []elementAddr, w *writer, sourceInfo internal.SourceInfoMap, parentPath []int32, indent int, useQuotes bool) {
 	p.indent(w, indent)
 	_, _ = fmt.Fprint(w, "reserved ")
 
@@ -1289,7 +1298,11 @@ func (p *Printer) printReservedNames(names []string, addrs []elementAddr, w *wri
 		}
 		el := addrs[i]
 		si := sourceInfo.Get(append(parentPath, el.elementType, int32(el.elementIndex)))
-		p.printElementString(si, w, indent, quotedString(name))
+		if useQuotes {
+			p.printElementString(si, w, indent, quotedString(name))
+		} else {
+			p.printElementString(si, w, indent, name)
+		}
 	}
 
 	_, _ = fmt.Fprintln(w, ";")
@@ -1381,7 +1394,7 @@ func (p *Printer) printEnum(ed *desc.EnumDescriptor, reg *protoregistry.Types, w
 					addrs = append(addrs, elnext)
 					skip[rn] = true
 				}
-				p.printReservedNames(names, addrs, w, sourceInfo, path, indent)
+				p.printReservedNames(names, addrs, w, sourceInfo, path, indent, useQuotedReserved(ed.GetFile()))
 			}
 		}
 
@@ -2145,12 +2158,12 @@ func (a elementAddrs) Less(i, j int) bool {
 
 	case *desc.EnumValueDescriptor:
 		// enum values ordered by number then name,
-		// but first value number must be 0 in proto3
+		// but first value number must be 0 for open enums
 		vj := dj.(*desc.EnumValueDescriptor)
 		if vi.GetNumber() == vj.GetNumber() {
 			return vi.GetName() < vj.GetName()
 		}
-		if vi.GetFile().IsProto3() {
+		if !vi.GetEnum().UnwrapEnum().IsClosed() {
 			if vj.GetNumber() == 0 {
 				return false
 			}

desc/protoprint/print_test.go

@@ -227,6 +227,13 @@ 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.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/desc_test_editions.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/descriptor-compact.proto

@@ -64,13 +64,18 @@ enum Edition {
   // should not be depended on, but they will always be time-ordered for easy
   // comparison.
   EDITION_2023 = 1000;
+  EDITION_2024 = 1001;
   // Placeholder editions for testing feature resolution.  These should not be
   // used or relyed on outside of tests.
   EDITION_1_TEST_ONLY = 1;
   EDITION_2_TEST_ONLY = 2;
   EDITION_99997_TEST_ONLY = 99997;
   EDITION_99998_TEST_ONLY = 99998;
   EDITION_99999_TEST_ONLY = 99999;
+  // Placeholder for specifying unbounded edition support.  This should only
+  // ever be used by plugins that can expect to never require any changes to
+  // support a new edition.
+  EDITION_MAX = 2147483647;
 }
 // Describes a complete .proto file.
 message FileDescriptorProto {
@@ -166,7 +171,7 @@ message ExtensionRangeOptions {
   // The verification state of the range.
   // TODO: flip the default to DECLARATION once all empty ranges
   // are marked as UNVERIFIED.
-  optional VerificationState verification = 3 [default = UNVERIFIED];
+  optional VerificationState verification = 3 [default = UNVERIFIED, retention = RETENTION_SOURCE];
   extensions 1000 to max;
 }
 // Describes a field within a message.
@@ -244,12 +249,12 @@ message FieldDescriptorProto {
   // If true, this is a proto3 "optional". When a proto3 field is optional, it
   // tracks presence regardless of field type.
   //
-  // When proto3_optional is true, this field must be belong to a oneof to
-  // signal to old proto3 clients that presence is tracked for this field. This
-  // oneof is known as a "synthetic" oneof, and this field must be its sole
-  // member (each proto3 optional field gets its own synthetic oneof). Synthetic
-  // oneofs exist in the descriptor only, and do not generate any API. Synthetic
-  // oneofs must be ordered after all "real" oneofs.
+  // When proto3_optional is true, this field must belong to a oneof to signal
+  // to old proto3 clients that presence is tracked for this field. This oneof
+  // is known as a "synthetic" oneof, and this field must be its sole member
+  // (each proto3 optional field gets its own synthetic oneof). Synthetic oneofs
+  // exist in the descriptor only, and do not generate any API. Synthetic oneofs
+  // must be ordered after all "real" oneofs.
   //
   // For message fields, proto3_optional doesn't create any semantic change,
   // since non-repeated message fields always track presence. However it still
@@ -402,7 +407,7 @@ message FileOptions {
   optional bool cc_generic_services = 16 [default = false];
   optional bool java_generic_services = 17 [default = false];
   optional bool py_generic_services = 18 [default = false];
-  optional bool php_generic_services = 42 [default = false];
+  reserved 42;
   // Is this file deprecated?
   // Depending on the target platform, this can emit Deprecated annotations
   // for everything in the file, or it will be completely ignored; in the very
@@ -474,10 +479,6 @@ message MessageOptions {
   // this is a formalization for deprecating messages.
   optional bool deprecated = 3 [default = false];
   reserved 4, 5, 6;
-  // NOTE: Do not set the option in .proto files. Always use the maps syntax
-  // instead. The option should only be implicitly set by the proto compiler
-  // parser.
-  //
   // Whether the message is an automatically generated map entry type for the
   // maps field.
   //
@@ -495,6 +496,10 @@ message MessageOptions {
   // use a native map in the target language to hold the keys and values.
   // The reflection APIs in such implementations still need to work as
   // if the field is a repeated message field.
+  //
+  // NOTE: Do not set the option in .proto files. Always use the maps syntax
+  // instead. The option should only be implicitly set by the proto compiler
+  // parser.
   optional bool map_entry = 7;
   reserved 8, 9;
   // Enable the legacy handling of JSON field name conflicts.  This lowercases
@@ -579,19 +584,11 @@ message FieldOptions {
   // call from multiple threads concurrently, while non-const methods continue
   // to require exclusive access.
   //
-  // Note that implementations may choose not to check required fields within
-  // a lazy sub-message.  That is, calling IsInitialized() on the outer message
-  // may return true even if the inner message has missing required fields.
-  // This is necessary because otherwise the inner message would have to be
-  // parsed in order to perform the check, defeating the purpose of lazy
-  // parsing.  An implementation which chooses not to check required fields
-  // must be consistent about it.  That is, for any particular sub-message, the
-  // implementation must either *always* check its required fields, or *never*
-  // check its required fields, regardless of whether or not the message has
-  // been parsed.
-  //
-  // As of May 2022, lazy verifies the contents of the byte stream during
-  // parsing.  An invalid byte stream will cause the overall parsing to fail.
+  // Note that lazy message fields are still eagerly verified to check
+  // ill-formed wireformat or missing required fields. Calling IsInitialized()
+  // on the outer message would fail if the inner message has missing required
+  // fields. Failed verification would result in parsing failure (except when
+  // uninitialized messages are acceptable).
   optional bool lazy = 5 [default = false];
   // unverified_lazy does no correctness checks on the byte stream. This should
   // only be used where lazy with verification is prohibitive for performance
@@ -807,8 +804,8 @@ message FeatureSet {
   ];
   enum Utf8Validation {
     UTF8_VALIDATION_UNKNOWN = 0;
-    NONE = 1;
     VERIFY = 2;
+    NONE = 3;
   }
   optional Utf8Validation utf8_validation = 4 [
     retention = RETENTION_RUNTIME,
@@ -842,7 +839,7 @@ message FeatureSet {
     edition_defaults = { value: "ALLOW", edition: EDITION_PROTO3 }
   ];
   reserved 999;
-  extensions 1000, 1001, 9995 to 9999;
+  extensions 1000, 1001, 1002, 9995 to 9999, 10000;
 }
 // A compiled specification for the defaults of a set of features.  These
 // messages are generated from FeatureSet extensions and can be used to seed
@@ -919,7 +916,7 @@ message SourceCodeInfo {
     // location.
     //
     // Each element is a field number or an index.  They form a path from
-    // the root FileDescriptorProto to the place where the definition occurs.
+    // the root FileDescriptorProto to the place where the definition appears.
     // For example, this path:
     //   [ 4, 3, 2, 7, 1 ]
     // refers to: