Remove canonical bytes feature

internal/benchmarks/benchmark_test.go

@@ -46,7 +46,6 @@ import (
 	"github.com/bufbuild/protocompile"
 	"github.com/bufbuild/protocompile/ast"
 	"github.com/bufbuild/protocompile/internal/protoc"
-	"github.com/bufbuild/protocompile/linker"
 	"github.com/bufbuild/protocompile/parser"
 	"github.com/bufbuild/protocompile/parser/fastscan"
 	"github.com/bufbuild/protocompile/protoutil"
@@ -235,7 +234,7 @@ func downloadAndExpand(url, targetDir string) (e error) {
 }
 
 func BenchmarkGoogleapisProtocompile(b *testing.B) {
-	benchmarkGoogleapisProtocompile(b, false, func() *protocompile.Compiler {
+	benchmarkGoogleapisProtocompile(b, func() *protocompile.Compiler {
 		return &protocompile.Compiler{
 			Resolver: protocompile.WithStandardImports(&protocompile.SourceResolver{
 				ImportPaths: []string{googleapisDir},
@@ -246,20 +245,8 @@ func BenchmarkGoogleapisProtocompile(b *testing.B) {
 	})
 }
 
-func BenchmarkGoogleapisProtocompileCanonical(b *testing.B) {
-	benchmarkGoogleapisProtocompile(b, true, func() *protocompile.Compiler {
-		return &protocompile.Compiler{
-			Resolver: protocompile.WithStandardImports(&protocompile.SourceResolver{
-				ImportPaths: []string{googleapisDir},
-			}),
-			SourceInfoMode: protocompile.SourceInfoStandard,
-			// leave MaxParallelism unset to let it use all cores available
-		}
-	})
-}
-
 func BenchmarkGoogleapisProtocompileNoSourceInfo(b *testing.B) {
-	benchmarkGoogleapisProtocompile(b, false, func() *protocompile.Compiler {
+	benchmarkGoogleapisProtocompile(b, func() *protocompile.Compiler {
 		return &protocompile.Compiler{
 			Resolver: protocompile.WithStandardImports(&protocompile.SourceResolver{
 				ImportPaths: []string{googleapisDir},
@@ -270,23 +257,19 @@ func BenchmarkGoogleapisProtocompileNoSourceInfo(b *testing.B) {
 	})
 }
 
-func benchmarkGoogleapisProtocompile(b *testing.B, canonicalBytes bool, factory func() *protocompile.Compiler) {
+func benchmarkGoogleapisProtocompile(b *testing.B, factory func() *protocompile.Compiler) {
 	for i := 0; i < b.N; i++ {
-		benchmarkProtocompile(b, factory(), googleapisSources, canonicalBytes)
+		benchmarkProtocompile(b, factory(), googleapisSources)
 	}
 }
 
-func benchmarkProtocompile(b *testing.B, c *protocompile.Compiler, sources []string, canonicalBytes bool) {
+func benchmarkProtocompile(b *testing.B, c *protocompile.Compiler, sources []string) {
 	fds, err := c.Compile(context.Background(), sources...)
 	require.NoError(b, err)
 	var fdSet descriptorpb.FileDescriptorSet
 	fdSet.File = make([]*descriptorpb.FileDescriptorProto, len(fds))
 	for i, fd := range fds {
-		if canonicalBytes {
-			fdSet.File[i] = fd.(linker.Result).CanonicalProto()
-		} else {
-			fdSet.File[i] = protoutil.ProtoFromFileDescriptor(fd)
-		}
+		fdSet.File[i] = protoutil.ProtoFromFileDescriptor(fd)
 	}
 	// protoc is writing output to file descriptor set, so we should, too
 	writeToNull(b, &fdSet)
@@ -484,7 +467,7 @@ func BenchmarkGoogleapisProtocompileSingleThreaded(b *testing.B) {
 				// need to run a single-threaded compile
 				MaxParallelism: 1,
 			}
-			benchmarkProtocompile(b, c, googleapisSources, false)
+			benchmarkProtocompile(b, c, googleapisSources)
 		}
 	})
 }

internal/prototest/util.go

@@ -15,7 +15,6 @@
 package prototest
 
 import (
-	"fmt"
 	"os"
 	"testing"
 
@@ -73,27 +72,11 @@ func findFileInSet(fps *descriptorpb.FileDescriptorSet, name string) *descriptor
 	return nil
 }
 
-func AssertMessagesEqual(t *testing.T, exp, act proto.Message, msgAndArgs ...interface{}) {
+func AssertMessagesEqual(t *testing.T, exp, act proto.Message, description string) bool {
 	t.Helper()
-	AssertMessagesEqualWithOptions(t, exp, act, nil, msgAndArgs...)
-}
-
-func AssertMessagesEqualWithOptions(t *testing.T, exp, act proto.Message, opts []cmp.Option, msgAndArgs ...interface{}) {
-	t.Helper()
-	cmpOpts := []cmp.Option{protocmp.Transform()}
-	cmpOpts = append(cmpOpts, opts...)
-	if diff := cmp.Diff(exp, act, cmpOpts...); diff != "" {
-		var prefix string
-		if len(msgAndArgs) == 1 {
-			if msg, ok := msgAndArgs[0].(string); ok {
-				prefix = msg + ": "
-			} else {
-				prefix = fmt.Sprintf("%+v: ", msgAndArgs[0])
-			}
-		} else if len(msgAndArgs) > 1 {
-			prefix = fmt.Sprintf(msgAndArgs[0].(string)+": ", msgAndArgs[1:]...)
-		}
-
-		t.Errorf("%smessage mismatch (-want +got):\n%v", prefix, diff)
+	if diff := cmp.Diff(exp, act, protocmp.Transform()); diff != "" {
+		t.Errorf("%s: message mismatch (-want, +got):\n%s", description, diff)
+		return false
 	}
+	return true
 }

internal/tags.go

@@ -243,4 +243,9 @@ const (
 	// UninterpretedNameNameTag is the tag number of the name element in an
 	// uninterpreted option name proto.
 	UninterpretedNameNameTag = 1
+
+	// AnyTypeURLTag is the tag number of the type_url field of the Any proto.
+	AnyTypeURLTag = 1
+	// AnyValueTag is the tag number of the value field of the Any proto.
+	AnyValueTag = 2
 )

internal/testdata/options/options.proto

@@ -227,4 +227,4 @@ option (any) = {
     pr_i32: [0,1,2,3]
     str: "foo"
   }
-};
+};

internal/util.go

@@ -230,6 +230,12 @@ func CanPack(k protoreflect.Kind) bool {
 	}
 }
 
+func ClonePath(path protoreflect.SourcePath) protoreflect.SourcePath {
+	clone := make(protoreflect.SourcePath, len(path))
+	copy(clone, path)
+	return clone
+}
+
 func reverse(p protoreflect.SourcePath) protoreflect.SourcePath {
 	for i, j := 0, len(p)-1; i < j; i, j = i+1, j-1 {
 		p[i], p[j] = p[j], p[i]

linker/descriptors.go

@@ -151,11 +151,6 @@ type result struct {
 	// interpreting options.
 	usedImports map[string]struct{}
 
-	// A map of descriptor options messages to their pre-serialized bytes (using
-	// a canonical serialization format based on how protoc renders options to
-	// bytes).
-	optionBytes map[proto.Message][]byte
-
 	// A map of AST nodes that represent identifiers in ast.FieldReferenceNodes
 	// to their fully-qualified name. The identifiers are for field names in
 	// message literals (in option values) that are extension fields. These names
@@ -316,141 +311,6 @@ func asSourceLocations(srcInfoProtos []*descriptorpb.SourceCodeInfo_Location) []
 	return locs
 }
 
-// AddOptionBytes associates the given opts (an options message encoded in the
-// binary format) with the given options protobuf message. The protobuf message
-// should exist in the hierarchy of this result's FileDescriptorProto. This
-// allows the FileDescriptorProto to be marshaled to bytes in a way that
-// preserves the way options are defined in source (just as is done by protoc,
-// but not possible when only using the generated Go types and standard
-// marshaling APIs in the protobuf runtime).
-func (r *result) AddOptionBytes(pm proto.Message, opts []byte) {
-	if r.optionBytes == nil {
-		r.optionBytes = map[proto.Message][]byte{}
-	}
-	r.optionBytes[pm] = append(r.optionBytes[pm], opts...)
-}
-
-func (r *result) CanonicalProto() *descriptorpb.FileDescriptorProto {
-	origFd := r.FileDescriptorProto()
-	// make a copy that we can mutate
-	fd := proto.Clone(origFd).(*descriptorpb.FileDescriptorProto) //nolint:errcheck
-
-	r.storeOptionBytesInFile(fd, origFd)
-
-	return fd
-}
-
-func (r *result) storeOptionBytes(opts, origOpts proto.Message) {
-	optionBytes := r.optionBytes[origOpts]
-	if len(optionBytes) == 0 {
-		// If we don't know about this options message, leave it alone.
-		return
-	}
-	proto.Reset(opts)
-	opts.ProtoReflect().SetUnknown(optionBytes)
-}
-
-func (r *result) storeOptionBytesInFile(fd, origFd *descriptorpb.FileDescriptorProto) {
-	if fd.Options != nil {
-		r.storeOptionBytes(fd.Options, origFd.Options)
-	}
-
-	for i, md := range fd.MessageType {
-		origMd := origFd.MessageType[i]
-		r.storeOptionBytesInMessage(md, origMd)
-	}
-
-	for i, ed := range fd.EnumType {
-		origEd := origFd.EnumType[i]
-		r.storeOptionBytesInEnum(ed, origEd)
-	}
-
-	for i, exd := range fd.Extension {
-		origExd := origFd.Extension[i]
-		r.storeOptionBytesInField(exd, origExd)
-	}
-
-	for i, sd := range fd.Service {
-		origSd := origFd.Service[i]
-		if sd.Options != nil {
-			r.storeOptionBytes(sd.Options, origSd.Options)
-		}
-
-		for j, mtd := range sd.Method {
-			origMtd := origSd.Method[j]
-			if mtd.Options != nil {
-				r.storeOptionBytes(mtd.Options, origMtd.Options)
-			}
-		}
-	}
-}
-
-func (r *result) storeOptionBytesInMessage(md, origMd *descriptorpb.DescriptorProto) {
-	if md.GetOptions().GetMapEntry() {
-		// Map entry messages are synthesized. They won't have any option bytes
-		// since they don't actually appear in the source and thus have any option
-		// declarations in the source.
-		return
-	}
-
-	if md.Options != nil {
-		r.storeOptionBytes(md.Options, origMd.Options)
-	}
-
-	for i, fld := range md.Field {
-		origFld := origMd.Field[i]
-		r.storeOptionBytesInField(fld, origFld)
-	}
-
-	for i, ood := range md.OneofDecl {
-		origOod := origMd.OneofDecl[i]
-		if ood.Options != nil {
-			r.storeOptionBytes(ood.Options, origOod.Options)
-		}
-	}
-
-	for i, exr := range md.ExtensionRange {
-		origExr := origMd.ExtensionRange[i]
-		if exr.Options != nil {
-			r.storeOptionBytes(exr.Options, origExr.Options)
-		}
-	}
-
-	for i, nmd := range md.NestedType {
-		origNmd := origMd.NestedType[i]
-		r.storeOptionBytesInMessage(nmd, origNmd)
-	}
-
-	for i, ed := range md.EnumType {
-		origEd := origMd.EnumType[i]
-		r.storeOptionBytesInEnum(ed, origEd)
-	}
-
-	for i, exd := range md.Extension {
-		origExd := origMd.Extension[i]
-		r.storeOptionBytesInField(exd, origExd)
-	}
-}
-
-func (r *result) storeOptionBytesInEnum(ed, origEd *descriptorpb.EnumDescriptorProto) {
-	if ed.Options != nil {
-		r.storeOptionBytes(ed.Options, origEd.Options)
-	}
-
-	for i, evd := range ed.Value {
-		origEvd := origEd.Value[i]
-		if evd.Options != nil {
-			r.storeOptionBytes(evd.Options, origEvd.Options)
-		}
-	}
-}
-
-func (r *result) storeOptionBytesInField(fld, origFld *descriptorpb.FieldDescriptorProto) {
-	if fld.Options != nil {
-		r.storeOptionBytes(fld.Options, origFld.Options)
-	}
-}
-
 type fileImports struct {
 	protoreflect.FileImports
 	files []protoreflect.FileImport

linker/linker.go

@@ -18,7 +18,6 @@ import (
 	"fmt"
 
 	"google.golang.org/protobuf/reflect/protoreflect"
-	"google.golang.org/protobuf/types/descriptorpb"
 
 	"github.com/bufbuild/protocompile/ast"
 	"github.com/bufbuild/protocompile/parser"
@@ -128,30 +127,6 @@ type Result interface {
 	// interpreting options (which is done after linking).
 	PopulateSourceCodeInfo()
 
-	// CanonicalProto returns the file descriptor proto in a form that
-	// will be serialized in a canonical way. The "canonical" way matches
-	// the way that "protoc" emits option values, which is a way that
-	// mostly matches the way options are defined in source, including
-	// ordering and de-structuring. Unlike the FileDescriptorProto() method,
-	// this method is more expensive and results in a new descriptor proto
-	// being constructed with each call.
-	//
-	// The returned value will have all options (fields of the various
-	// descriptorpb.*Options message types) represented via unrecognized
-	// fields. So the returned value will serialize as desired, but it
-	// is otherwise not useful since all option values are treated as
-	// unknown.
-	//
-	// Note that CanonicalProto is a no-op if the options in this file
-	// were not interpreted by this module (e.g. the underlying descriptor
-	// proto was provided, with options already interpreted, instead of
-	// parsed from source). If the underlying descriptor proto was provided,
-	// but with a mix of interpreted and uninterpreted options, this method
-	// will effectively clear the already-interpreted fields and only the
-	// options actually interpreted by the compile operation will be
-	// retained.
-	CanonicalProto() *descriptorpb.FileDescriptorProto
-
 	// RemoveAST drops the AST information from this result.
 	RemoveAST()
 }

linker/linker_test.go

@@ -2213,7 +2213,7 @@ func TestLinkerValidation(t *testing.T) {
 					}
 				`,
 			},
-			expectedErr: `test.proto:3:18: feature "enum_type" is allowed on [enum,file], not on field`,
+			expectedErr: `test.proto:3:27: feature "enum_type" is allowed on [enum,file], not on field`,
 		},
 		"failure_editions_feature_on_wrong_target_type_msg_literal": {
 			input: map[string]string{