THRIFT-6391: Import packages that collide with -remote stub locals under an alias

compiler/cpp/src/thrift/generate/t_go_generator.cc

@@ -600,7 +600,10 @@ string t_go_generator::render_program_import(const t_program* program, string& u
       // path components to generate a more readable name unique identifier (e.g. use
       // packageacommon as the alias for packagea/common instead of common=). But just
       // appending an integer is much simpler code
-      value = tmp(value);
+      string base = value;
+      do {
+        value = tmp(base);
+      } while (package_identifiers_set_.find(value) != package_identifiers_set_.end());
     }
     package_identifiers_set_.insert(value);
     it = package_identifiers_.emplace(go_module, std::move(value)).first;
@@ -2652,6 +2655,20 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
 
   string unused_protection;
 
+  // The stub is its own package main, so it gets its own import identifiers: those of the
+  // program's other files are restored once the stub is written. main() below declares these
+  // local variables; an imported package with one of these names would be shadowed by them, so
+  // the stub imports such a package under an alias. Keep this list in sync with main().
+  auto saved_package_identifiers = std::move(package_identifiers_);
+  auto saved_package_identifiers_set = std::move(package_identifiers_set_);
+  package_identifiers_.clear();
+  package_identifiers_set_ = {"cfg",      "client",    "cmd",       "err",
+                              "framed",   "headers",   "host",      "httptrans",
+                              "iprot",    "m",         "oprot",     "parsedUrl",
+                              "parts",    "port",      "portStr",   "protocol",
+                              "protocolFactory",       "trans",     "urlString",
+                              "useHttp"};
+
   std::vector<string> system_packages;
   system_packages.push_back("context");
   system_packages.push_back("flag");
@@ -3200,6 +3217,9 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
   f_remote << indent() << "}" << '\n';
   indent_down();
   f_remote << indent() << "}" << '\n';
+  package_identifiers_ = std::move(saved_package_identifiers);
+  package_identifiers_set_ = std::move(saved_package_identifiers_set);
+
   // Close service file
   f_remote.close();
   format_go_output(f_remote_name);

lib/go/test/Makefile.am

@@ -72,7 +72,11 @@ gopath: $(THRIFT) $(THRIFTTEST) \
 				ValidateTest.thrift \
 				ForwardType.thrift \
 				StringParseAllocationTest.thrift \
-				DocCommentTest.thrift
+				DocCommentTest.thrift \
+				RemoteShadowAlias.thrift \
+				RemoteShadowClient.thrift \
+				RemoteShadowHost.thrift \
+				RemoteShadowTest.thrift
 	mkdir -p gopath/src
 	grep -v 'set<Insanity>' $(THRIFTTEST) > ThriftTest.thrift
 	$(THRIFT) $(THRIFTARGS) $(RECURSIVE)
@@ -115,6 +119,7 @@ gopath: $(THRIFT) $(THRIFTTEST) \
 	$(THRIFT) $(THRIFTARGS) ForwardType.thrift
 	$(THRIFT) $(THRIFTARGS) StringParseAllocationTest.thrift
 	$(THRIFT) $(THRIFTARGS) DocCommentTest.thrift
+	$(THRIFT) $(THRIFTARGS) -r RemoteShadowTest.thrift
 	ln -nfs ../../tests gopath/src/tests
 	cp -r ./dontexportrwtest gopath/src
 	touch gopath
@@ -149,7 +154,9 @@ check: gopath
 				./gopath/src/processormiddlewaretest \
 				./gopath/src/clientmiddlewareexceptiontest \
 				./gopath/src/validatetest \
-				./gopath/src/forwardtypetest
+				./gopath/src/forwardtypetest \
+				./gopath/src/remoteshadowtest \
+				./gopath/src/remoteshadowtest/shadow_service-remote
 	$(GO) test github.com/apache/thrift/lib/go/thrift
 	$(GO) test ./gopath/src/tests ./gopath/src/dontexportrwtest
 
@@ -200,6 +207,10 @@ EXTRA_DIST = \
 	OptionalFieldsTest.thrift \
 	ProcessorMiddlewareTest.thrift \
 	RefAnnotationFieldsTest.thrift \
+	RemoteShadowAlias.thrift \
+	RemoteShadowClient.thrift \
+	RemoteShadowHost.thrift \
+	RemoteShadowTest.thrift \
 	RequiredFieldTest.thrift \
 	ServicesTest.thrift \
 	StringParseAllocationTest.thrift \

lib/go/test/RemoteShadowAlias.thrift

@@ -0,0 +1,26 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+// Its Go package is named like the alias the generator would pick for the
+// client package, and sorts before it.
+namespace go alias.client0
+
+struct Tag {
+  1: i32 y
+}

lib/go/test/RemoteShadowClient.thrift

@@ -0,0 +1,25 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+// Its Go package is named like a local variable of the generated -remote stub.
+namespace go client
+
+struct Info {
+  1: i32 x
+}

lib/go/test/RemoteShadowHost.thrift

@@ -0,0 +1,25 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+// Its Go package is named like a local variable of the generated -remote stub.
+namespace go host
+
+struct Addr {
+  1: string name
+}

lib/go/test/RemoteShadowTest.thrift

@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+// The -remote stub declares local variables such as client and host; packages
+// with those names, used by the arguments below, must not be shadowed by them,
+// and the alias given to such a package must not be a name already in use.
+namespace go remoteshadowtest
+
+include "RemoteShadowAlias.thrift"
+include "RemoteShadowClient.thrift"
+include "RemoteShadowHost.thrift"
+
+service ShadowService {
+  void f(1: RemoteShadowClient.Info info, 2: RemoteShadowHost.Addr addr, 3: RemoteShadowAlias.Tag tag)
+}