protoc-gen-go: don't generate blank // import comment (#554)
Fix a bug which caused us to generate a blank import comment when go_package
contains just a package name (no import path):
package foo // import ""
Fixes #553
diff --git a/protoc-gen-go/generator/generator.go b/protoc-gen-go/generator/generator.go
index f943322..d8e512e 100644
--- a/protoc-gen-go/generator/generator.go
+++ b/protoc-gen-go/generator/generator.go
@@ -291,17 +291,17 @@
if opt == "" {
return "", "", false
}
+ // A semicolon-delimited suffix delimits the import path and package name.
+ sc := strings.Index(opt, ";")
+ if sc >= 0 {
+ return GoImportPath(opt[:sc]), cleanPackageName(opt[sc+1:]), true
+ }
// The presence of a slash implies there's an import path.
slash := strings.LastIndex(opt, "/")
- if slash < 0 {
- return "", GoPackageName(opt), true
- }
- // A semicolon-delimited suffix overrides the package name.
- sc := strings.Index(opt, ";")
- if sc < 0 {
+ if slash >= 0 {
return GoImportPath(opt), cleanPackageName(opt[slash+1:]), true
}
- return GoImportPath(opt[:sc]), cleanPackageName(opt[sc+1:]), true
+ return "", cleanPackageName(opt), true
}
// goPackageName returns the Go package name to use in the
@@ -1360,8 +1360,8 @@
g.P()
name, _ := g.file.goPackageName()
- importPath, _, haveImportPath := g.file.goPackageOption()
- if !haveImportPath {
+ importPath, _, _ := g.file.goPackageOption()
+ if importPath == "" {
g.P("package ", name)
} else {
g.P("package ", name, " // import ", g.ImportPrefix+importPath)
diff --git a/protoc-gen-go/golden_test.go b/protoc-gen-go/golden_test.go
index 950f1f0..19c7d0d 100644
--- a/protoc-gen-go/golden_test.go
+++ b/protoc-gen-go/golden_test.go
@@ -3,6 +3,7 @@
import (
"bytes"
"flag"
+ "fmt"
"go/parser"
"go/token"
"io/ioutil"
@@ -289,6 +290,64 @@
}
}
+func TestPackageComment(t *testing.T) {
+ workdir, err := ioutil.TempDir("", "proto-test")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer os.RemoveAll(workdir)
+
+ var packageRE = regexp.MustCompile(`(?m)^package .*`)
+
+ for i, test := range []struct {
+ goPackageOption string
+ wantPackage string
+ }{{
+ goPackageOption: ``,
+ wantPackage: `package proto_package`,
+ }, {
+ goPackageOption: `option go_package = "go_package";`,
+ wantPackage: `package go_package`,
+ }, {
+ goPackageOption: `option go_package = "import/path/of/go_package";`,
+ wantPackage: `package go_package // import "import/path/of/go_package"`,
+ }, {
+ goPackageOption: `option go_package = "import/path/of/something;go_package";`,
+ wantPackage: `package go_package // import "import/path/of/something"`,
+ }, {
+ goPackageOption: `option go_package = "import_path;go_package";`,
+ wantPackage: `package go_package // import "import_path"`,
+ }} {
+ srcName := filepath.Join(workdir, fmt.Sprintf("%d.proto", i))
+ tgtName := filepath.Join(workdir, fmt.Sprintf("%d.pb.go", i))
+
+ buf := &bytes.Buffer{}
+ fmt.Fprintln(buf, `syntax = "proto3";`)
+ fmt.Fprintln(buf, `package proto_package;`)
+ fmt.Fprintln(buf, test.goPackageOption)
+ if err := ioutil.WriteFile(srcName, buf.Bytes(), 0666); err != nil {
+ t.Fatal(err)
+ }
+
+ protoc(t, []string{"-I" + workdir, "--go_out=paths=source_relative:" + workdir, srcName})
+
+ out, err := ioutil.ReadFile(tgtName)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ pkg := packageRE.Find(out)
+ if pkg == nil {
+ t.Errorf("generated .pb.go contains no package line\n\nsource:\n%v\n\noutput:\n%v", buf.String(), string(out))
+ continue
+ }
+
+ if got, want := string(pkg), test.wantPackage; got != want {
+ t.Errorf("unexpected package statement with go_package = %q\n got: %v\nwant: %v", test.goPackageOption, got, want)
+ }
+ }
+}
+
// parseImports returns a list of all packages imported by a file.
func parseImports(source string) ([]string, error) {
fset := token.NewFileSet()
