From 40dc264aaf3aff5f63650265cc67a2cc8cc49395 Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Fri, 30 Jan 2015 18:27:23 -0800 Subject: [PATCH 1/4] Revise codegen plugin to accommodate more cases and fix some bugs --- rpc/compiler/go_generator.cc | 101 ++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 48 deletions(-) diff --git a/rpc/compiler/go_generator.cc b/rpc/compiler/go_generator.cc index 21484f3b..0c2ce94a 100644 --- a/rpc/compiler/go_generator.cc +++ b/rpc/compiler/go_generator.cc @@ -44,6 +44,9 @@ using namespace std; namespace grpc_go_generator { +static map g_import_alias; +static set g_imports; + bool NoStreaming(const google::protobuf::MethodDescriptor* method) { return !method->client_streaming() && !method->server_streaming(); } @@ -88,25 +91,32 @@ std::string BadToUnderscore(std::string str) { return str; } -const string GetFullName(const string& selfPkg, - const string& msgPkg, - const string& msgName) { - if (selfPkg == msgPkg) { - return msgName; +string GenerateFullPackage(const google::protobuf::FileDescriptor* file) { + // In opensouce environment, assume each directory has at most one package. + size_t pos = file->name().find_last_of('/'); + if (pos != string::npos) { + return file->name().substr(0, pos); } - return BadToUnderscore(msgPkg) + "." + msgName; + return ""; +} + +const string GetFullName(const google::protobuf::Descriptor* desc) { + string pkg = GenerateFullPackage(desc->file()); + if (g_imports.find(pkg) == g_imports.end()) { + return desc->name(); + } + if (g_import_alias.find(pkg) != g_import_alias.end()) { + return g_import_alias[pkg] + "." + desc->name(); + } + return BadToUnderscore(desc->file()->package()) + "." + desc->name(); } void PrintClientMethodDef(google::protobuf::io::Printer* printer, const google::protobuf::MethodDescriptor* method, map* vars) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = GetFullName((*vars)["PackageName"], - method->input_type()->file()->package(), - method->input_type()->name()); - (*vars)["Response"] = GetFullName((*vars)["PackageName"], - method->output_type()->file()->package(), - method->output_type()->name()); + (*vars)["Request"] = GetFullName(method->input_type()); + (*vars)["Response"] = GetFullName(method->output_type()); if (NoStreaming(method)) { printer->Print(*vars, "\t$Method$(ctx context.Context, in *$Request$, opts " @@ -132,12 +142,8 @@ void PrintClientMethodImpl(google::protobuf::io::Printer* printer, const google::protobuf::MethodDescriptor* method, map* vars) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = GetFullName((*vars)["PackageName"], - method->input_type()->file()->package(), - method->input_type()->name()); - (*vars)["Response"] = GetFullName((*vars)["PackageName"], - method->output_type()->file()->package(), - method->output_type()->name()); + (*vars)["Request"] = GetFullName(method->input_type()); + (*vars)["Response"] = GetFullName(method->output_type()); if (NoStreaming(method)) { printer->Print( *vars, @@ -306,12 +312,8 @@ void PrintServerMethodDef(google::protobuf::io::Printer* printer, const google::protobuf::MethodDescriptor* method, map* vars) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = GetFullName((*vars)["PackageName"], - method->input_type()->file()->package(), - method->input_type()->name()); - (*vars)["Response"] = GetFullName((*vars)["PackageName"], - method->output_type()->file()->package(), - method->output_type()->name()); + (*vars)["Request"] = GetFullName(method->input_type()); + (*vars)["Response"] = GetFullName(method->output_type()); if (NoStreaming(method)) { printer->Print( *vars, @@ -330,12 +332,8 @@ void PrintServerHandler(google::protobuf::io::Printer* printer, const google::protobuf::MethodDescriptor* method, map* vars) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = GetFullName((*vars)["PackageName"], - method->input_type()->file()->package(), - method->input_type()->name()); - (*vars)["Response"] = GetFullName((*vars)["PackageName"], - method->output_type()->file()->package(), - method->output_type()->name()); + (*vars)["Request"] = GetFullName(method->input_type()); + (*vars)["Response"] = GetFullName(method->output_type()); if (NoStreaming(method)) { printer->Print( *vars, @@ -513,42 +511,49 @@ void PrintServer(google::protobuf::io::Printer* printer, "}\n\n"); } +bool IsSelfImport(const google::protobuf::FileDescriptor* self, + const google::protobuf::FileDescriptor* import) { + if (GenerateFullPackage(self) == GenerateFullPackage(import)) { + return true; + } + return false; +} + void PrintMessageImports( google::protobuf::io::Printer* printer, const google::protobuf::FileDescriptor* file, map* vars) { set descs; - set importedPkgs; for (int i = 0; i < file->service_count(); ++i) { const google::protobuf::ServiceDescriptor* service = file->service(i); for (int j = 0; j < service->method_count(); ++j) { - const google::protobuf::MethodDescriptor* method = service->method(i); - // Remove duplicated imports. - if (importedPkgs.find( - method->input_type()->file()->package()) == importedPkgs.end()) { + const google::protobuf::MethodDescriptor* method = service->method(j); + if (!IsSelfImport(file, method->input_type()->file())) { descs.insert(method->input_type()->file()); - importedPkgs.insert(method->input_type()->file()->package()); } - if (importedPkgs.find( - method->output_type()->file()->package()) == importedPkgs.end()) { + if (!IsSelfImport(file, method->output_type()->file())) { descs.insert(method->output_type()->file()); - importedPkgs.insert(method->output_type()->file()->package()); } } } + int idx = 0; for (auto fd : descs) { - if (fd->package() == (*vars)["PackageName"]) { - continue; + string pkg = GenerateFullPackage(fd); + if (pkg != "") { + g_imports.insert(pkg); + if (file->package() == fd->package()) { + // the same package name in different directories. Require an alias. + g_import_alias[pkg] = "apb" + std::to_string(idx++); + } } - string name = fd->name(); - string import_path = "import \""; - if (name.find('/') == string::npos) { - // Assume all the proto in the same directory belong to the same package. - continue; - } else { - import_path += name.substr(0, name.find_last_of('/')) + "\""; + } + for (auto import : g_imports) { + string import_path = "import "; + if (g_import_alias.find(import) != g_import_alias.end()) { + import_path += g_import_alias[import] + " "; } + import_path += "\"" + import + "\""; printer->Print(import_path.c_str()); printer->Print("\n"); } From 55680c22a195add8da2aa176ca7042b017dce630 Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Mon, 2 Feb 2015 16:37:58 -0800 Subject: [PATCH 2/4] i) Eliminate the global vars; ii) properly handle the case that an external imported package contains multiple files; and iii) some function name touch-up --- rpc/compiler/go_generator.cc | 104 +++++++++++++++++++++-------------- 1 file changed, 64 insertions(+), 40 deletions(-) diff --git a/rpc/compiler/go_generator.cc b/rpc/compiler/go_generator.cc index 0c2ce94a..f5517436 100644 --- a/rpc/compiler/go_generator.cc +++ b/rpc/compiler/go_generator.cc @@ -42,11 +42,9 @@ using namespace std; +// TODO(zhaoq): Support go_package option. namespace grpc_go_generator { -static map g_import_alias; -static set g_imports; - bool NoStreaming(const google::protobuf::MethodDescriptor* method) { return !method->client_streaming() && !method->server_streaming(); } @@ -91,7 +89,7 @@ std::string BadToUnderscore(std::string str) { return str; } -string GenerateFullPackage(const google::protobuf::FileDescriptor* file) { +string GenerateFullGoPackage(const google::protobuf::FileDescriptor* file) { // In opensouce environment, assume each directory has at most one package. size_t pos = file->name().find_last_of('/'); if (pos != string::npos) { @@ -100,23 +98,30 @@ string GenerateFullPackage(const google::protobuf::FileDescriptor* file) { return ""; } -const string GetFullName(const google::protobuf::Descriptor* desc) { - string pkg = GenerateFullPackage(desc->file()); - if (g_imports.find(pkg) == g_imports.end()) { +const string GetFullMessageQualifiedName( + const google::protobuf::Descriptor* desc, + set& imports, + map& import_alias) { + string pkg = GenerateFullGoPackage(desc->file()); + if (imports.find(pkg) == imports.end()) { return desc->name(); } - if (g_import_alias.find(pkg) != g_import_alias.end()) { - return g_import_alias[pkg] + "." + desc->name(); + if (import_alias.find(pkg) != import_alias.end()) { + return import_alias[pkg] + "." + desc->name(); } return BadToUnderscore(desc->file()->package()) + "." + desc->name(); } void PrintClientMethodDef(google::protobuf::io::Printer* printer, const google::protobuf::MethodDescriptor* method, - map* vars) { + map* vars, + set& imports, + map& import_alias) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = GetFullName(method->input_type()); - (*vars)["Response"] = GetFullName(method->output_type()); + (*vars)["Request"] = + GetFullMessageQualifiedName(method->input_type(), imports, import_alias); + (*vars)["Response"] = + GetFullMessageQualifiedName(method->output_type(), imports, import_alias); if (NoStreaming(method)) { printer->Print(*vars, "\t$Method$(ctx context.Context, in *$Request$, opts " @@ -140,10 +145,14 @@ void PrintClientMethodDef(google::protobuf::io::Printer* printer, void PrintClientMethodImpl(google::protobuf::io::Printer* printer, const google::protobuf::MethodDescriptor* method, - map* vars) { + map* vars, + set& imports, + map& import_alias) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = GetFullName(method->input_type()); - (*vars)["Response"] = GetFullName(method->output_type()); + (*vars)["Request"] = + GetFullMessageQualifiedName(method->input_type(), imports, import_alias); + (*vars)["Response"] = + GetFullMessageQualifiedName(method->output_type(), imports, import_alias); if (NoStreaming(method)) { printer->Print( *vars, @@ -285,12 +294,14 @@ void PrintClientMethodImpl(google::protobuf::io::Printer* printer, void PrintClient(google::protobuf::io::Printer* printer, const google::protobuf::ServiceDescriptor* service, - map* vars) { + map* vars, + set& imports, + map& import_alias) { (*vars)["Service"] = service->name(); (*vars)["ServiceStruct"] = LowerCaseService(service->name()); printer->Print(*vars, "type $Service$Client interface {\n"); for (int i = 0; i < service->method_count(); ++i) { - PrintClientMethodDef(printer, service->method(i), vars); + PrintClientMethodDef(printer, service->method(i), vars, imports, import_alias); } printer->Print("}\n\n"); @@ -304,16 +315,20 @@ void PrintClient(google::protobuf::io::Printer* printer, "\treturn &$ServiceStruct$Client{cc}\n" "}\n\n"); for (int i = 0; i < service->method_count(); ++i) { - PrintClientMethodImpl(printer, service->method(i), vars); + PrintClientMethodImpl(printer, service->method(i), vars, imports, import_alias); } } void PrintServerMethodDef(google::protobuf::io::Printer* printer, const google::protobuf::MethodDescriptor* method, - map* vars) { + map* vars, + set& imports, + map& import_alias) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = GetFullName(method->input_type()); - (*vars)["Response"] = GetFullName(method->output_type()); + (*vars)["Request"] = + GetFullMessageQualifiedName(method->input_type(), imports, import_alias); + (*vars)["Response"] = + GetFullMessageQualifiedName(method->output_type(), imports, import_alias); if (NoStreaming(method)) { printer->Print( *vars, @@ -330,10 +345,14 @@ void PrintServerMethodDef(google::protobuf::io::Printer* printer, void PrintServerHandler(google::protobuf::io::Printer* printer, const google::protobuf::MethodDescriptor* method, - map* vars) { + map* vars, + set& imports, + map& import_alias) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = GetFullName(method->input_type()); - (*vars)["Response"] = GetFullName(method->output_type()); + (*vars)["Request"] = + GetFullMessageQualifiedName(method->input_type(), imports, import_alias); + (*vars)["Response"] = + GetFullMessageQualifiedName(method->output_type(), imports, import_alias); if (NoStreaming(method)) { printer->Print( *vars, @@ -471,11 +490,13 @@ void PrintServerStreamingMethodDesc( void PrintServer(google::protobuf::io::Printer* printer, const google::protobuf::ServiceDescriptor* service, - map* vars) { + map* vars, + set& imports, + map& import_alias) { (*vars)["Service"] = service->name(); printer->Print(*vars, "type $Service$Server interface {\n"); for (int i = 0; i < service->method_count(); ++i) { - PrintServerMethodDef(printer, service->method(i), vars); + PrintServerMethodDef(printer, service->method(i), vars, imports, import_alias); } printer->Print("}\n\n"); @@ -485,7 +506,7 @@ void PrintServer(google::protobuf::io::Printer* printer, "}\n\n"); for (int i = 0; i < service->method_count(); ++i) { - PrintServerHandler(printer, service->method(i), vars); + PrintServerHandler(printer, service->method(i), vars, imports, import_alias); } printer->Print(*vars, @@ -513,7 +534,7 @@ void PrintServer(google::protobuf::io::Printer* printer, bool IsSelfImport(const google::protobuf::FileDescriptor* self, const google::protobuf::FileDescriptor* import) { - if (GenerateFullPackage(self) == GenerateFullPackage(import)) { + if (GenerateFullGoPackage(self) == GenerateFullGoPackage(import)) { return true; } return false; @@ -522,7 +543,9 @@ bool IsSelfImport(const google::protobuf::FileDescriptor* self, void PrintMessageImports( google::protobuf::io::Printer* printer, const google::protobuf::FileDescriptor* file, - map* vars) { + map* vars, + set* imports, + map* import_alias) { set descs; for (int i = 0; i < file->service_count(); ++i) { const google::protobuf::ServiceDescriptor* service = file->service(i); @@ -539,19 +562,19 @@ void PrintMessageImports( int idx = 0; for (auto fd : descs) { - string pkg = GenerateFullPackage(fd); + string pkg = GenerateFullGoPackage(fd); if (pkg != "") { - g_imports.insert(pkg); - if (file->package() == fd->package()) { + auto ret = imports->insert(pkg); + if (ret.second == true && file->package() == fd->package()) { // the same package name in different directories. Require an alias. - g_import_alias[pkg] = "apb" + std::to_string(idx++); + (*import_alias)[pkg] = "apb" + std::to_string(idx++); } } } - for (auto import : g_imports) { + for (auto import : *imports) { string import_path = "import "; - if (g_import_alias.find(import) != g_import_alias.end()) { - import_path += g_import_alias[import] + " "; + if (import_alias->find(import) != import_alias->end()) { + import_path += (*import_alias)[import] + " "; } import_path += "\"" + import + "\""; printer->Print(import_path.c_str()); @@ -565,7 +588,8 @@ string GetServices(const google::protobuf::FileDescriptor* file) { google::protobuf::io::StringOutputStream output_stream(&output); google::protobuf::io::Printer printer(&output_stream, '$'); map vars; - + map import_alias; + set imports; string package_name = !file->options().go_package().empty() ? file->options().go_package() : file->package(); @@ -583,7 +607,7 @@ string GetServices(const google::protobuf::FileDescriptor* file) { "\tproto \"github.com/golang/protobuf/proto\"\n" ")\n\n"); - PrintMessageImports(&printer, file, &vars); + PrintMessageImports(&printer, file, &vars, &imports, &import_alias); // $Package$ is used to fully qualify method names. vars["Package"] = file->package(); @@ -592,9 +616,9 @@ string GetServices(const google::protobuf::FileDescriptor* file) { } for (int i = 0; i < file->service_count(); ++i) { - PrintClient(&printer, file->service(0), &vars); + PrintClient(&printer, file->service(0), &vars, imports, import_alias); printer.Print("\n"); - PrintServer(&printer, file->service(0), &vars); + PrintServer(&printer, file->service(0), &vars, imports, import_alias); printer.Print("\n"); } return output; From 0f4f9d04d32acfd5d47fa8bd966c280869826c6c Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Mon, 2 Feb 2015 17:02:29 -0800 Subject: [PATCH 3/4] resolved some comments --- rpc/compiler/go_generator.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rpc/compiler/go_generator.cc b/rpc/compiler/go_generator.cc index f5517436..9085f4d6 100644 --- a/rpc/compiler/go_generator.cc +++ b/rpc/compiler/go_generator.cc @@ -104,9 +104,12 @@ const string GetFullMessageQualifiedName( map& import_alias) { string pkg = GenerateFullGoPackage(desc->file()); if (imports.find(pkg) == imports.end()) { + // The message is in the same package as the services definition. return desc->name(); } if (import_alias.find(pkg) != import_alias.end()) { + // The message is in a package whose name is as same as the one consisting + // of the service definition. Use the alias to differentiate. return import_alias[pkg] + "." + desc->name(); } return BadToUnderscore(desc->file()->package()) + "." + desc->name(); @@ -565,7 +568,7 @@ void PrintMessageImports( string pkg = GenerateFullGoPackage(fd); if (pkg != "") { auto ret = imports->insert(pkg); - if (ret.second == true && file->package() == fd->package()) { + if (ret.second && file->package() == fd->package()) { // the same package name in different directories. Require an alias. (*import_alias)[pkg] = "apb" + std::to_string(idx++); } From 4a1cba09a6b7f2319a749e754bc19df5990708df Mon Sep 17 00:00:00 2001 From: iamqizhao Date: Mon, 2 Feb 2015 17:05:03 -0800 Subject: [PATCH 4/4] add a comment --- rpc/compiler/go_generator.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rpc/compiler/go_generator.cc b/rpc/compiler/go_generator.cc index 9085f4d6..bbc5a562 100644 --- a/rpc/compiler/go_generator.cc +++ b/rpc/compiler/go_generator.cc @@ -568,6 +568,8 @@ void PrintMessageImports( string pkg = GenerateFullGoPackage(fd); if (pkg != "") { auto ret = imports->insert(pkg); + // Use ret.second to guarantee if a package spans multiple files, it only + // gets 1 alias. if (ret.second && file->package() == fd->package()) { // the same package name in different directories. Require an alias. (*import_alias)[pkg] = "apb" + std::to_string(idx++);