Skip to content

Commit

Permalink
compiler/protogen: add module= generator option
Browse files Browse the repository at this point in the history 
Add a generator option that strips a prefix from the generated
filenames.

Consider this case: We have google/protobuf/empty.proto, with a
go_package option of "google.golang.org/protobuf/types/known/emptypb".
We want to generate the code for this file, placing it into the
appropriate directory of our repository.

In the default mode used by the code generator (paths=import),
the generator outputs the file:

	google.golang.org/protobuf/types/known/emptypb/empty.pb.go

This is close to what we want, but has an unnecessary
"google.golang.org/protobuf/" prefix. In the GOPATH world, we could pass
--go_out=$GOPATH to protoc and get a generated file in the desired
location, but this path is not useful in the modules world.

The 'module' option allows us to strip off the module prefix, generating
the desired filename (types/known/emptypb/empty.pb.go):

	protoc --go_out=. --go_opt=module=google.golang.org/protobuf google/protobuf/empty.proto

The module name must be an exact, character-for-character match. This
matches protoc's file handling in general.

Default to and require the paths=import option when module= is
specified, since it only makes sense when combined with it.

Updates golang/protobuf#992.

Change-Id: Idbfe4826b6c0ece30d64dbc577131a4f16391936
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/219298
Reviewed-by: Joe Tsai <[email protected]>
  • Loading branch information
@neild
neild committed Mar 20, 2020
1 parent 9d39786 commit ffbc5fd
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 69 deletions.
29 changes: 27 additions & 2 deletions compiler/protogen/protogen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ type Plugin struct {
messagesByName map[protoreflect.FullName]*Message
annotateCode bool
pathType pathType
module string
genFiles []*GeneratedFile
opts Options
err error
Expand Down Expand Up @@ -172,6 +173,8 @@ func (opts Options) New(req *pluginpb.CodeGeneratorRequest) (*Plugin, error) {
// Ignore.
case "import_path":
packageImportPath = GoImportPath(value)
case "module":
gen.module = value
case "paths":
switch value {
case "import":
Expand Down Expand Up @@ -210,6 +213,18 @@ func (opts Options) New(req *pluginpb.CodeGeneratorRequest) (*Plugin, error) {
}
}
}
if gen.module != "" {
// When the module= option is provided, we strip the module name
// prefix from generated files. This only makes sense if generated
// filenames are based on the import path, so default to paths=import
// and complain if source_relative was selected manually.
switch gen.pathType {
case pathTypeLegacy:
gen.pathType = pathTypeImport
case pathTypeSourceRelative:
return nil, fmt.Errorf("cannot use module= with paths=source_relative")
}
}

// Figure out the import path and package name for each file.
//
Expand Down Expand Up @@ -396,8 +411,18 @@ func (gen *Plugin) Response() *pluginpb.CodeGeneratorResponse {
Error: proto.String(err.Error()),
}
}
filename := g.filename
if gen.module != "" {
trim := gen.module + "/"
if !strings.HasPrefix(filename, trim) {
return &pluginpb.CodeGeneratorResponse{
Error: proto.String(fmt.Sprintf("%v: generated file does not match prefix %q", filename, gen.module)),
}
}
filename = strings.TrimPrefix(filename, trim)
}
resp.File = append(resp.File, &pluginpb.CodeGeneratorResponse_File{
Name: proto.String(g.filename),
Name: proto.String(filename),
Content: proto.String(string(content)),
})
if gen.annotateCode && strings.HasSuffix(g.filename, ".go") {
Expand All @@ -408,7 +433,7 @@ func (gen *Plugin) Response() *pluginpb.CodeGeneratorResponse {
}
}
resp.File = append(resp.File, &pluginpb.CodeGeneratorResponse_File{
Name: proto.String(g.filename + ".meta"),
Name: proto.String(filename + ".meta"),
Content: proto.String(meta),
})
}
Expand Down
153 changes: 86 additions & 67 deletions compiler/protogen/protogen_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,89 +93,106 @@ func TestPackageNamesAndPaths(t *testing.T) {
protoPackageName = "proto.package"
)
for _, test := range []struct {
desc string
parameter string
goPackageOption string
generate bool
wantPackageName GoPackageName
wantImportPath GoImportPath
wantFilenamePrefix string
desc string
parameter string
goPackageOption string
generate bool
wantPackageName GoPackageName
wantImportPath GoImportPath
wantFilename string
}{
{
desc: "no parameters, no go_package option",
generate: true,
wantPackageName: "proto_package",
wantImportPath: "dir",
wantFilenamePrefix: "dir/filename",
desc: "no parameters, no go_package option",
generate: true,
wantPackageName: "proto_package",
wantImportPath: "dir",
wantFilename: "dir/filename",
},
{
desc: "go_package option sets import path",
goPackageOption: "golang.org/x/foo",
generate: true,
wantPackageName: "foo",
wantImportPath: "golang.org/x/foo",
wantFilenamePrefix: "golang.org/x/foo/filename",
desc: "go_package option sets import path",
goPackageOption: "golang.org/x/foo",
generate: true,
wantPackageName: "foo",
wantImportPath: "golang.org/x/foo",
wantFilename: "golang.org/x/foo/filename",
},
{
desc: "go_package option sets import path and package",
goPackageOption: "golang.org/x/foo;bar",
generate: true,
wantPackageName: "bar",
wantImportPath: "golang.org/x/foo",
wantFilenamePrefix: "golang.org/x/foo/filename",
desc: "go_package option sets import path and package",
goPackageOption: "golang.org/x/foo;bar",
generate: true,
wantPackageName: "bar",
wantImportPath: "golang.org/x/foo",
wantFilename: "golang.org/x/foo/filename",
},
{
desc: "go_package option sets package",
goPackageOption: "foo",
generate: true,
wantPackageName: "foo",
wantImportPath: "dir",
wantFilenamePrefix: "dir/filename",
desc: "go_package option sets package",
goPackageOption: "foo",
generate: true,
wantPackageName: "foo",
wantImportPath: "dir",
wantFilename: "dir/filename",
},
{
desc: "command line sets import path for a file",
parameter: "Mdir/filename.proto=golang.org/x/bar",
goPackageOption: "golang.org/x/foo",
generate: true,
wantPackageName: "foo",
wantImportPath: "golang.org/x/bar",
wantFilenamePrefix: "golang.org/x/foo/filename",
desc: "command line sets import path for a file",
parameter: "Mdir/filename.proto=golang.org/x/bar",
goPackageOption: "golang.org/x/foo",
generate: true,
wantPackageName: "foo",
wantImportPath: "golang.org/x/bar",
wantFilename: "golang.org/x/foo/filename",
},
{
desc: "command line sets import path for a file with package name specified",
parameter: "Mdir/filename.proto=golang.org/x/bar;bar",
goPackageOption: "golang.org/x/foo",
generate: true,
wantPackageName: "bar",
wantImportPath: "golang.org/x/bar",
wantFilenamePrefix: "golang.org/x/foo/filename",
desc: "command line sets import path for a file with package name specified",
parameter: "Mdir/filename.proto=golang.org/x/bar;bar",
goPackageOption: "golang.org/x/foo",
generate: true,
wantPackageName: "bar",
wantImportPath: "golang.org/x/bar",
wantFilename: "golang.org/x/foo/filename",
},
{
desc: "import_path parameter sets import path of generated files",
parameter: "import_path=golang.org/x/bar",
goPackageOption: "golang.org/x/foo",
generate: true,
wantPackageName: "foo",
wantImportPath: "golang.org/x/bar",
wantFilenamePrefix: "golang.org/x/foo/filename",
desc: "import_path parameter sets import path of generated files",
parameter: "import_path=golang.org/x/bar",
goPackageOption: "golang.org/x/foo",
generate: true,
wantPackageName: "foo",
wantImportPath: "golang.org/x/bar",
wantFilename: "golang.org/x/foo/filename",
},
{
desc: "import_path parameter does not set import path of dependencies",
parameter: "import_path=golang.org/x/bar",
goPackageOption: "golang.org/x/foo",
generate: false,
wantPackageName: "foo",
wantImportPath: "golang.org/x/foo",
wantFilenamePrefix: "golang.org/x/foo/filename",
desc: "import_path parameter does not set import path of dependencies",
parameter: "import_path=golang.org/x/bar",
goPackageOption: "golang.org/x/foo",
generate: false,
wantPackageName: "foo",
wantImportPath: "golang.org/x/foo",
wantFilename: "golang.org/x/foo/filename",
},
{
desc: "paths=import uses import path from command line",
parameter: "paths=import,Mdir/filename.proto=golang.org/x/bar",
goPackageOption: "golang.org/x/foo",
generate: true,
wantPackageName: "foo",
wantImportPath: "golang.org/x/bar",
wantFilenamePrefix: "golang.org/x/bar/filename",
desc: "module option set",
parameter: "module=golang.org/x",
goPackageOption: "golang.org/x/foo",
generate: false,
wantPackageName: "foo",
wantImportPath: "golang.org/x/foo",
wantFilename: "foo/filename",
},
{
desc: "paths=import uses import path from command line",
parameter: "paths=import,Mdir/filename.proto=golang.org/x/bar",
goPackageOption: "golang.org/x/foo",
generate: true,
wantPackageName: "foo",
wantImportPath: "golang.org/x/bar",
wantFilename: "golang.org/x/bar/filename",
},
{
desc: "module option implies paths=import",
parameter: "module=golang.org/x,Mdir/filename.proto=golang.org/x/foo",
generate: false,
wantPackageName: "proto_package",
wantImportPath: "golang.org/x/foo",
wantFilename: "foo/filename",
},
} {
context := fmt.Sprintf(`
Expand Down Expand Up @@ -218,8 +235,10 @@ TEST: %v
if got, want := gotFile.GoImportPath, test.wantImportPath; got != want {
t.Errorf("%vGoImportPath=%v, want %v", context, got, want)
}
if got, want := gotFile.GeneratedFilenamePrefix, test.wantFilenamePrefix; got != want {
t.Errorf("%vGeneratedFilenamePrefix=%v, want %v", context, got, want)
gen.NewGeneratedFile(gotFile.GeneratedFilenamePrefix, "")
resp := gen.Response()
if got, want := resp.File[0].GetName(), test.wantFilename; got != want {
t.Errorf("%vgenerated filename=%v, want %v", context, got, want)
}
}
}
Expand Down

0 comments on commit ffbc5fd

Please sign in to comment.