From 4762b4e15f071f06a5d2511128f93f7cbd95c2b1 Mon Sep 17 00:00:00 2001 From: "Ethan J. Gallant" Date: Tue, 28 Nov 2023 09:33:09 -0400 Subject: [PATCH] THRIFT-3037: Support Struct Typedefs in Go Codegen Client: Go --- .../cpp/src/thrift/generate/t_go_generator.cc | 33 +++++----- .../cpp/src/thrift/generate/t_go_generator.h | 8 ++- lib/go/test/Makefile.am | 10 ++- lib/go/test/TypedefServiceTest.thrift | 63 +++++++++++++++++++ lib/go/test/tests/typedef_service_test.go | 31 +++++++++ 5 files changed, 123 insertions(+), 22 deletions(-) create mode 100644 lib/go/test/TypedefServiceTest.thrift create mode 100644 lib/go/test/tests/typedef_service_test.go diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc b/compiler/cpp/src/thrift/generate/t_go_generator.cc index 54422c82670..5fc29b72efd 100644 --- a/compiler/cpp/src/thrift/generate/t_go_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_go_generator.cc @@ -760,13 +760,14 @@ void t_go_generator::close_generator() { void t_go_generator::generate_typedef(t_typedef* ttypedef) { generate_go_docstring(f_types_, ttypedef); string new_type_name(publicize(ttypedef->get_symbolic())); - string base_type(type_to_go_type(ttypedef->get_type())); + string base_type(type_to_go_type_with_opt(ttypedef->get_type(), false, true)); if (base_type == new_type_name) { return; } - f_types_ << "type " << new_type_name << " " << base_type << endl << endl; + f_types_ << "type " << new_type_name << " = " << base_type << endl << endl; + // Generate a convenience function that converts an instance of a type // (which may be a constant) into a pointer to an instance of a type. f_types_ << "func " << new_type_name << "Ptr(v " << new_type_name << ") *" << new_type_name @@ -1175,8 +1176,9 @@ void t_go_generator::get_publicized_name_and_def_value(t_field* tfield, void t_go_generator::generate_go_struct_initializer(ostream& out, t_struct* tstruct, - bool is_args_or_result) { - out << publicize(type_name(tstruct), is_args_or_result) << "{"; + bool is_args_or_result, + string alias_name) { + out << publicize((alias_name != "") ? alias_name : type_name(tstruct), is_args_or_result) << "{"; const vector& members = tstruct->get_members(); for (auto member : members) { bool pointer_field = is_pointer_field(member); @@ -3049,7 +3051,8 @@ void t_go_generator::generate_deserialize_field(ostream& out, (t_struct*)type, is_pointer_field(tfield, in_container_value), declare, - name); + name, + (orig_type->is_typedef()) ? orig_type->get_name() : ""); } else if (type->is_container()) { generate_deserialize_container(out, orig_type, is_pointer_field(tfield), declare, name); } else if (type->is_base_type() || type->is_enum()) { @@ -3150,11 +3153,12 @@ void t_go_generator::generate_deserialize_struct(ostream& out, t_struct* tstruct, bool pointer_field, bool declare, - string prefix) { + string prefix, + string alias_name) { string eq(declare ? " := " : " = "); out << indent() << prefix << eq << (pointer_field ? "&" : ""); - generate_go_struct_initializer(out, tstruct); + generate_go_struct_initializer(out, tstruct, false, alias_name); out << indent() << "if err := " << prefix << "." << read_method_name_ << "(ctx, iprot); err != nil {" << endl; out << indent() << " return thrift.PrependError(fmt.Sprintf(\"%T error reading struct: \", " << prefix << "), err)" << endl; @@ -3914,20 +3918,19 @@ string t_go_generator::type_to_go_key_type(t_type* type) { * Converts the parse type to a go type */ string t_go_generator::type_to_go_type(t_type* type) { - return type_to_go_type_with_opt(type, false); + return type_to_go_type_with_opt(type, false, false); } /** * Converts the parse type to a go type, taking into account whether the field * associated with the type is T_OPTIONAL. */ -string t_go_generator::type_to_go_type_with_opt(t_type* type, - bool optional_field) { +string t_go_generator::type_to_go_type_with_opt(t_type* ttype, + bool optional_field, + bool no_struct_ptr) { string maybe_pointer(optional_field ? "*" : ""); - if (type->is_typedef() && ((t_typedef*)type)->is_forward_typedef()) { - type = ((t_typedef*)type)->get_true_type(); - } + t_type* type = get_true_type(ttype); if (type->is_base_type()) { t_base_type::t_base tbase = ((t_base_type*)type)->get_base(); @@ -3970,7 +3973,7 @@ string t_go_generator::type_to_go_type_with_opt(t_type* type, } else if (type->is_enum()) { return maybe_pointer + publicize(type_name(type)); } else if (type->is_struct() || type->is_xception()) { - return "*" + publicize(type_name(type)); + return (no_struct_ptr ? "" : "*") + publicize(type_name(ttype)); } else if (type->is_map()) { t_map* t = (t_map*)type; string keyType = type_to_go_key_type(t->get_key_type()); @@ -3984,8 +3987,6 @@ string t_go_generator::type_to_go_type_with_opt(t_type* type, t_list* t = (t_list*)type; string elemType = type_to_go_type(t->get_elem_type()); return maybe_pointer + string("[]") + elemType; - } else if (type->is_typedef()) { - return maybe_pointer + publicize(type_name(type)); } throw "INVALID TYPE IN type_to_go_type: " + type->get_name(); diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.h b/compiler/cpp/src/thrift/generate/t_go_generator.h index a67485c5506..d3cc46eb8ae 100644 --- a/compiler/cpp/src/thrift/generate/t_go_generator.h +++ b/compiler/cpp/src/thrift/generate/t_go_generator.h @@ -124,7 +124,8 @@ class t_go_generator : public t_generator { bool is_args = false); void generate_go_struct_initializer(std::ostream& out, t_struct* tstruct, - bool is_args_or_result = false); + bool is_args_or_result = false, + string alias_name = ""); void generate_isset_helpers(std::ostream& out, t_struct* tstruct, const string& tstruct_name, @@ -176,7 +177,8 @@ class t_go_generator : public t_generator { t_struct* tstruct, bool is_pointer_field, bool declare, - std::string prefix = ""); + std::string prefix = "", + string alias_name = ""); void generate_deserialize_container(std::ostream& out, t_type* ttype, @@ -264,7 +266,7 @@ class t_go_generator : public t_generator { std::string argument_list(t_struct* tstruct); std::string type_to_enum(t_type* ttype); std::string type_to_go_type(t_type* ttype); - std::string type_to_go_type_with_opt(t_type* ttype, bool optional_field); + std::string type_to_go_type_with_opt(t_type* ttype, bool optional_field = false, bool no_struct_ptr = false); std::string type_to_go_key_type(t_type* ttype); std::string type_to_spec_args(t_type* ttype); diff --git a/lib/go/test/Makefile.am b/lib/go/test/Makefile.am index cb8928bc8c3..62ec952d99c 100644 --- a/lib/go/test/Makefile.am +++ b/lib/go/test/Makefile.am @@ -62,7 +62,8 @@ gopath: $(THRIFT) $(THRIFTTEST) \ ProcessorMiddlewareTest.thrift \ ClientMiddlewareExceptionTest.thrift \ ValidateTest.thrift \ - ForwardType.thrift + ForwardType.thrift \ + TypedefServiceTest.thrift mkdir -p gopath/src grep -v list.*map.*list.*map $(THRIFTTEST) | grep -v 'set' > ThriftTest.thrift $(THRIFT) $(THRIFTARGS) -r IncludesTest.thrift @@ -98,6 +99,7 @@ gopath: $(THRIFT) $(THRIFTTEST) \ $(THRIFT) $(THRIFTARGS) ClientMiddlewareExceptionTest.thrift $(THRIFT) $(THRIFTARGS) ValidateTest.thrift $(THRIFT) $(THRIFTARGS) ForwardType.thrift + $(THRIFT) $(THRIFTARGS) TypedefServiceTest.thrift ln -nfs ../../tests gopath/src/tests cp -r ./dontexportrwtest gopath/src touch gopath @@ -124,7 +126,8 @@ check: gopath ./gopath/src/processormiddlewaretest \ ./gopath/src/clientmiddlewareexceptiontest \ ./gopath/src/validatetest \ - ./gopath/src/forwardtypetest + ./gopath/src/forwardtypetest \ + ./gopath/src/typedefservicetest $(GO) test github.com/apache/thrift/lib/go/thrift $(GO) test ./gopath/src/tests ./gopath/src/dontexportrwtest @@ -172,4 +175,5 @@ EXTRA_DIST = \ TypedefFieldTest.thrift \ UnionBinaryTest.thrift \ UnionDefaultValueTest.thrift \ - ValidateTest.thrift + ValidateTest.thrift \ + TypedefServiceTest.thrift diff --git a/lib/go/test/TypedefServiceTest.thrift b/lib/go/test/TypedefServiceTest.thrift new file mode 100644 index 00000000000..f6294bfed5e --- /dev/null +++ b/lib/go/test/TypedefServiceTest.thrift @@ -0,0 +1,63 @@ +# +# 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. +# + +# We are only testing that generated code compiles, no correctness checking is done + +struct ExampleRequest { + 1: required string example_field +} + +struct ExampleResponse { + 1: required string example_field +} + +struct ExampleOptionalRequest { + 1: optional string example_field +} + +struct ExampleOptionalResponse { + 1: optional string example_field +} + +struct ExampleNested { + 1: required string example_field +} + +struct ExampleOptionalNestedRequest { + 1: optional ExampleNested example_field +} + +struct ExampleOptionalNestedResponse { + 1: optional ExampleNested example_field +} + +typedef ExampleRequest TypedefExampleRequest +typedef ExampleResponse TypedefExampleResponse + +typedef string PrimativeTypedefExampleRequest +typedef string PrimativeTypedefExampleResponse + +service TypeDefService { + ExampleResponse exampleMethod(1: ExampleRequest request) + TypedefExampleResponse typedefExampleMethod(1: TypedefExampleRequest request) + string primativeExampleMethod(1: string request) + PrimativeTypedefExampleResponse primativeTypedefExampleMethod(1: PrimativeTypedefExampleRequest request) + ExampleOptionalResponse exampleOptionalMethod(1: ExampleOptionalRequest request) + ExampleOptionalNestedResponse exampleOptionalNestedMethod(1: ExampleOptionalNestedRequest request) +} diff --git a/lib/go/test/tests/typedef_service_test.go b/lib/go/test/tests/typedef_service_test.go new file mode 100644 index 00000000000..c29c43a0c25 --- /dev/null +++ b/lib/go/test/tests/typedef_service_test.go @@ -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. + */ + +package tests + +import ( + "testing" + + "github.com/apache/thrift/lib/go/test/gopath/src/typedefservicetest" +) + +func TestTypedefService(t *testing.T) { + // We need to import the generated code to ensure it compiles + _ = &typedefservicetest.TypedefExampleRequest{} +}