From 8326ab10da4d5751d8d1ce25540b6b354ac54bcf Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 27 Sep 2026 18:02:15 +0200 Subject: [PATCH] THRIFT-6391: Import packages that collide with -remote stub locals under an alias Client: go The Go -remote stub's main() declares locals such as client, cmd and host, which shadowed an imported package of the same name, so the stub did not compile. Apache Accumulo (client.thrift) and Apache Pegasus (package cmd) hit it. The stub now gets import identifiers of its own, with those names reserved, so such a package is imported under an alias there; the program's other files are generated as before. An alias is also never one already taken by another import. Co-Authored-By: Claude Opus 5.5 --- .../cpp/src/thrift/generate/t_go_generator.cc | 22 ++++++++++++- lib/go/test/Makefile.am | 15 +++++++-- lib/go/test/RemoteShadowAlias.thrift | 26 ++++++++++++++++ lib/go/test/RemoteShadowClient.thrift | 25 +++++++++++++++ lib/go/test/RemoteShadowHost.thrift | 25 +++++++++++++++ lib/go/test/RemoteShadowTest.thrift | 31 +++++++++++++++++++ 6 files changed, 141 insertions(+), 3 deletions(-) create mode 100644 lib/go/test/RemoteShadowAlias.thrift create mode 100644 lib/go/test/RemoteShadowClient.thrift create mode 100644 lib/go/test/RemoteShadowHost.thrift create mode 100644 lib/go/test/RemoteShadowTest.thrift diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc b/compiler/cpp/src/thrift/generate/t_go_generator.cc index 8bab9ae0650..8059b602a04 100644 --- a/compiler/cpp/src/thrift/generate/t_go_generator.cc +++ b/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 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); diff --git a/lib/go/test/Makefile.am b/lib/go/test/Makefile.am index 00d022f76c0..1a06c2f9323 100644 --- a/lib/go/test/Makefile.am +++ b/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' $(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 \ diff --git a/lib/go/test/RemoteShadowAlias.thrift b/lib/go/test/RemoteShadowAlias.thrift new file mode 100644 index 00000000000..d2c001fa0f4 --- /dev/null +++ b/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 +} diff --git a/lib/go/test/RemoteShadowClient.thrift b/lib/go/test/RemoteShadowClient.thrift new file mode 100644 index 00000000000..e185bb70b18 --- /dev/null +++ b/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 +} diff --git a/lib/go/test/RemoteShadowHost.thrift b/lib/go/test/RemoteShadowHost.thrift new file mode 100644 index 00000000000..5146ef0d31b --- /dev/null +++ b/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 +} diff --git a/lib/go/test/RemoteShadowTest.thrift b/lib/go/test/RemoteShadowTest.thrift new file mode 100644 index 00000000000..63b179d818d --- /dev/null +++ b/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) +}