compiler/protogen: deprecate certain ways to derive Go package information

There are currently at least 5 ways to derive Go package information:
* From the 'M' command-line flag.
* From the 'import_path' command-line flag.
* From the 'go_package' proto file option.
* From the proto package name in proto source file.
* From the path of the proto source file.
Technically, there are more than 5 ways since information can be derived
from a convoluted combination of all the methods.

We should move towards a sensible and consistent world where each
Go package information for each proto source file is:
* only derived from the command-line OR
* only derived from the proto source file itself.
It should never be derived from a mixture of methods.

In the future, all .proto source files will be required to have a
"go_package" option specified, unless the "M" command-line argument is
specified. If the "M" flag is given it takes precedence over 'go_package'.

This CL prints warnings if the user is generating proto files without
a "M" flag or "go_package" option specified. It suggests to the user
a "go_package" option that preserves the current semantics.

Change-Id: I5cf8d40a245146bb145b3b610d42f1bcd140b449
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/194158
Reviewed-by: Damien Neil <dneil@google.com>
This commit is contained in:
Joe Tsai 2019-09-07 13:06:27 -07:00
parent 8ee364e4bf
commit 3e80249d38

View File

@ -21,6 +21,7 @@ import (
"go/token"
"go/types"
"io/ioutil"
"log"
"os"
"path"
"path/filepath"
@ -164,6 +165,7 @@ func New(req *pluginpb.CodeGeneratorRequest, opts *Options) (*Plugin, error) {
packageNames := make(map[string]GoPackageName) // filename -> package name
importPaths := make(map[string]GoImportPath) // filename -> import path
mfiles := make(map[string]bool) // filename set
var packageImportPath GoImportPath
for _, param := range strings.Split(req.GetParameter(), ",") {
var value string
@ -196,6 +198,7 @@ func New(req *pluginpb.CodeGeneratorRequest, opts *Options) (*Plugin, error) {
default:
if param[0] == 'M' {
importPaths[param[1:]] = GoImportPath(value)
mfiles[param[1:]] = true
continue
}
if opts.ParamFunc != nil {
@ -261,10 +264,16 @@ func New(req *pluginpb.CodeGeneratorRequest, opts *Options) (*Plugin, error) {
}
for _, fdesc := range gen.Request.ProtoFile {
filename := fdesc.GetName()
packageName, _ := goPackageOption(fdesc)
packageName, importPath := goPackageOption(fdesc)
defaultPackageName := packageNameForImportPath[importPaths[filename]]
switch {
case packageName != "":
// TODO: For the "M" command-line argument, this means that the
// package name can be derived from the go_package option.
// Go package information should either consistently come from the
// command-line or the .proto source file, but not both.
// See how to make this consistent.
// Source file: option go_package = "quux/bar";
packageNames[filename] = packageName
case defaultPackageName != "":
@ -284,6 +293,33 @@ func New(req *pluginpb.CodeGeneratorRequest, opts *Options) (*Plugin, error) {
// Source filename.
packageNames[filename] = cleanPackageName(baseName(filename))
}
goPkgOpt := string(importPaths[filename])
if path.Base(string(goPkgOpt)) != string(packageNames[filename]) {
goPkgOpt += ";" + string(packageNames[filename])
}
switch {
case packageImportPath != "":
// Command line: import_path=quux/bar
log.Printf("WARNING: Deprecated use of the 'import_path' command-line argument. In %q, please specify:\n"+
"\toption go_package = %q;\n"+
"A future release of protoc-gen-go will no longer support the 'import_path' argument.\n"+
"\n", fdesc.GetName(), goPkgOpt)
case mfiles[filename]:
// Command line: M=foo.proto=quux/bar
case packageName != "" && importPath == "":
// Source file: option go_package = "quux";
log.Printf("WARNING: Deprecated use of 'go_package' option without a full import path in %q, please specify:\n"+
"\toption go_package = %q;\n"+
"A future release of protoc-gen-go will require the import path be specified.\n"+
"\n", fdesc.GetName(), goPkgOpt)
case packageName == "" && importPath == "":
// No Go package information provided.
log.Printf("WARNING: Missing 'go_package' option in %q, please specify:\n"+
"\toption go_package = %q;\n"+
"A future release of protoc-gen-go will require this be specified.\n"+
"\n", fdesc.GetName(), goPkgOpt)
}
}
// Consistency check: Every file with the same Go import path should have
@ -498,17 +534,26 @@ func goPackageOption(d *descriptorpb.FileDescriptorProto) (pkg GoPackageName, im
if opt == "" {
return "", ""
}
rawPkg, impPath := goPackageOptionRaw(opt)
pkg = cleanPackageName(rawPkg)
if string(pkg) != rawPkg && impPath != "" {
log.Printf("WARNING: Malformed 'go_package' option in %q, please specify:\n"+
"\toption go_package = %q;\n"+
"A future release of protoc-gen-go will reject this.\n"+
"\n", d.GetName(), string(impPath)+";"+string(pkg))
}
return pkg, impPath
}
func goPackageOptionRaw(opt string) (rawPkg string, impPath GoImportPath) {
// A semicolon-delimited suffix delimits the import path and package name.
if i := strings.Index(opt, ";"); i >= 0 {
// TODO: The package name is explicitly provided by the .proto file.
// Rather than sanitizing it, we should pass it verbatim.
return cleanPackageName(opt[i+1:]), GoImportPath(opt[:i])
return opt[i+1:], GoImportPath(opt[:i])
}
// The presence of a slash implies there's an import path.
if i := strings.LastIndex(opt, "/"); i >= 0 {
return cleanPackageName(opt[i+1:]), GoImportPath(opt)
return opt[i+1:], GoImportPath(opt)
}
return cleanPackageName(opt), ""
return opt, ""
}
// An Enum describes an enum.