From 0f06ab30179b1a70248e5d56a70e7f41d037695f Mon Sep 17 00:00:00 2001 From: Konstantin Shaposhnikov Date: Wed, 20 May 2015 22:26:26 +0800 Subject: [PATCH] THRIFT-3150 go: add option to generate non-exported Read and Write methods This change adds a new option to Go trift compiler: dont_export_rw. When it is specified generated Read and Write methods will be called 'read' and 'write' (and hence are non-exported). This can be useful if a package author wants to make the generated structs part of public package API but doesn't want the package depend on thrift library. This will make vendoring thrift library easier too. --- compiler/cpp/src/generate/t_go_generator.cc | 44 +++++++++++++------- lib/go/test/DontExportRWTest.thrift | 27 ++++++++++++ lib/go/test/Makefile.am | 10 +++-- lib/go/test/dontexportrwtest/compile_test.go | 38 +++++++++++++++++ 4 files changed, 102 insertions(+), 17 deletions(-) create mode 100644 lib/go/test/DontExportRWTest.thrift create mode 100644 lib/go/test/dontexportrwtest/compile_test.go diff --git a/compiler/cpp/src/generate/t_go_generator.cc b/compiler/cpp/src/generate/t_go_generator.cc index 0030d24c4d4..b7b9ce0fe96 100644 --- a/compiler/cpp/src/generate/t_go_generator.cc +++ b/compiler/cpp/src/generate/t_go_generator.cc @@ -94,6 +94,9 @@ class t_go_generator : public t_generator { if (iter != parsed_options.end()) { package_flag = (iter->second); } + + iter = parsed_options.find("dont_export_rw"); + dont_export_rw_ = (iter != parsed_options.end()); } /** @@ -278,6 +281,7 @@ class t_go_generator : public t_generator { private: std::string gen_package_prefix_; std::string gen_thrift_import_; + bool dont_export_rw_; /** * File streams @@ -292,6 +296,8 @@ class t_go_generator : public t_generator { std::string package_name_; std::string package_dir_; + std::string read_method_name_; + std::string write_method_name_; std::set commonInitialisms; @@ -677,6 +683,15 @@ void t_go_generator::init_generator() { commonInitialisms.insert("XSRF"); commonInitialisms.insert("XSS"); + // names of read and write methods + if (dont_export_rw_) { + read_method_name_ = "read"; + write_method_name_ = "write"; + } else { + read_method_name_ = "Read"; + write_method_name_ = "Write"; + } + while (true) { // TODO: Do better error checking here. MKDIR(package_dir_.c_str()); @@ -1358,7 +1373,7 @@ void t_go_generator::generate_go_struct_reader(ofstream& out, const vector& fields = tstruct->get_members(); vector::const_iterator f_iter; string escaped_tstruct_name(escape_string(tstruct->get_name())); - out << indent() << "func (p *" << tstruct_name << ") Read(iprot thrift.TProtocol) error {" + out << indent() << "func (p *" << tstruct_name << ") " << read_method_name_ << "(iprot thrift.TProtocol) error {" << endl; indent_up(); out << indent() << "if _, err := iprot.ReadStructBegin(); err != nil {" << endl; @@ -1504,7 +1519,7 @@ void t_go_generator::generate_go_struct_writer(ofstream& out, string name(tstruct->get_name()); const vector& fields = tstruct->get_sorted_members(); vector::const_iterator f_iter; - indent(out) << "func (p *" << tstruct_name << ") Write(oprot thrift.TProtocol) error {" << endl; + indent(out) << "func (p *" << tstruct_name << ") " << write_method_name_ << "(oprot thrift.TProtocol) error {" << endl; indent_up(); if (tstruct->is_union() && uses_countsetfields) { std::string tstruct_name(publicize(tstruct->get_name())); @@ -1881,7 +1896,7 @@ void t_go_generator::generate_service_client(t_service* tservice) { f_service_ << indent() << "}" << endl; // Write to the stream - f_service_ << indent() << "if err = args.Write(oprot); err != nil {" << endl; + f_service_ << indent() << "if err = args." << write_method_name_ << "(oprot); err != nil {" << endl; indent_up(); f_service_ << indent() << " return" << endl; indent_down(); @@ -1936,7 +1951,7 @@ void t_go_generator::generate_service_client(t_service* tservice) { << " := thrift.NewTApplicationException(thrift.UNKNOWN_APPLICATION_EXCEPTION, " "\"Unknown Exception\")" << endl; f_service_ << indent() << " var " << error2 << " error" << endl; - f_service_ << indent() << " " << error2 << ", err = " << error << ".Read(iprot)" << endl; + f_service_ << indent() << " " << error2 << ", err = " << error << "." << read_method_name_ << "(iprot)" << endl; f_service_ << indent() << " if err != nil {" << endl; f_service_ << indent() << " return" << endl; f_service_ << indent() << " }" << endl; @@ -1953,7 +1968,7 @@ void t_go_generator::generate_service_client(t_service* tservice) { f_service_ << indent() << " return" << endl; f_service_ << indent() << "}" << endl; f_service_ << indent() << "result := " << resultname << "{}" << endl; - f_service_ << indent() << "if err = result.Read(iprot); err != nil {" << endl; + f_service_ << indent() << "if err = result." << read_method_name_ << "(iprot); err != nil {" << endl; f_service_ << indent() << " return" << endl; f_service_ << indent() << "}" << endl; f_service_ << indent() << "if err = iprot.ReadMessageEnd(); err != nil {" << endl; @@ -2305,7 +2320,7 @@ void t_go_generator::generate_service_remote(t_service* tservice) { << endl; f_remote << indent() << "argvalue" << i << " := " << package_name_ << ".New" << tstruct_name << "()" << endl; - f_remote << indent() << err2 << " := argvalue" << i << ".Read(" << jsProt << ")" << endl; + f_remote << indent() << err2 << " := argvalue" << i << "." << read_method_name_ << "(" << jsProt << ")" << endl; f_remote << indent() << "if " << err2 << " != nil {" << endl; f_remote << indent() << " Usage()" << endl; f_remote << indent() << " return" << endl; @@ -2505,7 +2520,7 @@ void t_go_generator::generate_service_server(t_service* tservice) { << " := thrift.NewTApplicationException(thrift.UNKNOWN_METHOD, \"Unknown function " "\" + name)" << endl; f_service_ << indent() << " oprot.WriteMessageBegin(name, thrift.EXCEPTION, seqId)" << endl; - f_service_ << indent() << " " << x << ".Write(oprot)" << endl; + f_service_ << indent() << " " << x << "." << write_method_name_ << "(oprot)" << endl; f_service_ << indent() << " oprot.WriteMessageEnd()" << endl; f_service_ << indent() << " oprot.Flush()" << endl; f_service_ << indent() << " return false, " << x << endl; @@ -2561,7 +2576,7 @@ void t_go_generator::generate_process_function(t_service* tservice, t_function* "thrift.TException) {" << endl; indent_up(); f_service_ << indent() << "args := " << argsname << "{}" << endl; - f_service_ << indent() << "if err = args.Read(iprot); err != nil {" << endl; + f_service_ << indent() << "if err = args." << read_method_name_ << "(iprot); err != nil {" << endl; f_service_ << indent() << " iprot.ReadMessageEnd()" << endl; if (!tfunction->is_oneway()) { f_service_ << indent() @@ -2569,7 +2584,7 @@ void t_go_generator::generate_process_function(t_service* tservice, t_function* << endl; f_service_ << indent() << " oprot.WriteMessageBegin(\"" << escape_string(tfunction->get_name()) << "\", thrift.EXCEPTION, seqId)" << endl; - f_service_ << indent() << " x.Write(oprot)" << endl; + f_service_ << indent() << " x." << write_method_name_ << "(oprot)" << endl; f_service_ << indent() << " oprot.WriteMessageEnd()" << endl; f_service_ << indent() << " oprot.Flush()" << endl; } @@ -2635,7 +2650,7 @@ void t_go_generator::generate_process_function(t_service* tservice, t_function* << ": \" + err2.Error())" << endl; f_service_ << indent() << " oprot.WriteMessageBegin(\"" << escape_string(tfunction->get_name()) << "\", thrift.EXCEPTION, seqId)" << endl; - f_service_ << indent() << " x.Write(oprot)" << endl; + f_service_ << indent() << " x." << write_method_name_ << "(oprot)" << endl; f_service_ << indent() << " oprot.WriteMessageEnd()" << endl; f_service_ << indent() << " oprot.Flush()" << endl; } @@ -2667,7 +2682,7 @@ void t_go_generator::generate_process_function(t_service* tservice, t_function* << endl; f_service_ << indent() << " err = err2" << endl; f_service_ << indent() << "}" << endl; - f_service_ << indent() << "if err2 = result.Write(oprot); err == nil && err2 != nil {" << endl; + f_service_ << indent() << "if err2 = result." << write_method_name_ << "(oprot); err == nil && err2 != nil {" << endl; f_service_ << indent() << " err = err2" << endl; f_service_ << indent() << "}" << endl; f_service_ << indent() << "if err2 = oprot.WriteMessageEnd(); err == nil && err2 != nil {" @@ -2820,7 +2835,7 @@ void t_go_generator::generate_deserialize_struct(ofstream& out, out << indent() << prefix << eq << (pointer_field ? "&" : ""); generate_go_struct_initializer(out, tstruct); - out << indent() << "if err := " << prefix << ".Read(iprot); err != nil {" << endl; + out << indent() << "if err := " << prefix << "." << read_method_name_ << "(iprot); err != nil {" << endl; out << indent() << " return thrift.PrependError(fmt.Sprintf(\"%T error reading struct: \", " << prefix << "), err)" << endl; out << indent() << "}" << endl; @@ -3049,7 +3064,7 @@ void t_go_generator::generate_serialize_field(ofstream& out, */ void t_go_generator::generate_serialize_struct(ofstream& out, t_struct* tstruct, string prefix) { (void)tstruct; - out << indent() << "if err := " << prefix << ".Write(oprot); err != nil {" << endl; + out << indent() << "if err := " << prefix << "." << write_method_name_ << "(oprot); err != nil {" << endl; out << indent() << " return thrift.PrependError(fmt.Sprintf(\"%T error writing struct: \", " << prefix << "), err)" << endl; out << indent() << "}" << endl; @@ -3535,4 +3550,5 @@ bool format_go_output(const string& file_path) { THRIFT_REGISTER_GENERATOR(go, "Go", " package_prefix= Package prefix for generated files.\n" \ " thrift_import= Override thrift package import path (default:" + default_thrift_import + ")\n" \ - " package= Package name (default: inferred from thrift file name)\n") + " package= Package name (default: inferred from thrift file name)\n" \ + " dont_export_rw= Do not export Read & Write methods (i.e. name them read & write)\n") diff --git a/lib/go/test/DontExportRWTest.thrift b/lib/go/test/DontExportRWTest.thrift new file mode 100644 index 00000000000..39e5a6fc85f --- /dev/null +++ b/lib/go/test/DontExportRWTest.thrift @@ -0,0 +1,27 @@ +# +# 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. +# + +struct InnerStruct { + 1: required string id +} + +struct TestStruct { + 1: required string id + 2: required InnerStruct inner +} diff --git a/lib/go/test/Makefile.am b/lib/go/test/Makefile.am index aa3d95bdb8b..d6277f7fa15 100644 --- a/lib/go/test/Makefile.am +++ b/lib/go/test/Makefile.am @@ -34,7 +34,8 @@ gopath: $(THRIFT) $(THRIFTTEST) \ RefAnnotationFieldsTest.thrift \ ErrorTest.thrift \ NamesTest.thrift \ - InitialismsTest.thrift + InitialismsTest.thrift \ + DontExportRWTest.thrift dontexportrwtest/compile_test.go mkdir -p gopath/src grep -v list.*map.*list.*map $(THRIFTTEST) | grep -v 'set' > ThriftTest.thrift $(THRIFT) $(THRIFTARGS) -r IncludesTest.thrift @@ -49,9 +50,11 @@ gopath: $(THRIFT) $(THRIFTTEST) \ $(THRIFT) $(THRIFTARGS) ErrorTest.thrift $(THRIFT) $(THRIFTARGS) NamesTest.thrift $(THRIFT) $(THRIFTARGS) InitialismsTest.thrift + $(THRIFT) $(THRIFTARGS),dont_export_rw DontExportRWTest.thrift GOPATH=`pwd`/gopath $(GO) get code.google.com/p/gomock/gomock ln -nfs ../../../thrift gopath/src/thrift ln -nfs ../../tests gopath/src/tests + cp -r ./dontexportrwtest gopath/src touch gopath check: gopath @@ -63,8 +66,9 @@ check: gopath refannotationfieldstest \ errortest \ namestest \ - initialismstest - GOPATH=`pwd`/gopath $(GO) test thrift tests + initialismstest \ + dontexportrwtest + GOPATH=`pwd`/gopath $(GO) test thrift tests dontexportrwtest clean-local: $(RM) -r gopath ThriftTest.thrift gen-go diff --git a/lib/go/test/dontexportrwtest/compile_test.go b/lib/go/test/dontexportrwtest/compile_test.go new file mode 100644 index 00000000000..deff68228e9 --- /dev/null +++ b/lib/go/test/dontexportrwtest/compile_test.go @@ -0,0 +1,38 @@ +/* + * 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 dontexportrwtest + +import ( + "fmt" + "testing" +) + +// Make sure that thrift generates non-exported read/write methods if +// dont_export_rw option is specified +func TestReadWriteMethodsArePrivate(t *testing.T) { + // This will only compile if read/write methods exist + s := NewTestStruct() + fmt.Sprintf("%v", s.read) + fmt.Sprintf("%v", s.write) + + is := NewInnerStruct() + fmt.Sprintf("%v", is.read) + fmt.Sprintf("%v", is.write) +}