commit a2522c0c8329a76bab42af251f06bd348ad0ecbd
author Yuxuan 'fishy' Wang <yuxuan.wang@reddit.com> Wed Feb 17 12:58:40 2021 -0800
committer Yuxuan 'fishy' Wang <fishywang@gmail.com> Wed Feb 17 18:50:47 2021 -0800
tree e8d2f92beabf596f40fd0a053c3b3c22ecc91a6a
parent 36bd59ff8260b2d5bd5825d3361121423022948e

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

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 @@
     }
 
     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 @@
           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 @@
       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 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 @@
   }
 
   if (addError) {
-    signature += "err error";
+    signature += "_err error";
   }
 
   signature += ")";

test/ThriftTest.thrift

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 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,
+  )
+}