Skip to content

Commit 1918e1f

Browse files
committed
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.
1 parent 9a73c7f commit 1918e1f

File tree

2 files changed

+16
-43
lines changed

2 files changed

+16
-43
lines changed

protoc-gen-go/generator/generator.go

Lines changed: 9 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -973,39 +973,6 @@ func (g *Generator) ObjectNamed(typeName string) Object {
973973
if !ok {
974974
g.Fail("can't find object with type", typeName)
975975
}
976-
977-
// If the file of this object isn't a direct dependency of the current file,
978-
// or in the current file, then this object has been publicly imported into
979-
// a dependency of the current file.
980-
// We should return the ImportedDescriptor object for it instead.
981-
direct := *o.File().Name == *g.file.Name
982-
if !direct {
983-
for _, dep := range g.file.Dependency {
984-
if *g.fileByName(dep).Name == *o.File().Name {
985-
direct = true
986-
break
987-
}
988-
}
989-
}
990-
if !direct {
991-
found := false
992-
Loop:
993-
for _, dep := range g.file.Dependency {
994-
df := g.fileByName(*g.fileByName(dep).Name)
995-
for _, td := range df.imp {
996-
if td.o == o {
997-
// Found it!
998-
o = td
999-
found = true
1000-
break Loop
1001-
}
1002-
}
1003-
}
1004-
if !found {
1005-
log.Printf("protoc-gen-go: WARNING: failed finding publicly imported dependency for %v, used in %v", typeName, *g.file.Name)
1006-
}
1007-
}
1008-
1009976
return o
1010977
}
1011978

@@ -1689,11 +1656,16 @@ func (g *Generator) GoType(message *Descriptor, field *descriptor.FieldDescripto
16891656
}
16901657

16911658
func (g *Generator) RecordTypeUse(t string) {
1692-
if _, ok := g.typeNameToObject[t]; ok {
1693-
// Call ObjectNamed to get the true object to record the use.
1694-
obj := g.ObjectNamed(t)
1695-
g.usedPackages[obj.GoImportPath()] = true
1659+
if _, ok := g.typeNameToObject[t]; !ok {
1660+
return
1661+
}
1662+
importPath := g.ObjectNamed(t).GoImportPath()
1663+
if importPath == g.outputImportPath {
1664+
// Don't record use of objects in our package.
1665+
return
16961666
}
1667+
g.AddImport(importPath)
1668+
g.usedPackages[importPath] = true
16971669
}
16981670

16991671
// Method names that may be generated. Fields with these names get an

protoc-gen-go/testdata/import_public/importing/importing.pb.go

Lines changed: 7 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)