diff --git a/protoc-gen-go/generator/generator.go b/protoc-gen-go/generator/generator.go index 24367e2db9..fb2dd160be 100644 --- a/protoc-gen-go/generator/generator.go +++ b/protoc-gen-go/generator/generator.go @@ -270,8 +270,9 @@ type FileDescriptor struct { // This is used for supporting public imports. exported map[Object][]symbol - fingerprint string // Fingerprint of this file's contents. - importPath GoImportPath // Import path of this file's package. + fingerprint string // Fingerprint of this file's contents. + importPath GoImportPath // Import path of this file's package. + packageName GoPackageName // Name of this file's Go package. proto3 bool // whether to generate proto3 code for this file } @@ -306,25 +307,6 @@ func (d *FileDescriptor) goPackageOption() (impPath GoImportPath, pkg GoPackageN return "", cleanPackageName(opt), true } -// goPackageName returns the Go package name to use in the -// generated Go file. The result explicit reports whether the name -// came from an option go_package statement. If explicit is false, -// the name was derived from the protocol buffer's package statement -// or the input file name. -func (d *FileDescriptor) goPackageName() (name GoPackageName, explicit bool) { - // Does the file have a "go_package" option? - if _, pkg, ok := d.goPackageOption(); ok { - return pkg, true - } - - // Does the file have a package clause? - if p := d.GetPackage(); p != "" { - return cleanPackageName(p), false - } - // Use the file base name. - return cleanPackageName(baseName(d.GetName())), false -} - // goFileName returns the output name for the generated Go file. func (d *FileDescriptor) goFileName(pathType pathType) string { name := *d.Name @@ -773,15 +755,45 @@ func (g *Generator) defaultGoPackage() GoPackageName { func (g *Generator) SetPackageNames() { g.outputImportPath = g.genFiles[0].importPath + defaultPackageNames := make(map[GoImportPath]GoPackageName) + for _, f := range g.genFiles { + if _, p, ok := f.goPackageOption(); ok { + defaultPackageNames[f.importPath] = p + } + } + for _, f := range g.genFiles { + if _, p, ok := f.goPackageOption(); ok { + // Source file: option go_package = "quux/bar"; + f.packageName = p + } else if p, ok := defaultPackageNames[f.importPath]; ok { + // A go_package option in another file in the same package. + // + // This is a poor choice in general, since every source file should + // contain a go_package option. Supported mainly for historical + // compatibility. + f.packageName = p + } else if p := g.defaultGoPackage(); p != "" { + // Command-line: import_path=quux/bar. + // + // The import_path flag sets a package name for files which don't + // contain a go_package option. + f.packageName = p + } else if p := f.GetPackage(); p != "" { + // Source file: package quux.bar; + f.packageName = cleanPackageName(p) + } else { + // Source filename. + f.packageName = cleanPackageName(baseName(f.GetName())) + } + } + // Check that all files have a consistent package name and import path. - pkg, _ := g.genFiles[0].goPackageName() for _, f := range g.genFiles[1:] { if a, b := g.genFiles[0].importPath, f.importPath; a != b { - g.Fail(fmt.Sprint("inconsistent package import paths: ", a, b)) + g.Fail(fmt.Sprintf("inconsistent package import paths: %v, %v", a, b)) } - thisPkg, _ := f.goPackageName() - if pkg != thisPkg { - g.Fail(fmt.Sprint("inconsistent package names: ", thisPkg, pkg)) + if a, b := g.genFiles[0].packageName, f.packageName; a != b { + g.Fail(fmt.Sprintf("inconsistent package names: %v, %v", a, b)) } } @@ -800,17 +812,37 @@ func (g *Generator) SetPackageNames() { func (g *Generator) WrapTypes() { g.allFiles = make([]*FileDescriptor, 0, len(g.Request.ProtoFile)) g.allFilesByName = make(map[string]*FileDescriptor, len(g.allFiles)) + genFileNames := make(map[string]bool) + for _, n := range g.Request.FileToGenerate { + genFileNames[n] = true + } for _, f := range g.Request.ProtoFile { fd := &FileDescriptor{ FileDescriptorProto: f, exported: make(map[Object][]symbol), proto3: fileIsProto3(f), } + // The import path may be set in a number of ways. if substitution, ok := g.ImportMap[f.GetName()]; ok { + // Command-line: M=foo.proto=quux/bar. + // + // Explicit mapping of source file to import path. fd.importPath = GoImportPath(substitution) + } else if genFileNames[f.GetName()] && g.PackageImportPath != "" { + // Command-line: import_path=quux/bar. + // + // The import_path flag sets the import path for every file that + // we generate code for. + fd.importPath = GoImportPath(g.PackageImportPath) } else if p, _, _ := fd.goPackageOption(); p != "" { + // Source file: option go_package = "quux/bar"; + // + // The go_package option sets the import path. Most users should use this. fd.importPath = p } else { + // Source filename. + // + // Last resort when nothing else is available. fd.importPath = GoImportPath(path.Dir(f.GetName())) } // We must wrap the descriptors before we wrap the enums @@ -1335,12 +1367,11 @@ func (g *Generator) generateHeader() { } g.P() - name, _ := g.file.goPackageName() importPath, _, _ := g.file.goPackageOption() if importPath == "" { - g.P("package ", name) + g.P("package ", g.file.packageName) } else { - g.P("package ", name, " // import ", GoImportPath(g.ImportPrefix)+importPath) + g.P("package ", g.file.packageName, " // import ", GoImportPath(g.ImportPrefix)+importPath) } g.P() diff --git a/protoc-gen-go/golden_test.go b/protoc-gen-go/golden_test.go index 19c7d0dc97..70f3971246 100644 --- a/protoc-gen-go/golden_test.go +++ b/protoc-gen-go/golden_test.go @@ -115,14 +115,14 @@ var fdescRE = regexp.MustCompile(`(?ms)^var fileDescriptor.*}`) const ( aProto = ` syntax = "proto3"; -package alpha; +package test.alpha; option go_package = "package/alpha"; import "beta/b.proto"; -message M { beta.M field = 1; }` +message M { test.beta.M field = 1; }` bProto = ` syntax = "proto3"; -package beta; +package test.beta; // no go_package option message M {}` ) @@ -132,12 +132,16 @@ func TestParameters(t *testing.T) { parameters string wantFiles map[string]bool wantImportsA map[string]bool + wantPackageA string + wantPackageB string }{{ parameters: "", wantFiles: map[string]bool{ "package/alpha/a.pb.go": true, "beta/b.pb.go": true, }, + wantPackageA: "alpha", + wantPackageB: "test_beta", wantImportsA: map[string]bool{ "github.com/golang/protobuf/proto": true, "beta": true, @@ -148,6 +152,8 @@ func TestParameters(t *testing.T) { "package/alpha/a.pb.go": true, "beta/b.pb.go": true, }, + wantPackageA: "alpha", + wantPackageB: "test_beta", wantImportsA: map[string]bool{ // This really doesn't seem like useful behavior. "prefixgithub.com/golang/protobuf/proto": true, @@ -155,7 +161,9 @@ func TestParameters(t *testing.T) { }, }, { // import_path only affects the 'package' line. - parameters: "import_path=import/path/of/pkg", + parameters: "import_path=import/path/of/pkg", + wantPackageA: "alpha", + wantPackageB: "pkg", wantFiles: map[string]bool{ "package/alpha/a.pb.go": true, "beta/b.pb.go": true, @@ -166,6 +174,8 @@ func TestParameters(t *testing.T) { "package/alpha/a.pb.go": true, "beta/b.pb.go": true, }, + wantPackageA: "alpha", + wantPackageB: "test_beta", wantImportsA: map[string]bool{ "github.com/golang/protobuf/proto": true, // Rewritten by the M parameter. @@ -177,6 +187,8 @@ func TestParameters(t *testing.T) { "package/alpha/a.pb.go": true, "beta/b.pb.go": true, }, + wantPackageA: "alpha", + wantPackageB: "test_beta", wantImportsA: map[string]bool{ // import_prefix applies after M. "prefixpackage/gamma": true, @@ -187,6 +199,8 @@ func TestParameters(t *testing.T) { "alpha/a.pb.go": true, "beta/b.pb.go": true, }, + wantPackageA: "alpha", + wantPackageB: "test_beta", }, { parameters: "paths=source_relative,import_prefix=prefix", wantFiles: map[string]bool{ @@ -194,6 +208,8 @@ func TestParameters(t *testing.T) { "alpha/a.pb.go": true, "beta/b.pb.go": true, }, + wantPackageA: "alpha", + wantPackageB: "test_beta", }} { name := test.parameters if name == "" { @@ -232,19 +248,20 @@ func TestParameters(t *testing.T) { filepath.Join(workdir, "beta", "b.proto"), }) - var aGen string + contents := make(map[string]string) gotFiles := make(map[string]bool) outdir := filepath.Join(workdir, "out") filepath.Walk(outdir, func(p string, info os.FileInfo, _ error) error { if info.IsDir() { return nil } - if filepath.Base(p) == "a.pb.go" { + base := filepath.Base(p) + if base == "a.pb.go" || base == "b.pb.go" { b, err := ioutil.ReadFile(p) if err != nil { t.Fatal(err) } - aGen = string(b) + contents[base] = string(b) } relPath, _ := filepath.Rel(outdir, p) gotFiles[relPath] = true @@ -266,10 +283,20 @@ func TestParameters(t *testing.T) { t.Errorf("missing output file: %v", want) } } - gotImports, err := parseImports(aGen) + gotPackageA, gotImports, err := parseFile(contents["a.pb.go"]) if err != nil { t.Fatal(err) } + gotPackageB, _, err := parseFile(contents["b.pb.go"]) + if err != nil { + t.Fatal(err) + } + if got, want := gotPackageA, test.wantPackageA; want != got { + t.Errorf("output file a.pb.go is package %q, want %q", got, want) + } + if got, want := gotPackageB, test.wantPackageB; want != got { + t.Errorf("output file b.pb.go is package %q, want %q", got, want) + } missingImport := false WantImport: for want := range test.wantImportsA { @@ -348,18 +375,17 @@ func TestPackageComment(t *testing.T) { } } -// parseImports returns a list of all packages imported by a file. -func parseImports(source string) ([]string, error) { +// parseFile returns a file's package name and a list of all packages it imports. +func parseFile(source string) (packageName string, imports []string, err error) { fset := token.NewFileSet() f, err := parser.ParseFile(fset, "", source, parser.ImportsOnly) if err != nil { - return nil, err + return "", nil, err } - var imports []string for _, imp := range f.Imports { imports = append(imports, imp.Path.Value) } - return imports, nil + return f.Name.Name, imports, nil } func protoc(t *testing.T, args []string) {