THRIFT-2689 struct names that differ only in capitalization of first character generate broken erlang code Client: Erlang Patch: Alisdair Sullivan
This closes #204 Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/cc4ac495 Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/cc4ac495 Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/cc4ac495 Branch: refs/heads/master Commit: cc4ac4955c968a6171aec0f65b14e16d141f81ce Parents: 5aff35f Author: Jens Geyer <[email protected]> Authored: Wed Oct 1 21:13:39 2014 +0200 Committer: Jens Geyer <[email protected]> Committed: Wed Oct 1 21:13:39 2014 +0200 ---------------------------------------------------------------------- compiler/cpp/src/generate/t_erl_generator.cc | 20 ++--- test/NameConflictTest.thrift | 9 +++ test/erl/src/nameConflictTest_test.erl | 98 ++++++++++++++--------- 3 files changed, 81 insertions(+), 46 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/cc4ac495/compiler/cpp/src/generate/t_erl_generator.cc ---------------------------------------------------------------------- diff --git a/compiler/cpp/src/generate/t_erl_generator.cc b/compiler/cpp/src/generate/t_erl_generator.cc index 9228325..2876788 100644 --- a/compiler/cpp/src/generate/t_erl_generator.cc +++ b/compiler/cpp/src/generate/t_erl_generator.cc @@ -128,7 +128,7 @@ class t_erl_generator : public t_generator { return in; } - std::string uncapitalize(std::string in) { + std::string make_safe_for_module_name(std::string in) { in[0] = tolower(in[0]); return in; } @@ -249,7 +249,7 @@ string t_erl_generator::render_includes() { const vector<t_program*>& includes = program_->get_includes(); string result = ""; for (size_t i = 0; i < includes.size(); ++i) { - result += "-include(\"" + uncapitalize(includes[i]->get_name()) + "_types.hrl\").\n"; + result += "-include(\"" + make_safe_for_module_name(includes[i]->get_name()) + "_types.hrl\").\n"; } if (includes.size() > 0) { result += "\n"; @@ -391,7 +391,7 @@ string t_erl_generator::render_const_value(t_type* type, t_const_value* value) { indent(out) << value->get_integer(); } else if (type->is_struct() || type->is_xception()) { - out << "#'" << uncapitalize(type->get_name()) << "'{"; + out << "#'" << type->get_name() << "'{"; const vector<t_field*>& fields = ((t_struct*)type)->get_members(); vector<t_field*>::const_iterator f_iter; const map<t_const_value*, t_const_value*>& val = value->get_map(); @@ -474,7 +474,7 @@ string t_erl_generator::render_const_value(t_type* type, t_const_value* value) { string t_erl_generator::render_default_value(t_field* field) { t_type *type = field->get_type(); if (type->is_struct() || type->is_xception()) { - return "#'" + uncapitalize(type->get_name()) + "'{}"; + return "#'" + type->get_name() + "'{}"; } else if (type->is_map()) { return "dict:new()"; } else if (type->is_set()) { @@ -508,7 +508,7 @@ string t_erl_generator::render_member_type(t_field * field) { } else if (type->is_enum()) { return "integer()"; } else if (type->is_struct() || type->is_xception()) { - return "'" + uncapitalize(type->get_name()) + "'()"; + return "'" + type->get_name() + "'()"; } else if (type->is_map()) { return "dict()"; } else if (type->is_set()) { @@ -666,7 +666,7 @@ void t_erl_generator::generate_service(t_service* tservice) { if (tservice->get_extends() != NULL) { f_service_hrl_ << "-include(\"" << - uncapitalize(tservice->get_extends()->get_name()) << "_thrift.hrl\"). % inherit " << endl; + make_safe_for_module_name(tservice->get_extends()->get_name()) << "_thrift.hrl\"). % inherit " << endl; } f_service_hrl_ << @@ -686,7 +686,7 @@ void t_erl_generator::generate_service(t_service* tservice) { "-behaviour(thrift_service)." << endl << endl << erl_imports() << endl; - f_service_file_ << "-include(\"" << uncapitalize(tservice->get_name()) << "_thrift.hrl\")." << endl << endl; + f_service_file_ << "-include(\"" << make_safe_for_module_name(tservice->get_name()) << "_thrift.hrl\")." << endl << endl; f_service_file_ << "-export([" << export_lines_.str() << "])." << endl << endl; @@ -751,7 +751,7 @@ void t_erl_generator::generate_service_interface(t_service* tservice) { if (tservice->get_extends() != NULL) { indent(f_service_) << "function_info(Function, InfoType) ->" << endl; indent_up(); - indent(f_service_) << uncapitalize(tservice->get_extends()->get_name()) + indent(f_service_) << make_safe_for_module_name(tservice->get_extends()->get_name()) << "_thrift:function_info(Function, InfoType)." << endl; indent_down(); } else { @@ -888,7 +888,7 @@ string t_erl_generator::type_name(t_type* ttype) { string name = ttype->get_name(); if (ttype->is_struct() || ttype->is_xception() || ttype->is_service()) { - name = "'" + uncapitalize(ttype->get_name()) + "'"; + name = "'" + ttype->get_name() + "'"; } return prefix + name; @@ -1023,7 +1023,7 @@ std::string t_erl_generator::render_type_term(t_type* type, bool expand_structs, } std::string t_erl_generator::type_module(t_type* ttype) { - return uncapitalize(ttype->get_program()->get_name()) + "_types"; + return make_safe_for_module_name(ttype->get_program()->get_name()) + "_types"; } THRIFT_REGISTER_GENERATOR(erl, "Erlang", "") http://git-wip-us.apache.org/repos/asf/thrift/blob/cc4ac495/test/NameConflictTest.thrift ---------------------------------------------------------------------- diff --git a/test/NameConflictTest.thrift b/test/NameConflictTest.thrift index 4960f54..24757c0 100644 --- a/test/NameConflictTest.thrift +++ b/test/NameConflictTest.thrift @@ -76,6 +76,15 @@ struct TheEdgeCase { 6: bool TheEdgeCase__ } +struct theEdgeCase { + 1: bool theEdgeCase + 2: bool theEdgeCase_ + 3: bool theEdgeCase__ + 4: bool TheEdgeCase + 5: bool TheEdgeCase_ + 6: bool TheEdgeCase__ +} + struct Tricky_ { 1: bool tricky 2: bool Tricky http://git-wip-us.apache.org/repos/asf/thrift/blob/cc4ac495/test/erl/src/nameConflictTest_test.erl ---------------------------------------------------------------------- diff --git a/test/erl/src/nameConflictTest_test.erl b/test/erl/src/nameConflictTest_test.erl index 48a08be..7ce4886 100644 --- a/test/erl/src/nameConflictTest_test.erl +++ b/test/erl/src/nameConflictTest_test.erl @@ -43,8 +43,8 @@ record_generation_test_() -> #partial{using=null} )}, {"ClassAndProp record", ?_assertMatch( - {classAndProp, _, _, _, _}, - #classAndProp{ + {'ClassAndProp', _, _, _, _}, + #'ClassAndProp'{ 'ClassAndProp'=null, 'ClassAndProp_'=null, 'ClassAndProp__'=null, @@ -61,8 +61,8 @@ record_generation_test_() -> } )}, {"NOW_EAT_THIS record", ?_assertMatch( - {nOW_EAT_THIS, _, _, _, _}, - #nOW_EAT_THIS{ + {'NOW_EAT_THIS', _, _, _, _}, + #'NOW_EAT_THIS'{ now_eat_this=null, now_eat_this_=null, now_eat_this__=null, @@ -70,6 +70,17 @@ record_generation_test_() -> } )}, {"TheEdgeCase record", ?_assertMatch( + {'TheEdgeCase', _, _, _, _, _, _}, + #'TheEdgeCase'{ + theEdgeCase=null, + theEdgeCase_=null, + theEdgeCase__=null, + 'TheEdgeCase'=null, + 'TheEdgeCase_'=null, + 'TheEdgeCase__'=null + } + )}, + {"theEdgeCase record", ?_assertMatch( {theEdgeCase, _, _, _, _, _, _}, #theEdgeCase{ theEdgeCase=null, @@ -81,12 +92,12 @@ record_generation_test_() -> } )}, {"Tricky_ record", ?_assertMatch( - {tricky_, _, _}, - #tricky_{tricky=null,'Tricky'=null} + {'Tricky_', _, _}, + #'Tricky_'{tricky=null,'Tricky'=null} )}, {"Nested record", ?_assertMatch( - {nested, _, _, _, _, _, _}, - #nested{ + {'Nested', _, _, _, _, _, _}, + #'Nested'{ 'ClassAndProp'=null, second_chance=null, 'NOW_EAT_THIS'=null, @@ -96,8 +107,8 @@ record_generation_test_() -> } )}, {"Problem_ record", ?_assertMatch( - {problem_, _, _}, - #problem_{problem=null,'Problem'=null} + {'Problem_', _, _}, + #'Problem_'{problem=null,'Problem'=null} )} ]. @@ -124,7 +135,7 @@ struct_info_test_() -> )}, {"ClassAndProp definition", ?_assertEqual( {struct, [{1, bool},{2, bool},{3, bool},{4, bool}]}, - nameConflictTest_types:struct_info(classAndProp) + nameConflictTest_types:struct_info('ClassAndProp') )}, {"second_chance definition", ?_assertEqual( {struct, [{1, bool},{2, bool},{3, bool},{4, bool}]}, @@ -132,30 +143,34 @@ struct_info_test_() -> )}, {"NOW_EAT_THIS definition", ?_assertEqual( {struct, [{1, bool},{2, bool},{3, bool},{4, bool}]}, - nameConflictTest_types:struct_info(nOW_EAT_THIS) + nameConflictTest_types:struct_info('NOW_EAT_THIS') )}, {"TheEdgeCase definition", ?_assertEqual( {struct, [{1, bool},{2, bool},{3, bool},{4, bool},{5, bool},{6, bool}]}, + nameConflictTest_types:struct_info('TheEdgeCase') + )}, + {"theEdgeCase definition", ?_assertEqual( + {struct, [{1, bool},{2, bool},{3, bool},{4, bool},{5, bool},{6, bool}]}, nameConflictTest_types:struct_info(theEdgeCase) )}, {"Tricky_ definition", ?_assertEqual( {struct, [{1, bool},{2, bool}]}, - nameConflictTest_types:struct_info(tricky_) + nameConflictTest_types:struct_info('Tricky_') )}, {"Nested definition", ?_assertEqual( {struct, [ - {1, {struct, {nameConflictTest_types, classAndProp}}}, + {1, {struct, {nameConflictTest_types, 'ClassAndProp'}}}, {2, {struct, {nameConflictTest_types, second_chance}}}, - {3, {struct, {nameConflictTest_types, nOW_EAT_THIS}}}, - {4, {struct, {nameConflictTest_types, theEdgeCase}}}, - {5, {struct, {nameConflictTest_types, tricky_}}}, - {6, {struct, {nameConflictTest_types, nested}}} + {3, {struct, {nameConflictTest_types, 'NOW_EAT_THIS'}}}, + {4, {struct, {nameConflictTest_types, 'TheEdgeCase'}}}, + {5, {struct, {nameConflictTest_types, 'Tricky_'}}}, + {6, {struct, {nameConflictTest_types, 'Nested'}}} ]}, - nameConflictTest_types:struct_info(nested) + nameConflictTest_types:struct_info('Nested') )}, {"Problem_ definition", ?_assertEqual( {struct, [{1, bool},{2, bool}]}, - nameConflictTest_types:struct_info(problem_) + nameConflictTest_types:struct_info('Problem_') )}, {"using extended definition", ?_assertEqual( {struct, [ @@ -188,7 +203,7 @@ struct_info_test_() -> {3, undefined, bool, 'ClassAndProp__', undefined}, {4, undefined, bool, 'ClassAndProper', undefined} ]}, - nameConflictTest_types:struct_info_ext(classAndProp) + nameConflictTest_types:struct_info_ext('ClassAndProp') )}, {"second_chance extended definition", ?_assertEqual( {struct, [ @@ -206,7 +221,18 @@ struct_info_test_() -> {3, undefined, bool, now_eat_this__, undefined}, {4, undefined, bool, now_eat_this_and_this, undefined} ]}, - nameConflictTest_types:struct_info_ext(nOW_EAT_THIS) + nameConflictTest_types:struct_info_ext('NOW_EAT_THIS') + )}, + {"TheEdgeCase extended definition", ?_assertEqual( + {struct, [ + {1, undefined, bool, theEdgeCase, undefined}, + {2, undefined, bool, theEdgeCase_, undefined}, + {3, undefined, bool, theEdgeCase__, undefined}, + {4, undefined, bool, 'TheEdgeCase', undefined}, + {5, undefined, bool, 'TheEdgeCase_', undefined}, + {6, undefined, bool, 'TheEdgeCase__', undefined} + ]}, + nameConflictTest_types:struct_info_ext('TheEdgeCase') )}, {"TheEdgeCase extended definition", ?_assertEqual( {struct, [ @@ -224,43 +250,43 @@ struct_info_test_() -> {1, undefined, bool, tricky, undefined}, {2, undefined, bool, 'Tricky', undefined} ]}, - nameConflictTest_types:struct_info_ext(tricky_) + nameConflictTest_types:struct_info_ext('Tricky_') )}, {"Nested extended definition", ?_assertEqual( {struct, [ {1, undefined, {struct, { nameConflictTest_types, - classAndProp - }}, 'ClassAndProp', #classAndProp{}}, + 'ClassAndProp' + }}, 'ClassAndProp', #'ClassAndProp'{}}, {2, undefined, {struct, { nameConflictTest_types, second_chance }}, second_chance, #second_chance{}}, {3, undefined, {struct, { nameConflictTest_types, - nOW_EAT_THIS - }}, 'NOW_EAT_THIS', #nOW_EAT_THIS{}}, + 'NOW_EAT_THIS' + }}, 'NOW_EAT_THIS', #'NOW_EAT_THIS'{}}, {4, undefined, {struct, { nameConflictTest_types, - theEdgeCase - }}, 'TheEdgeCase', #theEdgeCase{}}, + 'TheEdgeCase' + }}, 'TheEdgeCase', #'TheEdgeCase'{}}, {5, undefined, {struct, { nameConflictTest_types, - tricky_ - }}, 'Tricky_', #tricky_{}}, + 'Tricky_' + }}, 'Tricky_', #'Tricky_'{}}, {6, undefined, {struct, { nameConflictTest_types, - nested + 'Nested' }}, 'Nested', undefined} ]}, - nameConflictTest_types:struct_info_ext(nested) + nameConflictTest_types:struct_info_ext('Nested') )}, {"Problem_ extended definition", ?_assertEqual( {struct, [ {1, undefined, bool, problem, undefined}, {2, undefined, bool, 'Problem', undefined} ]}, - nameConflictTest_types:struct_info_ext(problem_) + nameConflictTest_types:struct_info_ext('Problem_') )} ]. @@ -279,7 +305,7 @@ service_info_test_() -> extern_thrift:function_info(event, exceptions) )}, {"Foo params", ?_assertEqual( - {struct, [{1, {struct, {nameConflictTest_types, nested}}}]}, + {struct, [{1, {struct, {nameConflictTest_types, 'Nested'}}}]}, extern_thrift:function_info('Foo', params_type) )}, {"Foo reply", ?_assertEqual( @@ -287,7 +313,7 @@ service_info_test_() -> extern_thrift:function_info('Foo', reply_type) )}, {"Foo exceptions", ?_assertEqual( - {struct, [{1, {struct, {nameConflictTest_types, problem_}}}]}, + {struct, [{1, {struct, {nameConflictTest_types, 'Problem_'}}}]}, extern_thrift:function_info('Foo', exceptions) )} ]. \ No newline at end of file
