various fixes to protoprint; ports over PRs #580, #582, and #584 from v1

Makefile

@@ -52,6 +52,7 @@ errcheck:
 .PHONY: test
 test:
 	go test -cover -race ./...
+	pushd protoprint/testfiles && ./check-protos.sh && popd
 
 .PHONY: generate
 generate:

internal/testdata/desc_test_complex.proto

@@ -30,7 +30,7 @@ message Test {
 
 	extensions 100 to 200;
 
-	extensions 249, 300 to 350, 500 to 550, 20000 to max [(label) = "jazz"];
+	extensions 249, 300 to 350, 500 to 550, 20000 to max [(label) = "jazz \"hands\""];
 
 	message Nested {
 		extend google.protobuf.MessageOptions {
@@ -273,7 +273,7 @@ extend google.protobuf.FieldOptions {
 message KeywordCollisionOptions {
 	optional uint64 id = 1 [
 		(syntax) = true, (import) = true, (public) = true, (weak) = true, (package) = true,
-		(string) = "string", (bytes) = "bytes", (bool) = true,
+		(string) = "string\u8765\U00107f6d\a\b\f\n\r\t\v\\\"\'\?\x42", (bytes) = "bytes\u8765\U00107f6d\a\b\f\n\r\t\v\\\"\'\?\x42", (bool) = true,
 		(float) = 3.14, (double) = 3.14159,
 		(int32) = 32, (int64) = 64, (uint32) = 3200, (uint64) = 6400, (sint32) = -32, (sint64) = -64,
 		(fixed32) = 3232, (fixed64) = 6464, (sfixed32) = -3232, (sfixed64) = -6464,

internal/testdata/desc_test_proto3.proto

@@ -9,6 +9,7 @@ import "pkg/desc_test_pkg.proto";
 
 enum Proto3Enum {
 	UNKNOWN = 0;
+	VALUE_NEG1 = -1;
 	VALUE1 = 1;
 	VALUE2 = 2;
 }

internal/testdata/make_protos.sh

@@ -47,6 +47,7 @@ ${PROTOC} --descriptor_set_out=./desc_test1.protoset --include_source_info --inc
 ${PROTOC} --descriptor_set_out=./desc_test_comments.protoset --include_source_info --include_imports -I. desc_test_comments.proto
 ${PROTOC} --descriptor_set_out=./desc_test_complex.protoset -I. desc_test_complex.proto
 ${PROTOC} --descriptor_set_out=./desc_test_complex_source_info.protoset --include_source_info --include_imports -I. desc_test_complex.proto
+${PROTOC} --descriptor_set_out=./desc_test_proto3.protoset --include_source_info --include_imports -I. desc_test_proto3.proto
 ${PROTOC} --descriptor_set_out=./descriptor.protoset --include_source_info --include_imports -I./protoc/include/ google/protobuf/descriptor.proto
 ${PROTOC} --descriptor_set_out=./duration.protoset -I./protoc/include/ google/protobuf/duration.proto
 ${PROTOC} --descriptor_set_out=./proto3_optional/desc_test_proto3_optional.protoset --include_source_info --include_imports -I. proto3_optional/desc_test_proto3_optional.proto

protoprint/print.go

@@ -3,6 +3,7 @@ package protoprint
 import (
 	"bytes"
 	"fmt"
+	"github.com/jhump/protoreflect/v2/protowrap"
 	"io"
 	"math"
 	"os"
@@ -693,6 +694,13 @@ func (p *Printer) typeString(fld protoreflect.FieldDescriptor, scope protoreflec
 	if fld.IsMap() {
 		return fmt.Sprintf("map<%s, %s>", p.typeString(fld.MapKey(), scope), p.typeString(fld.MapValue(), scope))
 	}
+	fldProto := protowrap.ProtoFromFieldDescriptor(fld)
+	if fldProto.Type == nil && fldProto.TypeName != nil {
+		// In an unlinked proto, the type may be absent because it is not known
+		// whether the symbol is a message or an enum. In that case, just return
+		// the type name.
+		return fldProto.GetTypeName()
+	}
 	switch fld.Kind() {
 	case protoreflect.EnumKind:
 		return p.qualifyName(fld.ParentFile().Package(), scope, fld.Enum().FullName())
@@ -2033,7 +2041,7 @@ func uninterpretedToOptions(uninterp []*descriptorpb.UninterpretedOption) []opti
 		case unint.NegativeIntValue != nil:
 			v = unint.GetNegativeIntValue()
 		case unint.AggregateValue != nil:
-			v = ident(unint.GetAggregateValue())
+			v = ident("{ " + unint.GetAggregateValue() + " }")
 		}
 
 		opts[i] = option{name: buf.String(), val: v}
@@ -2072,7 +2080,7 @@ func quotedBytes(s string) string {
 		case '\t':
 			b.WriteString("\\t")
 		case '"':
-			b.WriteString("\\")
+			b.WriteString("\\\"")
 		case '\\':
 			b.WriteString("\\\\")
 		default:
@@ -2118,7 +2126,7 @@ func quotedString(s string) string {
 		case '\t':
 			b.WriteString("\\t")
 		case '"':
-			b.WriteString("\\")
+			b.WriteString("\\\"")
 		case '\\':
 			b.WriteString("\\\\")
 		default:
@@ -2162,20 +2170,22 @@ func (a elementAddrs) Len() int {
 
 func (a elementAddrs) Less(i, j int) bool {
 	// explicit order is considered first
-	if a.addrs[i].order < a.addrs[j].order {
+	addri := a.addrs[i]
+	addrj := a.addrs[j]
+	if addri.order < addrj.order {
 		return true
-	} else if a.addrs[i].order > a.addrs[j].order {
+	} else if addri.order > addrj.order {
 		return false
 	}
 	// if order is equal, sort by element type
-	if a.addrs[i].elementType < a.addrs[j].elementType {
+	if addri.elementType < addrj.elementType {
 		return true
-	} else if a.addrs[i].elementType > a.addrs[j].elementType {
+	} else if addri.elementType > addrj.elementType {
 		return false
 	}
 
-	di := a.at(a.addrs[i])
-	dj := a.at(a.addrs[j])
+	di := a.at(addri)
+	dj := a.at(addrj)
 
 	switch vi := di.(type) {
 	case protoreflect.FieldDescriptor:
@@ -2194,11 +2204,20 @@ func (a elementAddrs) Less(i, j int) bool {
 		return vi.Number() < vj.Number()
 
 	case protoreflect.EnumValueDescriptor:
-		// enum values ordered by number then name
+		// enum values ordered by number then name,
+		// but first value number must be 0 in proto3
 		vj := dj.(protoreflect.EnumValueDescriptor)
 		if vi.Number() == vj.Number() {
 			return vi.Name() < vj.Name()
 		}
+		if vi.ParentFile().Syntax() == protoreflect.Proto3 {
+			if vi.Number() == 0 {
+				return true
+			}
+			if vj.Number() == 0 {
+				return false
+			}
+		}
 		return vi.Number() < vj.Number()
 
 	case extensionRange:
@@ -2458,8 +2477,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.(protoreflect.EnumValueDescriptor); ok {
+			vj := dj.(protoreflect.EnumValueDescriptor)
+			if vi.ParentFile().Syntax() == protoreflect.Proto3 {
+				if vi.Number() == 0 {
+					return true
+				}
+				if vj.Number() == 0 {
+					return false
+				}
+			}
+		}
+	}
+
+	ei := asElement(di)
+	ej := asElement(dj)
 	return cso.less(ei, ej)
 }