[jira] [Commented] (THRIFT-4496) Dealing with language keywords in Thrift (e.g. service method names)

["Vera Filippova (JIRA)" <ji...@apache.org>]

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