diff --git a/compiler/protogen/protogen.go b/compiler/protogen/protogen.go index 5cd2a807f..118be73e3 100644 --- a/compiler/protogen/protogen.go +++ b/compiler/protogen/protogen.go @@ -109,6 +109,7 @@ type Plugin struct { messagesByName map[protoreflect.FullName]*Message annotateCode bool pathType pathType + module string genFiles []*GeneratedFile opts Options err error @@ -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": @@ -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. // @@ -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") { @@ -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), }) } diff --git a/compiler/protogen/protogen_test.go b/compiler/protogen/protogen_test.go index 0967516e5..36dbf72c8 100644 --- a/compiler/protogen/protogen_test.go +++ b/compiler/protogen/protogen_test.go @@ -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(` @@ -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) } } }