Vera Filippova commented on THRIFT-4496:

> It is hard to explain why a particular name should be disallowed for 
> everybody if only one out of 20 languages has a problem with it.

I understood it as this: the list of keywords in lexical analyzer serves as a 
guarantee that a service, once passing this check, is compilable to any of the 
supported languages. Even if a generator to one particular language can tweak a 
keyword, the signature of the generated method can be not obvious to a user. 
E.g. If a service has a 'delete' field, a user can expect to have a 'delete' 
method in the generated code. Now if the target language is python, there could 
not possibly be a 'delete' method in a class and it's somewhat confusing which 
method to use then.

However, if the list is not exhaustive, the purpose is not so clear.

> So the question stays, what context you plan to use for {{--screen-keywords}} 
> (and what "screen" means)

Well, English is not my first language. By "screen" I meant modify and make it 
acceptable, and it's not a correct word for this, sorry. Maybe it's better to 
call it {{--allow-keywords}}.

I think that until all generators are able to deal with keywords of their own 
languages, all keywords for supported languages should be deprecated by 
default. It would be nice to make a generator check identifiers only against 
keyword list of its own language, but it would be frustrating if a user decides 
to use one more language and discovers the new generator doesn't work with a 
keyword and he has to rename a service field.

> If the idea is, however, to *get completely rid of the global list and put 
>that buren onto the generators*, that is something I would *absolutely agree* 

This would be the best solution to my problem. To make sure we're on the same 
page though, I'll describe what I'd like to do next.
 * I can move the check that identifier is not a keyword in any of the 
supported languages to generators, to preserve the existing behavior. 
 * Then with a flag {{--allow-keywords}} enabled a generator can skip this 
check and apply its own mechanisms to tweak identifiers and make sure the code 
is compilable or display an error that tweaking keywords is not supported for 
its language.
 * Once all the generators are able to deal with keywords in their languages, 
the list of all keywords can be deleted, substituted by several lists of target 
languages' keywords or maybe generators can allow keywords by default.

> I would also loudly vote for *creating an appropriate set of detailed test 
> cases* before we even begin working on the feature.

Completely agree.

> Screen keywords in service method names
> ---------------------------------------
>                 Key: THRIFT-4496
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4496
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Python - Compiler
>            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

Reply via email to