Просмотр исходного кода

protoc-gen-go: reference publicly imported symbols directly

Consider this case:

File 1 defines M.
File 2 publicly imports file 1.
File 3 imports file 2, and references M.

Each file is in a different Go package: P1, P2, P3.

Should the generated Go code for file 3 reference P1.M, or P2.M?
The two should be equivalent, since file 2 will contain a forwarding
declaration such as "type M = P1.M".

Historically, we've gone with the latter (P2.M). This does make sense:
A generated file only imports the packages of the files that it directly
imports.

However, this does have some mildly surprising effects. If File 3
imports files 2a and 2b, each of which publicly imports file 1, we need
to arbitrarily pick one of P2a.M or P2b.M. (Admittedly an obscure case.)

Simplify the generator a little bit (and, more importantly, make it
consistent with the v2 generator) and change to referencing public
imports directly.
Damien Neil 7 лет назад
Родитель
Сommit
1918e1ff6f

+ 9 - 37
protoc-gen-go/generator/generator.go

@@ -973,39 +973,6 @@ func (g *Generator) ObjectNamed(typeName string) Object {
 	if !ok {
 		g.Fail("can't find object with type", typeName)
 	}
-
-	// If the file of this object isn't a direct dependency of the current file,
-	// or in the current file, then this object has been publicly imported into
-	// a dependency of the current file.
-	// We should return the ImportedDescriptor object for it instead.
-	direct := *o.File().Name == *g.file.Name
-	if !direct {
-		for _, dep := range g.file.Dependency {
-			if *g.fileByName(dep).Name == *o.File().Name {
-				direct = true
-				break
-			}
-		}
-	}
-	if !direct {
-		found := false
-	Loop:
-		for _, dep := range g.file.Dependency {
-			df := g.fileByName(*g.fileByName(dep).Name)
-			for _, td := range df.imp {
-				if td.o == o {
-					// Found it!
-					o = td
-					found = true
-					break Loop
-				}
-			}
-		}
-		if !found {
-			log.Printf("protoc-gen-go: WARNING: failed finding publicly imported dependency for %v, used in %v", typeName, *g.file.Name)
-		}
-	}
-
 	return o
 }
 
@@ -1689,11 +1656,16 @@ func (g *Generator) GoType(message *Descriptor, field *descriptor.FieldDescripto
 }
 
 func (g *Generator) RecordTypeUse(t string) {
-	if _, ok := g.typeNameToObject[t]; ok {
-		// Call ObjectNamed to get the true object to record the use.
-		obj := g.ObjectNamed(t)
-		g.usedPackages[obj.GoImportPath()] = true
+	if _, ok := g.typeNameToObject[t]; !ok {
+		return
+	}
+	importPath := g.ObjectNamed(t).GoImportPath()
+	if importPath == g.outputImportPath {
+		// Don't record use of objects in our package.
+		return
 	}
+	g.AddImport(importPath)
+	g.usedPackages[importPath] = true
 }
 
 // Method names that may be generated.  Fields with these names get an

+ 7 - 6
protoc-gen-go/testdata/import_public/importing/importing.pb.go

@@ -6,7 +6,8 @@ package importing
 import (
 	fmt "fmt"
 	proto "github.com/golang/protobuf/proto"
-	import_public "github.com/golang/protobuf/protoc-gen-go/testdata/import_public"
+	_ "github.com/golang/protobuf/protoc-gen-go/testdata/import_public"
+	sub "github.com/golang/protobuf/protoc-gen-go/testdata/import_public/sub"
 	math "math"
 )
 
@@ -23,10 +24,10 @@ const _ = proto.ProtoPackageIsVersion2 // please upgrade the proto package
 
 type M struct {
 	// Message type defined in a file publicly imported by a file we import.
-	M                    *import_public.M `protobuf:"bytes,1,opt,name=m" json:"m,omitempty"`
-	XXX_NoUnkeyedLiteral struct{}         `json:"-"`
-	XXX_unrecognized     []byte           `json:"-"`
-	XXX_sizecache        int32            `json:"-"`
+	M                    *sub.M   `protobuf:"bytes,1,opt,name=m" json:"m,omitempty"`
+	XXX_NoUnkeyedLiteral struct{} `json:"-"`
+	XXX_unrecognized     []byte   `json:"-"`
+	XXX_sizecache        int32    `json:"-"`
 }
 
 func (m *M) Reset()         { *m = M{} }
@@ -54,7 +55,7 @@ func (m *M) XXX_DiscardUnknown() {
 
 var xxx_messageInfo_M proto.InternalMessageInfo
 
-func (m *M) GetM() *import_public.M {
+func (m *M) GetM() *sub.M {
 	if m != nil {
 		return m.M
 	}