From 9a447cddcb43a2984d104158ef938ca9bb0b8d05 Mon Sep 17 00:00:00 2001
From: Yuxuan 'fishy' Wang <yuxuan.wang@reddit.com>
Date: Wed, 17 Feb 2021 12:58:40 -0800
Subject: [PATCH] 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.
---
 .../cpp/src/thrift/generate/t_go_generator.cc | 21 ++++++++++---------
 test/ThriftTest.thrift                        | 12 +++++++++++
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc b/compiler/cpp/src/thrift/generate/t_go_generator.cc
index d9f9d51c5af..37f99f1fbe7 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;
           }
@@ -2221,11 +2222,11 @@ void t_go_generator::generate_service_client(t_service* tservice) {
       // mistaken it as from the oneway call.
       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;
+      f_types_ << indent() << "if _, _err := p.Client_().Call(ctx, \""
+        << method << "\", &"<< argsName << ", nil); _err != nil {" << endl;
 
       indent_up();
-      f_types_ << indent() << "return err" << endl;
+      f_types_ << indent() << "return _err" << endl;
       indent_down();
       f_types_ << indent() << "}" << endl;
       f_types_ << indent() << "return nil" << 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 ac49aee01c8..06e7f03f8da 100644
--- a/test/ThriftTest.thrift
+++ b/test/ThriftTest.thrift
@@ -315,6 +315,18 @@ service ThriftTest
    * @param i32 secondsToSleep - the number of seconds to sleep
    */
   oneway void testOneway(1:i32 secondsToSleep)
+
+  /**
+   * 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 the netstd compiler to error out
+    3: string meta,
+    4: string r,
+    5: string err,
+  )
 }
 
 service SecondService