thriftpy2 icon indicating copy to clipboard operation
thriftpy2 copied to clipboard

Grammar error when missing field identifiers in service functions

Open dhuang opened this issue 3 years ago • 1 comments

I've observed that thriftpy2 throws a grammar error on load when a definition contains a service function missing its field identifiers.

Traceback with the field identifier:

service MyService {
  void foo(string s)
}
Traceback (most recent call last):
  File "example.py", line 2, in <module>
    thriftpy2.load('example.thrift')
  File "/path/to/thrift/env/lib/python3.7/site-packages/thriftpy2/parser/__init__.py", line 33, in load
    include_dir=include_dir)
  File "/path/to/thrift/env/lib/python3.7/site-packages/thriftpy2/parser/parser.py", line 600, in parse
    parser.parse(data)
  File "/path/to/thrift/env/lib/python3.7/site-packages/ply/yacc.py", line 333, in parse
    return self.parseopt_notrack(input, lexer, debug, tracking, tokenfunc)
  File "/path/to/thrift/env/lib/python3.7/site-packages/ply/yacc.py", line 1201, in parseopt_notrack
    tok = call_errorfunc(self.errorfunc, errtoken, self)
  File "/path/to/thrift/env/lib/python3.7/site-packages/ply/yacc.py", line 192, in call_errorfunc
    r = errorfunc(token)
  File "/path/to/thrift/env/lib/python3.7/site-packages/thriftpy2/parser/parser.py", line 25, in p_error
    (p.value, p.lineno))
thriftpy2.parser.exc.ThriftGrammerError: Grammer error 'string' at line 2

No issue with the field identifier:

service MyService {
  void foo(1: string s)
}

It is my understand that it is best to include them, but I don't believe it's required based on the Thrift IDL, so this behavior is surprising.

dhuang avatar Mar 28 '22 20:03 dhuang

as you said, it is required by thrift spec to provide the backwards-compatibility of IDL

ethe avatar Mar 29 '22 10:03 ethe