Jens Geyer commented on THRIFT-4496:

{quote}> 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.
The parser cannot, and should not, give guarantees about anything that happens 
to its output results. The parser is responsible for a proper syntax tree, 
nothing more. Whether or not any code generated based on that syntax tree is 
compileable or not, is something that the generator is responsible for - not 
the parser.
{quote}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.
Well, we have proper IDEs here with autocomplete ... but anyway, that is not 
more or less confusing as the current implementations. Remember, we already 
have that for a number of languages. In fact, it is the solution that in my 
opinion is superior to any global keyword list that - and this is the only 
useful purpose of such a list - goes beyond the purpose of the DSL syntax 
itself. Not any target language, not the implementors personal preferences, but 
the IDL syntax.
{quote}(...) However, if the list is not exhaustive, the purpose is not so 
 (...) all keywords for supported languages should be deprecated by default
 (...) 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.
Exactly. It all stands and falls with the definition of "exhaustive". Lets 
create a scenario. Assume, we have a compiler that supports 20 target 
languages. The current list is exhaustive in the sense of that, as you propose, 
all keywords of these 20 languages are covered and forbidden. Now we add 
language number 21, and we also have a major update of language 8 which also 
adds a few new keywords. These languages offer keywords that are not in our 
"exhaustive" list yet. Now, we realize that this is a really bad situation, as 
we have no chance to do it right.
 * If we add the new keywords to the list, all of a sudden IDL my files that 
got accepted without complaints yesterday and that would still generate code 
for language number 14 exactly as yesterday, are rejected today. Nobody changed 
a thing at language number 14! But the changed global keyword list now prevents 
me from using otherwise legal identifiers because language 21 was added - a 
language, that I don't even intend to use myself! That approach will break a 
lot of code in the real world with every release. Do you have any plans ready 
to tell that the users out there? Are you prepared for that kind of shitstorm? 
So that seems not to be a good idea.

 * If we, however, don't add these new keywords, the exhaustive list is no 
longer exhaustive and consequently is virtually useless with that new language 
21. Which in turn, as the last, final corrollary and consequence, raises the 
ultimate question: Why do we need that damn thing at all? What purpose does it 
really serve? I mean, other than creating headaches?

*The only conclusion from this dilemma, the only one that really makes any 
sense in the long run, is therefore this: Keep that global list at the absolute 
minimum level and leave all else in the hands of the generators.* This is the 
only approach that is guaranteed to minimize unwanted side effects and is at 
the same time forward-compatible with any future target language number 22, no 
matter how weird or exotic the syntax may be. And it also enables us to deal 
with the language 8 update by modifying just one single generator.

And that is the whole reason why I said: If your proposal is to get rid of that 
global monster, you have my full support. And you get the free advice to 
prepare appropriate test cases beforehand. :)
{quote}I think that until all generators are able to deal with keywords of 
their own languages, [...]
They should already be able to do that today. Anything else is a bug and you 
are free to file another ticket for it. Please add a useful test case.
{quote}Then with a flag {{--allow-keywords}} enabled a generator can skip this 
check and apply its own mechanisms
I still think we don't need that switch. It may be a bit of work, but doable. 
What we could do to ease the process is to hard-wire it in the relevant 
generators, whether or not there should be an additional check be called, made 
against that old global list - but this check is now made at generation stage, 
not during parsing! If the implementation of a particular language is 
considered complete, remove the call from that generator and test it. If the 
last generator has been completed and that check function is not used anymore 
at all, remove it from the code base and declare victory.

PS: Don't forget that we can generate multiple targets with one thrift compiler 
call. That kind of switch would then apply to all of them. But as I see it, we 
don't need it anyway, so why bother about that point.

> 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