Improvements to recently introduced sourceinfo stuff (#500)

The `desc.Load*` functions now return descriptors that include source info, if available (i.e. if protoc-gen-gosrcinfo plugin is used).

desc/builder/builder_test.go

@@ -540,8 +540,9 @@ func roundTripFile(t *testing.T, fd *desc.FileDescriptor) {
 	fdp.PublicDependency = nil
 	fdp.WeakDependency = nil
 
-	// Remove source code info that the builder generated since the original
-	// has none.
+	// Remove source code info: what the builder generates is not expected to
+	// match the original source.
+	fdp.SourceCodeInfo = nil
 	roundTripped.AsFileDescriptorProto().SourceCodeInfo = nil
 
 	// Finally, sort the imports. That way they match the built result (which

desc/load.go

@@ -8,6 +8,7 @@ import (
 	"github.com/golang/protobuf/proto"
 	dpb "github.com/golang/protobuf/protoc-gen-go/descriptor"
 
+	"github.com/jhump/protoreflect/desc/sourceinfo"
 	"github.com/jhump/protoreflect/internal"
 )
 
@@ -54,6 +55,7 @@ func loadFileDescriptorLocked(file string, r *ImportResolver) (*FileDescriptor,
 }
 
 func toFileDescriptorLocked(fd *dpb.FileDescriptorProto, r *ImportResolver) (*FileDescriptor, error) {
+	fd.SourceCodeInfo = sourceinfo.SourceInfoForFile(fd.GetName())
 	deps := make([]*FileDescriptor, len(fd.GetDependency()))
 	for i, dep := range fd.GetDependency() {
 		resolvedDep := r.ResolveImport(fd.GetName(), dep)

desc/protoparse/linker_test.go

@@ -87,6 +87,9 @@ func checkFiles(t *testing.T, act, exp *desc.FileDescriptor, checked map[string]
 	}
 	checked[act.GetName()] = struct{}{}
 
+	// remove any source code info from expected value, since actual won't have any
+	exp.AsFileDescriptorProto().SourceCodeInfo = nil
+
 	testutil.Require(t, proto.Equal(exp.AsFileDescriptorProto(), act.AsProto()), "linked descriptor did not match output from protoc:\nwanted: %s\ngot: %s", toString(exp.AsProto()), toString(act.AsProto()))
 
 	for i, dep := range act.GetDependencies() {

desc/protoprint/testfiles/desc_test2-compact.proto

@@ -1,9 +1,9 @@
 syntax = "proto2";
+option go_package = "github.com/jhump/protoreflect/internal/testprotos";
 package testprotos;
 import "desc_test1.proto";
 import "pkg/desc_test_pkg.proto";
 import "nopkg/desc_test_nopkg.proto";
-option go_package = "github.com/jhump/protoreflect/internal/testprotos";
 message Frobnitz {
   optional TestMessage a = 1;
   optional AnotherTestMessage b = 2;

desc/protoprint/testfiles/desc_test2-default.proto

@@ -1,5 +1,7 @@
 syntax = "proto2";
 
+option go_package = "github.com/jhump/protoreflect/internal/testprotos";
+
 package testprotos;
 
 import "desc_test1.proto";
@@ -8,8 +10,6 @@ import "pkg/desc_test_pkg.proto";
 
 import "nopkg/desc_test_nopkg.proto";
 
-option go_package = "github.com/jhump/protoreflect/internal/testprotos";
-
 message Frobnitz {
   optional TestMessage a = 1;
 

desc/protoprint/testfiles/desc_test2-multiline-style-comments.proto

@@ -1,5 +1,7 @@
 syntax = "proto2";
 
+option go_package = "github.com/jhump/protoreflect/internal/testprotos";
+
 package testprotos;
 
 import "desc_test1.proto";
@@ -8,8 +10,6 @@ import "pkg/desc_test_pkg.proto";
 
 import "nopkg/desc_test_nopkg.proto";
 
-option go_package = "github.com/jhump/protoreflect/internal/testprotos";
-
 message Frobnitz {
 	optional TestMessage a = 1;
 

desc/protoprint/testfiles/desc_test2-no-trailing-comments.proto

@@ -1,5 +1,7 @@
 syntax = "proto2";
 
+option go_package = "github.com/jhump/protoreflect/internal/testprotos";
+
 package testprotos;
 
 import "desc_test1.proto";
@@ -8,8 +10,6 @@ import "pkg/desc_test_pkg.proto";
 
 import "nopkg/desc_test_nopkg.proto";
 
-option go_package = "github.com/jhump/protoreflect/internal/testprotos";
-
 message Frobnitz {
   optional TestMessage a = 1;
 

desc/protoprint/testfiles/desc_test2-only-doc-comments.proto

@@ -1,5 +1,7 @@
 syntax = "proto2";
 
+option go_package = "github.com/jhump/protoreflect/internal/testprotos";
+
 package testprotos;
 
 import "desc_test1.proto";
@@ -8,8 +10,6 @@ import "pkg/desc_test_pkg.proto";
 
 import "nopkg/desc_test_nopkg.proto";
 
-option go_package = "github.com/jhump/protoreflect/internal/testprotos";
-
 message Frobnitz {
   optional TestMessage a = 1;
 

desc/protoprint/testfiles/desc_test2-trailing-on-next-line.proto

@@ -1,5 +1,7 @@
 syntax = "proto2";
 
+option go_package = "github.com/jhump/protoreflect/internal/testprotos";
+
 package testprotos;
 
 import "desc_test1.proto";
@@ -8,8 +10,6 @@ import "pkg/desc_test_pkg.proto";
 
 import "nopkg/desc_test_nopkg.proto";
 
-option go_package = "github.com/jhump/protoreflect/internal/testprotos";
-
 message Frobnitz {
   optional TestMessage a = 1;
 

desc/sourceinfo/cmd/protoc-gen-gosrcinfo/main.go

@@ -6,13 +6,11 @@ package main
 import (
 	"fmt"
 	"path"
-	"reflect"
 	"strings"
 
 	"github.com/golang/protobuf/proto"
 	"github.com/jhump/gopoet"
 	"github.com/jhump/goprotoc/plugins"
-	"google.golang.org/protobuf/types/descriptorpb"
 	"google.golang.org/protobuf/types/pluginpb"
 
 	"github.com/jhump/protoreflect/desc"
@@ -61,8 +59,6 @@ func genSourceInfo(req *plugins.CodeGenRequest, resp *plugins.CodeGenResponse) e
 	return nil
 }
 
-var typeOfSourceInfo = reflect.TypeOf((*descriptorpb.SourceCodeInfo)(nil)).Elem()
-
 func generateSourceInfo(fd *desc.FileDescriptor, names *plugins.GoNames, resp *plugins.CodeGenResponse, args codeGenArgs) error {
 	si := fd.AsFileDescriptorProto().GetSourceCodeInfo()
 	if len(si.GetLocation()) == 0 {
@@ -80,7 +76,7 @@ func generateSourceInfo(fd *desc.FileDescriptor, names *plugins.GoNames, resp *p
 		return fmt.Errorf("failed to serialize source code info: %w", err)
 	}
 
-	srcCodeInfo := f.EnsureTypeImported(gopoet.TypeNameForReflectType(typeOfSourceInfo))
+	descpbPkg := f.RegisterImport("google.golang.org/protobuf/types/descriptorpb", "descriptorpb")
 	srcInfoPkg := f.RegisterImport("github.com/jhump/protoreflect/desc/sourceinfo", "sourceinfo")
 	protoPkg := f.RegisterImport("google.golang.org/protobuf/proto", "proto")
 
@@ -104,7 +100,7 @@ func generateSourceInfo(fd *desc.FileDescriptor, names *plugins.GoNames, resp *p
 	initBlock.Println("}")
 	f.AddVar(gopoet.NewVar(varName).SetInitializer(&initBlock))
 	f.AddElement(gopoet.NewFunc("init").
-		Printlnf("var si %s", srcCodeInfo).
+		Printlnf("var si %sSourceCodeInfo", descpbPkg).
 		Printlnf("if err := %sUnmarshal(%s, &si); err != nil {", protoPkg, varName).
 		Println("    panic(err)").
 		Println("}").

desc/sourceinfo/srcinforeflection/serverreflection.go

@@ -2,6 +2,12 @@
 // that includes source code info, if the protoc-gen-gosrcinfo plugin was used
 // for the files that contain the descriptors being served. This allows for
 // sending comment information to dynamic/reflective clients.
+//
+// NOTE: This package is EXPERIMENTAL! It may not see the light of day in an
+// actual release. Instead, the hope is that the core gRPC reflection package
+// can be updated in a way to more easily accommodate customizations, such as
+// the caller wanting to use sourceinfo.GlobalFiles as the source of descriptors.
+// Related: https://github.com/grpc/grpc-go/pull/5197
 package srcinforeflection
 
 import (

dynamic/dynamic_message_test.go

@@ -2045,6 +2045,9 @@ func TestGetDescriptor(t *testing.T) {
 		testutil.Eq(t, expectedPath, actualPath, "%s: descriptor paths are not the same", testCase.name)
 
 		actualFd, err := internal.DecodeFileDescriptor("TestMessage", actualBytes)
+		// desc.LoadMessageDescriptorForMessage above can incorporate source code info that will
+		// not be present when directly calling generated messages's Descriptor method
+		actualFd.SourceCodeInfo = nil
 		testutil.Ok(t, err, "%s: failed to decode descriptor from bytes", testCase.name)
 		expectedFd, err := internal.DecodeFileDescriptor("TestMessage", expectedBytes)
 		testutil.Ok(t, err, "%s: failed to decode descriptor from bytes", testCase.name)