[ 
https://issues.apache.org/jira/browse/THRIFT-4496?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16402196#comment-16402196
 ] 

Vera Filippova commented on THRIFT-4496:
----------------------------------------

 Here is what I plan to do next. Could you please tell me if I'm on a right 
track?

1). Existing generators don't tweak keywords
I can see that many generators (e.g. cpp, java, py) don't modify identifiers 
from the parsed syntax tree node. This is 
[t_cpp_generator::function_signature|https://github.com/apache/thrift/blob/master/compiler/cpp/src/thrift/generate/t_cpp_generator.cc#L4353]:
{code:java}
return "void " + prefix + tfunction->get_name() + "(" + type_name(ttype)
+ (name_params ? "& _return" : "& /* _return */")
+ argument_list(arglist, name_params, true) + ")";
{code}
If this is a bug, as you say, I can write a functional test under 
thrift/compiler/cpp/tests/cpp. The test will create a syntax tree node for 
function/enum/service etc with keyword name, generate output, then compare it 
with a sample with a tweaked name.

2). How to move screening for keywords from a lexical analyzer to generators
Using no common methods to validate/modify an identifier also doesn't give me a 
convenient place to insert a check if an id is a keyword. So the easiest way to 
do it I think is to iterate over the whole syntax tree in the base t_generator 
and check each element with a name against the existing "full" keyword set 
before the generation begins. Then as soon as any child generator can deal with 
keywords in its language, it can turn off the check.

3). Testing the refactoring of keywords screening
The most convenient way for me to test that I didn't break the current behavior 
would be to generate sample thrift files, each file containing one thrift 
element (struct/function/enum etc) with a keyword name. Now if I generate a 
thrift file for each element with each keyword in a list, then run "thrift 
--gen language" for all languages on these files and expect it to fail every 
time, I can be pretty sure my change is covered.
As I understand, there is no place in a current build system where such testing 
script can be built in. There are functional tests for compilers, but each test 
tests only one language. Also I found 
[test/py/explicit_module/runtest.sh|https://github.com/apache/thrift/blob/master/test/py/explicit_module/runtest.sh]
 which does exactly this but only for one case: enum named "from" for py 
generator
{code:java}
../../../compiler/cpp/thrift --gen py test3.thrift && exit 1  # Fail since 
test3.thrift has python keywords
{code}
but It seems this script isn't called from a build system at all?

Can this kind of a testing script fit into the existing testing framework? If 
not, I would do the same in a functional test, but then, can I have one test 
for several languages?

> 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)

Reply via email to