protoc-gen-go: don't generate blank // import comment (protocolbuffer…

…s#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 protocolbuffers#553

protoc-gen-go/generator/generator.go

@@ -291,17 +291,17 @@ func (d *FileDescriptor) goPackageOption() (impPath GoImportPath, pkg GoPackageN
 	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 @@ func (g *Generator) generateHeader() {
 	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)

protoc-gen-go/golden_test.go

@@ -3,6 +3,7 @@ package main
 import (
 	"bytes"
 	"flag"
+	"fmt"
 	"go/parser"
 	"go/token"
 	"io/ioutil"
@@ -289,6 +290,64 @@ func TestParameters(t *testing.T) {
 	}
 }
 
+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()