From 453f45725e3fd474ff08c0633d8902bf1a8358f4 Mon Sep 17 00:00:00 2001 From: David Hull Date: Fri, 28 Jul 2017 00:09:42 +0000 Subject: [PATCH 1/2] THRIFT-4269: Add Erlang namespace test using ConstantsDemo.thrift. --- lib/erl/Makefile.am | 1 + lib/erl/test/test_const.erl | 34 ++++++++++++++++++++++++++++++++++ test/ConstantsDemo.thrift | 1 + 3 files changed, 36 insertions(+) create mode 100644 lib/erl/test/test_const.erl diff --git a/lib/erl/Makefile.am b/lib/erl/Makefile.am index 2502d3166e9..92e2204f676 100644 --- a/lib/erl/Makefile.am +++ b/lib/erl/Makefile.am @@ -19,6 +19,7 @@ THRIFT = ../../compiler/cpp/thrift THRIFT_FILES = $(wildcard test/*.thrift) \ + ../../test/ConstantsDemo.thrift \ ../../test/NameConflictTest.thrift \ ../../test/ThriftTest.thrift diff --git a/lib/erl/test/test_const.erl b/lib/erl/test/test_const.erl new file mode 100644 index 00000000000..c1f7d423612 --- /dev/null +++ b/lib/erl/test/test_const.erl @@ -0,0 +1,34 @@ +%% +%% 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. +%% + +-module(test_const). + +-ifdef(TEST). +-include_lib("eunit/include/eunit.hrl"). + +-include("gen-erl/constants_demo_types.hrl"). + +namespace_test() -> + %% Verify that records produced by ConstantsDemo.thrift have the right namespace. + io:format(user, "in namespace_test()\n", []), + {struct, _} = constants_demo_types:struct_info('consts.thing'), + {struct, _} = constants_demo_types:struct_info('consts.Blah'), + ok. + +-endif. %% TEST diff --git a/test/ConstantsDemo.thrift b/test/ConstantsDemo.thrift index b99bdb2f35b..a91b2155630 100644 --- a/test/ConstantsDemo.thrift +++ b/test/ConstantsDemo.thrift @@ -18,6 +18,7 @@ */ namespace cpp yozone +namespace erl consts struct thing { 1: i32 hello, From 50fade0cf7cd9b922d5d87c76d21afc9ce62ee15 Mon Sep 17 00:00:00 2001 From: David Hull Date: Thu, 27 Jul 2017 23:26:55 +0000 Subject: [PATCH 2/2] THRIFT-4269: Don't append '.' to Erlang namespace if it ends in '_'. THRIFT-3834 added support for namespaces to the Erlang code generator. However, that support uses a '.' between the namespace name and the type name. This is inconvenient because, although '.' is a valid character in an Erlang atom, atoms that contain '.' must be quoted. This means that a struct named MyStruct in the namespace NS will generate a record named 'NS.MyStruct'. The rules for naming atoms in Erlang are: Atoms begin with a lower-case letter, and may contain alphanumeric characters, underscores (_) or at-signs (@). Alternatively atoms can be specified by enclosing them in single quotes ('), necessary when they start with an uppercase character or contain characters other than underscores and at-signs. This commit changes the Erlang code generator so that if an Erlang namespace ends in a '_' then no '.' is added between the namespace and the struct name when creating the record. This preserves the current behavior unless the namespace ends is a '_', but allow users to override the normal behavior by adding an explicit '_' to the end of their namespace declarations. --- compiler/cpp/src/thrift/generate/t_erl_generator.cc | 13 ++++--------- lib/erl/test/test_const.erl | 4 ++-- test/ConstantsDemo.thrift | 2 +- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/compiler/cpp/src/thrift/generate/t_erl_generator.cc b/compiler/cpp/src/thrift/generate/t_erl_generator.cc index 6054a4e213d..9a5ce16b7f9 100644 --- a/compiler/cpp/src/thrift/generate/t_erl_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_erl_generator.cc @@ -1019,19 +1019,14 @@ string t_erl_generator::argument_list(t_struct* tstruct) { } string t_erl_generator::type_name(t_type* ttype) { - string prefix = ""; - string erl_namespace = ttype->get_program()->get_namespace("erl"); - - if (erl_namespace.length() > 0) { - prefix = erl_namespace + "."; + string prefix = ttype->get_program()->get_namespace("erl"); + size_t prefix_length = prefix.length(); + if (prefix_length > 0 && prefix[prefix_length - 1] != '_') { + prefix += '.'; } string name = ttype->get_name(); - if (ttype->is_struct() || ttype->is_xception() || ttype->is_service()) { - name = ttype->get_name(); - } - return atomify(prefix + name); } diff --git a/lib/erl/test/test_const.erl b/lib/erl/test/test_const.erl index c1f7d423612..ed0cf03eb81 100644 --- a/lib/erl/test/test_const.erl +++ b/lib/erl/test/test_const.erl @@ -27,8 +27,8 @@ namespace_test() -> %% Verify that records produced by ConstantsDemo.thrift have the right namespace. io:format(user, "in namespace_test()\n", []), - {struct, _} = constants_demo_types:struct_info('consts.thing'), - {struct, _} = constants_demo_types:struct_info('consts.Blah'), + {struct, _} = constants_demo_types:struct_info('consts_thing'), + {struct, _} = constants_demo_types:struct_info('consts_Blah'), ok. -endif. %% TEST diff --git a/test/ConstantsDemo.thrift b/test/ConstantsDemo.thrift index a91b2155630..a54534d510d 100644 --- a/test/ConstantsDemo.thrift +++ b/test/ConstantsDemo.thrift @@ -18,7 +18,7 @@ */ namespace cpp yozone -namespace erl consts +namespace erl consts_ struct thing { 1: i32 hello,