[
https://issues.apache.org/jira/browse/THRIFT-4496?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16631993#comment-16631993
]
ASF GitHub Bot commented on THRIFT-4496:
----------------------------------------
jeking3 closed pull request #1567: THRIFT-4496: python specific list of
keywords for python generator
URL: https://github.com/apache/thrift/pull/1567
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/compiler/cpp/CMakeLists.txt b/compiler/cpp/CMakeLists.txt
index 5da28aad44..e61887704e 100644
--- a/compiler/cpp/CMakeLists.txt
+++ b/compiler/cpp/CMakeLists.txt
@@ -62,6 +62,9 @@ set(thrift-compiler_SOURCES
src/thrift/audit/t_audit.cpp
)
+set(thrift_compiler_LANGS
+)
+
# This macro adds an option THRIFT_COMPILER_${NAME}
# that allows enabling or disabling certain languages
macro(THRIFT_ADD_COMPILER name description initial)
@@ -70,6 +73,7 @@ macro(THRIFT_ADD_COMPILER name description initial)
option(${enabler} ${description} ${initial})
if(${enabler})
list(APPEND thrift-compiler_SOURCES ${src})
+ list(APPEND thrift_compiler_LANGS ${name})
endif()
endmacro()
diff --git a/compiler/cpp/src/thrift/generate/t_generator.cc
b/compiler/cpp/src/thrift/generate/t_generator.cc
index 7549d5dc2e..ca3f5dd682 100644
--- a/compiler/cpp/src/thrift/generate/t_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_generator.cc
@@ -76,6 +76,77 @@ void t_generator::generate_program() {
close_generator();
}
+std::set<std::string> t_generator::lang_keywords() const {
+ std::string keywords[] = { "BEGIN", "END", "__CLASS__", "__DIR__",
"__FILE__", "__FUNCTION__",
+ "__LINE__", "__METHOD__", "__NAMESPACE__", "abstract", "alias", "and",
"args", "as",
+ "assert", "begin", "break", "case", "catch", "class", "clone",
"continue", "declare",
+ "def", "default", "del", "delete", "do", "dynamic", "elif", "else",
"elseif", "elsif",
+ "end", "enddeclare", "endfor", "endforeach", "endif", "endswitch",
"endwhile", "ensure",
+ "except", "exec", "finally", "float", "for", "foreach", "from",
"function", "global",
+ "goto", "if", "implements", "import", "in", "inline", "instanceof",
"interface", "is",
+ "lambda", "module", "native", "new", "next", "nil", "not", "or",
"package", "pass",
+ "public", "print", "private", "protected", "raise", "redo", "rescue",
"retry", "register",
+ "return", "self", "sizeof", "static", "super", "switch", "synchronized",
"then", "this",
+ "throw", "transient", "try", "undef", "unless", "unsigned", "until",
"use", "var",
+ "virtual", "volatile", "when", "while", "with", "xor", "yield" };
+ return std::set<std::string>(keywords, keywords +
sizeof(keywords)/sizeof(keywords[0]) );
+}
+
+void t_generator::validate_input() const {
+ validate(program_->get_enums());
+ validate(program_->get_typedefs());
+ validate(program_->get_objects());
+ validate(program_->get_consts());
+ validate(program_->get_services());
+}
+
+template <typename T>
+void t_generator::validate(const vector<T>& list) const{
+ typename vector<T>::const_iterator it;
+ for(it=list.begin(); it != list.end(); ++it) {
+ validate(*it);
+ }
+}
+
+void t_generator::validate(t_function const* f) const {
+ validate_id(f->get_name());
+ validate(f->get_arglist());
+ validate(f->get_xceptions());
+}
+
+void t_generator::validate(t_service const* s) const {
+ validate_id(s->get_name());
+ validate(s->get_functions());
+}
+
+void t_generator::validate(t_enum const* en) const {
+ validate_id(en->get_name());
+ validate(en->get_constants());
+}
+void t_generator::validate(t_struct const* s) const {
+ validate_id(s->get_name());
+ validate(s->get_members());
+}
+
+void t_generator::validate(t_enum_value const* en_val) const {
+ validate_id(en_val->get_name());
+}
+void t_generator::validate(t_typedef const* td) const {
+ validate_id(td->get_name());
+}
+void t_generator::validate(t_const const* c) const {
+ validate_id(c->get_name());
+}
+void t_generator::validate(t_field const* f) const {
+ validate_id(f->get_name());
+}
+
+void t_generator::validate_id(const string& id) const {
+ if (keywords_.find(id) != keywords_.end()) {
+ failure("Cannot use reserved language keyword: \"%s\"", id.c_str());
+ }
+}
+
string t_generator::escape_string(const string& in) const {
string result = "";
for (string::const_iterator it = in.begin(); it < in.end(); it++) {
diff --git a/compiler/cpp/src/thrift/generate/t_generator.h
b/compiler/cpp/src/thrift/generate/t_generator.h
index abe31fec94..cb9d076b50 100644
--- a/compiler/cpp/src/thrift/generate/t_generator.h
+++ b/compiler/cpp/src/thrift/generate/t_generator.h
@@ -42,7 +42,8 @@
*/
class t_generator {
public:
- t_generator(t_program* program) {
+ t_generator(t_program* program)
+ : keywords_(lang_keywords()){
tmp_ = 0;
indent_ = 0;
program_ = program;
@@ -96,7 +97,33 @@ class t_generator {
return escape_string(constval->get_string());
}
+ /**
+ * Check if all identifiers are valid for the target language
+ */
+ virtual void validate_input() const;
+
protected:
+ virtual std::set<std::string> lang_keywords() const;
+
+ /**
+ * A list of reserved words that cannot be used as identifiers.
+ */
+ const std::set<std::string> keywords_;
+
+ virtual void validate_id(const std::string& id) const;
+
+ virtual void validate(t_enum const* en) const;
+ virtual void validate(t_enum_value const* en_val) const;
+ virtual void validate(t_typedef const* td) const;
+ virtual void validate(t_const const* c) const;
+ virtual void validate(t_service const* s) const;
+ virtual void validate(t_struct const* c) const;
+ virtual void validate(t_field const* f) const;
+ virtual void validate(t_function const* f) const;
+
+ template <typename T>
+ void validate(const std::vector<T>& list) const;
+
/**
* Optional methods that may be implemented by subclasses to take necessary
* steps at the beginning or end of code generation.
diff --git a/compiler/cpp/src/thrift/generate/t_py_generator.cc
b/compiler/cpp/src/thrift/generate/t_py_generator.cc
index fba9f9daa5..93ba3d0ecf 100644
--- a/compiler/cpp/src/thrift/generate/t_py_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_py_generator.cc
@@ -51,10 +51,9 @@ class t_py_generator : public t_generator {
t_py_generator(t_program* program,
const std::map<std::string, std::string>& parsed_options,
const std::string& option_string)
- : t_generator(program) {
+ : t_generator (program) {
std::map<std::string, std::string>::const_iterator iter;
-
gen_newstyle_ = true;
gen_utf8strings_ = true;
gen_dynbase_ = false;
@@ -334,6 +333,15 @@ class t_py_generator : public t_generator {
std::string package_dir_;
std::string module_;
+
+protected:
+ virtual std::set<std::string> lang_keywords() {
+ std::string keywords[] = { "False", "None", "True", "and", "as", "assert",
"break", "class",
+ "continue", "def", "del", "elif", "else", "except", "exec",
"finally", "for", "from",
+ "global", "if", "import", "in", "is", "lambda", "nonlocal", "not",
"or", "pass", "print",
+ "raise", "return", "try", "while", "with", "yield" };
+ return std::set<std::string>(keywords, keywords +
sizeof(keywords)/sizeof(keywords[0]) );
+ }
};
/**
diff --git a/compiler/cpp/src/thrift/main.cc b/compiler/cpp/src/thrift/main.cc
index 0c21e0281a..cdc171c7ad 100644
--- a/compiler/cpp/src/thrift/main.cc
+++ b/compiler/cpp/src/thrift/main.cc
@@ -1028,6 +1028,7 @@ void generate(t_program* program, const vector<string>&
generator_strings) {
g_generator_failure = true;
#endif
} else if (generator) {
+ generator->validate_input();
pverbose("Generating \"%s\"\n", iter->c_str());
generator->generate_program();
delete generator;
diff --git a/compiler/cpp/src/thrift/thriftl.ll
b/compiler/cpp/src/thrift/thriftl.ll
index 30669a49df..4f783be96f 100644
--- a/compiler/cpp/src/thrift/thriftl.ll
+++ b/compiler/cpp/src/thrift/thriftl.ll
@@ -76,11 +76,6 @@
#include "thrift/thrifty.hh"
#endif
-void thrift_reserved_keyword(char* keyword) {
- yyerror("Cannot use reserved language keyword: \"%s\"\n", keyword);
- exit(1);
-}
-
void integer_overflow(char* text) {
yyerror("This integer is too big: \"%s\"\n", text);
exit(1);
@@ -269,111 +264,6 @@ literal_begin (['\"])
}
"&" { return tok_reference; }
-
-"BEGIN" { thrift_reserved_keyword(yytext); }
-"END" { thrift_reserved_keyword(yytext); }
-"__CLASS__" { thrift_reserved_keyword(yytext); }
-"__DIR__" { thrift_reserved_keyword(yytext); }
-"__FILE__" { thrift_reserved_keyword(yytext); }
-"__FUNCTION__" { thrift_reserved_keyword(yytext); }
-"__LINE__" { thrift_reserved_keyword(yytext); }
-"__METHOD__" { thrift_reserved_keyword(yytext); }
-"__NAMESPACE__" { thrift_reserved_keyword(yytext); }
-"abstract" { thrift_reserved_keyword(yytext); }
-"alias" { thrift_reserved_keyword(yytext); }
-"and" { thrift_reserved_keyword(yytext); }
-"args" { thrift_reserved_keyword(yytext); }
-"as" { thrift_reserved_keyword(yytext); }
-"assert" { thrift_reserved_keyword(yytext); }
-"begin" { thrift_reserved_keyword(yytext); }
-"break" { thrift_reserved_keyword(yytext); }
-"case" { thrift_reserved_keyword(yytext); }
-"catch" { thrift_reserved_keyword(yytext); }
-"class" { thrift_reserved_keyword(yytext); }
-"clone" { thrift_reserved_keyword(yytext); }
-"continue" { thrift_reserved_keyword(yytext); }
-"declare" { thrift_reserved_keyword(yytext); }
-"def" { thrift_reserved_keyword(yytext); }
-"default" { thrift_reserved_keyword(yytext); }
-"del" { thrift_reserved_keyword(yytext); }
-"delete" { thrift_reserved_keyword(yytext); }
-"do" { thrift_reserved_keyword(yytext); }
-"dynamic" { thrift_reserved_keyword(yytext); }
-"elif" { thrift_reserved_keyword(yytext); }
-"else" { thrift_reserved_keyword(yytext); }
-"elseif" { thrift_reserved_keyword(yytext); }
-"elsif" { thrift_reserved_keyword(yytext); }
-"end" { thrift_reserved_keyword(yytext); }
-"enddeclare" { thrift_reserved_keyword(yytext); }
-"endfor" { thrift_reserved_keyword(yytext); }
-"endforeach" { thrift_reserved_keyword(yytext); }
-"endif" { thrift_reserved_keyword(yytext); }
-"endswitch" { thrift_reserved_keyword(yytext); }
-"endwhile" { thrift_reserved_keyword(yytext); }
-"ensure" { thrift_reserved_keyword(yytext); }
-"except" { thrift_reserved_keyword(yytext); }
-"exec" { thrift_reserved_keyword(yytext); }
-"finally" { thrift_reserved_keyword(yytext); }
-"float" { thrift_reserved_keyword(yytext); }
-"for" { thrift_reserved_keyword(yytext); }
-"foreach" { thrift_reserved_keyword(yytext); }
-"from" { thrift_reserved_keyword(yytext); }
-"function" { thrift_reserved_keyword(yytext); }
-"global" { thrift_reserved_keyword(yytext); }
-"goto" { thrift_reserved_keyword(yytext); }
-"if" { thrift_reserved_keyword(yytext); }
-"implements" { thrift_reserved_keyword(yytext); }
-"import" { thrift_reserved_keyword(yytext); }
-"in" { thrift_reserved_keyword(yytext); }
-"inline" { thrift_reserved_keyword(yytext); }
-"instanceof" { thrift_reserved_keyword(yytext); }
-"interface" { thrift_reserved_keyword(yytext); }
-"is" { thrift_reserved_keyword(yytext); }
-"lambda" { thrift_reserved_keyword(yytext); }
-"module" { thrift_reserved_keyword(yytext); }
-"native" { thrift_reserved_keyword(yytext); }
-"new" { thrift_reserved_keyword(yytext); }
-"next" { thrift_reserved_keyword(yytext); }
-"nil" { thrift_reserved_keyword(yytext); }
-"not" { thrift_reserved_keyword(yytext); }
-"or" { thrift_reserved_keyword(yytext); }
-"package" { thrift_reserved_keyword(yytext); }
-"pass" { thrift_reserved_keyword(yytext); }
-"public" { thrift_reserved_keyword(yytext); }
-"print" { thrift_reserved_keyword(yytext); }
-"private" { thrift_reserved_keyword(yytext); }
-"protected" { thrift_reserved_keyword(yytext); }
-"raise" { thrift_reserved_keyword(yytext); }
-"redo" { thrift_reserved_keyword(yytext); }
-"rescue" { thrift_reserved_keyword(yytext); }
-"retry" { thrift_reserved_keyword(yytext); }
-"register" { thrift_reserved_keyword(yytext); }
-"return" { thrift_reserved_keyword(yytext); }
-"self" { thrift_reserved_keyword(yytext); }
-"sizeof" { thrift_reserved_keyword(yytext); }
-"static" { thrift_reserved_keyword(yytext); }
-"super" { thrift_reserved_keyword(yytext); }
-"switch" { thrift_reserved_keyword(yytext); }
-"synchronized" { thrift_reserved_keyword(yytext); }
-"then" { thrift_reserved_keyword(yytext); }
-"this" { thrift_reserved_keyword(yytext); }
-"throw" { thrift_reserved_keyword(yytext); }
-"transient" { thrift_reserved_keyword(yytext); }
-"try" { thrift_reserved_keyword(yytext); }
-"undef" { thrift_reserved_keyword(yytext); }
-"unless" { thrift_reserved_keyword(yytext); }
-"unsigned" { thrift_reserved_keyword(yytext); }
-"until" { thrift_reserved_keyword(yytext); }
-"use" { thrift_reserved_keyword(yytext); }
-"var" { thrift_reserved_keyword(yytext); }
-"virtual" { thrift_reserved_keyword(yytext); }
-"volatile" { thrift_reserved_keyword(yytext); }
-"when" { thrift_reserved_keyword(yytext); }
-"while" { thrift_reserved_keyword(yytext); }
-"with" { thrift_reserved_keyword(yytext); }
-"xor" { thrift_reserved_keyword(yytext); }
-"yield" { thrift_reserved_keyword(yytext); }
-
{intconstant} {
errno = 0;
yylval.iconst = strtoll(yytext, NULL, 10);
diff --git a/compiler/cpp/test/CMakeLists.txt b/compiler/cpp/test/CMakeLists.txt
index 7cf98a5176..a09f23d7c2 100644
--- a/compiler/cpp/test/CMakeLists.txt
+++ b/compiler/cpp/test/CMakeLists.txt
@@ -76,5 +76,17 @@ if(${WITH_PLUGIN})
-P ${CMAKE_CURRENT_SOURCE_DIR}/cpp_plugin_test.cmake)
endif()
+file(GLOB KEYWORD_SAMPLES
"${CMAKE_CURRENT_SOURCE_DIR}/keyword-samples/*.thrift")
+foreach(LANG ${thrift_compiler_LANGS})
+ foreach(SAMPLE ${KEYWORD_SAMPLES})
+ get_filename_component(FILENAME ${SAMPLE} NAME_WE)
+ add_test(NAME "${LANG}_${FILENAME}"
+ COMMAND thrift-compiler --gen ${LANG} ${SAMPLE})
+ set_tests_properties("${LANG}_${FILENAME}" PROPERTIES
+ PASS_REGULAR_EXPRESSION "Cannot use reserved language keyword")
+ endforeach()
+endforeach()
+
+
find_package(PythonInterp REQUIRED)
-add_test(NAME StalenessCheckTest COMMAND ${PYTHON_EXECUTABLE}
${CMAKE_CURRENT_SOURCE_DIR}/compiler/staleness_check.py ${THRIFT_COMPILER})
\ No newline at end of file
+add_test(NAME StalenessCheckTest COMMAND ${PYTHON_EXECUTABLE}
${CMAKE_CURRENT_SOURCE_DIR}/compiler/staleness_check.py ${THRIFT_COMPILER})
diff --git a/compiler/cpp/test/keyword-samples/const1_return.thrift
b/compiler/cpp/test/keyword-samples/const1_return.thrift
new file mode 100644
index 0000000000..735e4acd86
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/const1_return.thrift
@@ -0,0 +1 @@
+const bool return = 0
diff --git a/compiler/cpp/test/keyword-samples/enum1_return.thrift
b/compiler/cpp/test/keyword-samples/enum1_return.thrift
new file mode 100644
index 0000000000..6d834e1da8
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/enum1_return.thrift
@@ -0,0 +1,2 @@
+enum return {
+}
diff --git a/compiler/cpp/test/keyword-samples/enum2_return.thrift
b/compiler/cpp/test/keyword-samples/enum2_return.thrift
new file mode 100644
index 0000000000..a2caa8e148
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/enum2_return.thrift
@@ -0,0 +1,3 @@
+enum enum_name {
+ return
+}
diff --git a/compiler/cpp/test/keyword-samples/exception1_return.thrift
b/compiler/cpp/test/keyword-samples/exception1_return.thrift
new file mode 100644
index 0000000000..eadb338345
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/exception1_return.thrift
@@ -0,0 +1 @@
+exception return {}
diff --git a/compiler/cpp/test/keyword-samples/exception2_return.thrift
b/compiler/cpp/test/keyword-samples/exception2_return.thrift
new file mode 100644
index 0000000000..493c352976
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/exception2_return.thrift
@@ -0,0 +1,3 @@
+exception exception_name {
+ 1: required i8 return
+}
diff --git a/compiler/cpp/test/keyword-samples/service1_return.thrift
b/compiler/cpp/test/keyword-samples/service1_return.thrift
new file mode 100644
index 0000000000..5286a3691a
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/service1_return.thrift
@@ -0,0 +1 @@
+service return {}
diff --git a/compiler/cpp/test/keyword-samples/service2_return.thrift
b/compiler/cpp/test/keyword-samples/service2_return.thrift
new file mode 100644
index 0000000000..6f7331da0f
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/service2_return.thrift
@@ -0,0 +1,3 @@
+service service_name {
+ bool function_name(1: i32 return)
+}
diff --git a/compiler/cpp/test/keyword-samples/service3_return.thrift
b/compiler/cpp/test/keyword-samples/service3_return.thrift
new file mode 100644
index 0000000000..c6dd946fd3
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/service3_return.thrift
@@ -0,0 +1,3 @@
+service service_name {
+ void return()
+}
diff --git a/compiler/cpp/test/keyword-samples/service4_return.thrift
b/compiler/cpp/test/keyword-samples/service4_return.thrift
new file mode 100644
index 0000000000..d0787dfdee
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/service4_return.thrift
@@ -0,0 +1,5 @@
+exception exception_name {}
+
+service service_name {
+ void function_name() throws ( 1: exception_name return)
+}
diff --git a/compiler/cpp/test/keyword-samples/struct1_return.thrift
b/compiler/cpp/test/keyword-samples/struct1_return.thrift
new file mode 100644
index 0000000000..c82b8b9caf
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/struct1_return.thrift
@@ -0,0 +1 @@
+struct return {}
diff --git a/compiler/cpp/test/keyword-samples/struct2_return.thrift
b/compiler/cpp/test/keyword-samples/struct2_return.thrift
new file mode 100644
index 0000000000..a0700d101a
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/struct2_return.thrift
@@ -0,0 +1,3 @@
+struct struct_name {
+ 1: required bool return = 1
+}
diff --git a/compiler/cpp/test/keyword-samples/typedef1_return.thrift
b/compiler/cpp/test/keyword-samples/typedef1_return.thrift
new file mode 100644
index 0000000000..f159bb880e
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/typedef1_return.thrift
@@ -0,0 +1 @@
+typedef bool return
diff --git a/compiler/cpp/test/keyword-samples/union1_return.thrift
b/compiler/cpp/test/keyword-samples/union1_return.thrift
new file mode 100644
index 0000000000..368df1383e
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/union1_return.thrift
@@ -0,0 +1 @@
+union return {}
diff --git a/compiler/cpp/test/keyword-samples/union2_return.thrift
b/compiler/cpp/test/keyword-samples/union2_return.thrift
new file mode 100644
index 0000000000..9719d1e40c
--- /dev/null
+++ b/compiler/cpp/test/keyword-samples/union2_return.thrift
@@ -0,0 +1,3 @@
+union union_name {
+ 1: optional bool return=1
+}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Dealing with language keywords in Thrift (e.g. service method names)
> --------------------------------------------------------------------
>
> Key: THRIFT-4496
> URL: https://issues.apache.org/jira/browse/THRIFT-4496
> Project: Thrift
> Issue Type: New Feature
> Components: Compiler (General)
> Reporter: Vera Filippova
> Priority: Minor
>
> Apache Thrift compiler doesn't allow to use keywords in any of supported
> languages as field names. However, there are other compilers, like Scrooge,
> which do allow using some keywords as field identifiers, which leads to
> incompatibility.
> Assume we had a service with 'delete' method, with Java code generated by
> Scrooge. Now we'd like to generate Python code with Apache Thrift, but
> encounter an error because of the 'delete' keyword.
> I understand that using only Apache Thrift compiler, a user will never
> encounter this problem, but I think enabling keywords by request seems
> feasible.
> h1. Proposal
> It's possible to tweak keywords on code generation stage, e.g. use 'delete_'
> as a name of a generated function instead of 'delete', then use the original
> method name for a protocol message: writeMethodBegin('delete').
> This feature could be enabled with an additional flag, e.g. --screen-keywords.
> I have a draft for python generator here [https://github.com/nsrtvwls/thrift]
> The questions are, is this functionality welcome? If yes, would it require to
> have it supported for all languages?
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
