[thrift] branch master updated: THRIFT-4914: Fix name redeclaration bug in compiled go code

[yuxuan]

This is an automated email from the ASF dual-hosted git repository.

yuxuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git


The following commit(s) were added to refs/heads/master by this push:
     new a2522c0  THRIFT-4914: Fix name redeclaration bug in compiled go code
a2522c0 is described below

commit a2522c0c8329a76bab42af251f06bd348ad0ecbd
Author: Yuxuan 'fishy' Wang <yuxuan.w...@reddit.com>
AuthorDate: Wed Feb 17 12:58:40 2021 -0800

    THRIFT-4914: Fix name redeclaration bug in compiled go code
    
    Client: go
    
    This fixes the bug reported in
    https://github.com/apache/thrift/pull/2315#discussion_r577919697.
---
 compiler/cpp/src/thrift/generate/t_go_generator.cc | 17 +++++++++--------
 test/ThriftTest.thrift                             | 14 ++++++++++++++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc 
b/compiler/cpp/src/thrift/generate/t_go_generator.cc
index d9f9d51..2cef11f 100644
--- a/compiler/cpp/src/thrift/generate/t_go_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_go_generator.cc
@@ -2170,14 +2170,15 @@ void t_go_generator::generate_service_client(t_service* 
tservice) {
     }
 
     if (!(*f_iter)->is_oneway()) {
+      std::string metaName = tmp("_meta");
       std::string resultName = tmp("_result");
       std::string resultType = publicize(method + "_result", true);
       f_types_ << indent() << "var " << resultName << " " << resultType << 
endl;
-      f_types_ << indent() << "var meta thrift.ResponseMeta" << endl;
-      f_types_ << indent() << "meta, err = p.Client_().Call(ctx, \""
+      f_types_ << indent() << "var " << metaName << " thrift.ResponseMeta" << 
endl;
+      f_types_ << indent() << metaName << ", _err = p.Client_().Call(ctx, \""
         << method << "\", &" << argsName << ", &" << resultName << ")" << endl;
-      f_types_ << indent() << "p.SetLastResponseMeta_(meta)" << endl;
-      f_types_ << indent() << "if err != nil {" << endl;
+      f_types_ << indent() << "p.SetLastResponseMeta_(" << metaName << ")" << 
endl;
+      f_types_ << indent() << "if _err != nil {" << endl;
 
       indent_up();
       f_types_ << indent() << "return" << endl;
@@ -2199,7 +2200,7 @@ void t_go_generator::generate_service_client(t_service* 
tservice) {
           indent_up();
 
           if (!(*f_iter)->get_returntype()->is_void()) {
-            f_types_ << indent() << "return r, " << field << endl;
+            f_types_ << indent() << "return _r, " << field << endl;
           } else {
             f_types_ << indent() << "return "<< field << endl;
           }
@@ -2222,7 +2223,7 @@ void t_go_generator::generate_service_client(t_service* 
tservice) {
       f_types_ << indent() << "p.SetLastResponseMeta_(thrift.ResponseMeta{})" 
<< endl;
       // TODO: would be nice to not to duplicate the call generation
       f_types_ << indent() << "if _, err := p.Client_().Call(ctx, \""
-        << method << "\", &"<< argsName << ", nil); err != nil {" << endl;
+        << method << "\", &" << argsName << ", nil); err != nil {" << endl;
 
       indent_up();
       f_types_ << indent() << "return err" << endl;
@@ -3787,7 +3788,7 @@ string t_go_generator::function_signature_if(t_function* 
tfunction, string prefi
   string errs = argument_list(exceptions);
 
   if (!ret->is_void()) {
-    signature += "r " + type_to_go_type(ret);
+    signature += "_r " + type_to_go_type(ret);
 
     if (addError || errs.size() == 0) {
       signature += ", ";
@@ -3795,7 +3796,7 @@ string t_go_generator::function_signature_if(t_function* 
tfunction, string prefi
   }
 
   if (addError) {
-    signature += "err error";
+    signature += "_err error";
   }
 
   signature += ")";
diff --git a/test/ThriftTest.thrift b/test/ThriftTest.thrift
index ac49aee..c88187c 100644
--- a/test/ThriftTest.thrift
+++ b/test/ThriftTest.thrift
@@ -411,3 +411,17 @@ struct StructB {
 struct OptionalSetDefaultTest {
   1: optional set<string> with_default = [ "test" ]
 }
+
+service ConflictingNamesTest {
+  /**
+   * Use some names that could conflict with the compiler code as args
+   * to make sure that the compiler handled them correctly.
+   */
+  void testNameConflicts(
+    // 1: string args,   // args is already a reserved keyword in thrift 
compiler
+    // 2: string result, // result will cause problems in compiled netstd code
+    3: string meta,
+    4: string r,
+    5: string err,
+  )
+}