compiler/protogen: use consistent options pattern
Throughout the module options usually expressed as a struct where the
acting function is a method hanging off the options type.
Use this pattern for protogen as well.
Change-Id: I533a61387cb74971e4efc9313d400b66b8aac451
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/221424
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/cmd/protoc-gen-go/main.go b/cmd/protoc-gen-go/main.go
index 2cb3911..9ac544b 100644
--- a/cmd/protoc-gen-go/main.go
+++ b/cmd/protoc-gen-go/main.go
@@ -31,11 +31,10 @@
flags flag.FlagSet
plugins = flags.String("plugins", "", "deprecated option")
importPrefix = flags.String("import_prefix", "", "deprecated option")
- opts = &protogen.Options{
- ParamFunc: flags.Set,
- }
)
- protogen.Run(opts, func(gen *protogen.Plugin) error {
+ protogen.Options{
+ ParamFunc: flags.Set,
+ }.Run(func(gen *protogen.Plugin) error {
if *plugins != "" {
return errors.New("protoc-gen-go: plugins are not supported; use 'protoc --go-grpc_out=...' to generate gRPC")
}
diff --git a/compiler/protogen/protogen.go b/compiler/protogen/protogen.go
index 35b7295..9299404 100644
--- a/compiler/protogen/protogen.go
+++ b/compiler/protogen/protogen.go
@@ -50,16 +50,14 @@
//
// If a failure occurs while reading or writing, Run prints an error to
// os.Stderr and calls os.Exit(1).
-//
-// Passing a nil options is equivalent to passing a zero-valued one.
-func Run(opts *Options, f func(*Plugin) error) {
+func (opts Options) Run(f func(*Plugin) error) {
if err := run(opts, f); err != nil {
fmt.Fprintf(os.Stderr, "%s: %v\n", filepath.Base(os.Args[0]), err)
os.Exit(1)
}
}
-func run(opts *Options, f func(*Plugin) error) error {
+func run(opts Options, f func(*Plugin) error) error {
if len(os.Args) > 1 {
return fmt.Errorf("unknown argument %q (this program should be run by protoc, not directly)", os.Args[1])
}
@@ -71,7 +69,7 @@
if err := proto.Unmarshal(in, req); err != nil {
return err
}
- gen, err := New(req, opts)
+ gen, err := opts.New(req)
if err != nil {
return err
}
@@ -112,11 +110,10 @@
annotateCode bool
pathType pathType
genFiles []*GeneratedFile
- opts *Options
+ opts Options
err error
}
-// Options are optional parameters to New.
type Options struct {
// If ParamFunc is non-nil, it will be called with each unknown
// generator parameter.
@@ -150,12 +147,7 @@
}
// New returns a new Plugin.
-//
-// Passing a nil Options is equivalent to passing a zero-valued one.
-func New(req *pluginpb.CodeGeneratorRequest, opts *Options) (*Plugin, error) {
- if opts == nil {
- opts = &Options{}
- }
+func (opts Options) New(req *pluginpb.CodeGeneratorRequest) (*Plugin, error) {
gen := &Plugin{
Request: req,
FilesByPath: make(map[string]*File),
diff --git a/compiler/protogen/protogen_test.go b/compiler/protogen/protogen_test.go
index 6f08b2e..419305f 100644
--- a/compiler/protogen/protogen_test.go
+++ b/compiler/protogen/protogen_test.go
@@ -21,13 +21,12 @@
func TestPluginParameters(t *testing.T) {
var flags flag.FlagSet
value := flags.Int("integer", 0, "")
- opts := &Options{
- ParamFunc: flags.Set,
- }
const params = "integer=2"
- _, err := New(&pluginpb.CodeGeneratorRequest{
+ _, err := Options{
+ ParamFunc: flags.Set,
+ }.New(&pluginpb.CodeGeneratorRequest{
Parameter: proto.String(params),
- }, opts)
+ })
if err != nil {
t.Errorf("New(generator parameters %q): %v", params, err)
}
@@ -43,12 +42,11 @@
} {
var flags flag.FlagSet
flags.Bool("boolean", false, "")
- opts := &Options{
+ _, err := Options{
ParamFunc: flags.Set,
- }
- _, err := New(&pluginpb.CodeGeneratorRequest{
+ }.New(&pluginpb.CodeGeneratorRequest{
Parameter: proto.String(parameter),
- }, opts)
+ })
if err == nil {
t.Errorf("New(generator parameters %q): want error, got nil", parameter)
}
@@ -56,7 +54,7 @@
}
func TestNoGoPackage(t *testing.T) {
- gen, err := New(&pluginpb.CodeGeneratorRequest{
+ gen, err := Options{}.New(&pluginpb.CodeGeneratorRequest{
ProtoFile: []*descriptorpb.FileDescriptorProto{
{
Name: proto.String("testdata/go_package/no_go_package.proto"),
@@ -70,7 +68,7 @@
Dependency: []string{"testdata/go_package/no_go_package.proto"},
},
},
- }, nil)
+ })
if err != nil {
t.Fatal(err)
}
@@ -182,7 +180,7 @@
if test.generate {
req.FileToGenerate = []string{filename}
}
- gen, err := New(req, nil)
+ gen, err := Options{}.New(req)
if err != nil {
t.Errorf("%vNew(req) = %v", context, err)
continue
@@ -205,7 +203,7 @@
}
func TestPackageNameInference(t *testing.T) {
- gen, err := New(&pluginpb.CodeGeneratorRequest{
+ gen, err := Options{}.New(&pluginpb.CodeGeneratorRequest{
ProtoFile: []*descriptorpb.FileDescriptorProto{
{
Name: proto.String("dir/file1.proto"),
@@ -220,7 +218,7 @@
},
},
FileToGenerate: []string{"dir/file1.proto", "dir/file2.proto"},
- }, nil)
+ })
if err != nil {
t.Fatalf("New(req) = %v", err)
}
@@ -232,7 +230,7 @@
}
func TestInconsistentPackageNames(t *testing.T) {
- _, err := New(&pluginpb.CodeGeneratorRequest{
+ _, err := Options{}.New(&pluginpb.CodeGeneratorRequest{
ProtoFile: []*descriptorpb.FileDescriptorProto{
{
Name: proto.String("dir/file1.proto"),
@@ -250,14 +248,14 @@
},
},
FileToGenerate: []string{"dir/file1.proto", "dir/file2.proto"},
- }, nil)
+ })
if err == nil {
t.Fatalf("inconsistent package names for the same import path: New(req) = nil, want error")
}
}
func TestImports(t *testing.T) {
- gen, err := New(&pluginpb.CodeGeneratorRequest{}, nil)
+ gen, err := Options{}.New(&pluginpb.CodeGeneratorRequest{})
if err != nil {
t.Fatal(err)
}
@@ -303,11 +301,11 @@
}
func TestImportRewrites(t *testing.T) {
- gen, err := New(&pluginpb.CodeGeneratorRequest{}, &Options{
+ gen, err := Options{
ImportRewriteFunc: func(i GoImportPath) GoImportPath {
return "prefix/" + i
},
- })
+ }.New(&pluginpb.CodeGeneratorRequest{})
if err != nil {
t.Fatal(err)
}
diff --git a/internal/cmd/generate-protos/main.go b/internal/cmd/generate-protos/main.go
index c217e66..0f96484 100644
--- a/internal/cmd/generate-protos/main.go
+++ b/internal/cmd/generate-protos/main.go
@@ -71,7 +71,7 @@
// This is reasonable since we fully control the output.
detrand.Disable()
- protogen.Run(nil, func(gen *protogen.Plugin) error {
+ protogen.Options{}.Run(func(gen *protogen.Plugin) error {
for _, file := range gen.Files {
if file.Generate {
gengo.GenerateVersionMarkers = false
