THRIFT-2689 struct names that differ only in capitalization of first character generate broken erlang code
Client: Erlang
Patch: Alisdair Sullivan
This closes #204
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 @@
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 @@
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 @@
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_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 @@
} 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 @@
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 @@
"-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 @@
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 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::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", "")
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 @@
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
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 @@
#partial{using=null}
)},
{"ClassAndProp record", ?_assertMatch(
- {classAndProp, _, _, _, _},
- #classAndProp{
+ {'ClassAndProp', _, _, _, _},
+ #'ClassAndProp'{
'ClassAndProp'=null,
'ClassAndProp_'=null,
'ClassAndProp__'=null,
@@ -61,8 +61,8 @@
}
)},
{"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 @@
}
)},
{"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 @@
}
)},
{"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 @@
}
)},
{"Problem_ record", ?_assertMatch(
- {problem_, _, _},
- #problem_{problem=null,'Problem'=null}
+ {'Problem_', _, _},
+ #'Problem_'{problem=null,'Problem'=null}
)}
].
@@ -124,7 +135,7 @@
)},
{"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 @@
)},
{"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 @@
{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 @@
{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 @@
{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 @@
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 @@
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