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>
diff --git a/compiler/protogen/protogen.go b/compiler/protogen/protogen.go
index 3be898d..5dbdaf9 100644
--- a/compiler/protogen/protogen.go
+++ b/compiler/protogen/protogen.go
@@ -21,6 +21,7 @@
"go/token"
"go/types"
"io/ioutil"
+ "log"
"os"
"path"
"path/filepath"
@@ -164,6 +165,7 @@
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 @@
default:
if param[0] == 'M' {
importPaths[param[1:]] = GoImportPath(value)
+ mfiles[param[1:]] = true
continue
}
if opts.ParamFunc != nil {
@@ -261,10 +264,16 @@
}
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 @@
// 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 @@
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.
