From b72a08c9a2e284f352c68afd9f4728c1f47f196d Mon Sep 17 00:00:00 2001 From: chenzhuoyu Date: Thu, 16 Apr 2020 00:16:06 +0000 Subject: [PATCH] scrooge: fix handling of the function separator Problem The thrift parser is not handling optional function separators correctly. The ; or , separator appears AFTER the annotation section, not before it. https://github.com/apache/thrift/blob/master/compiler/cpp/src/thrift/thrifty.yy#L800 Solution Rearranging the term opt(listSeparator) after defaultedAnnotations solves the problem. Signed-off-by: Bryce Anderson Differential Revision: https://phabricator.twitter.biz/D467476 --- CHANGELOG.rst | 3 +++ .../com/twitter/scrooge/frontend/ThriftParserSpec.scala | 5 +++-- .../scala/com/twitter/scrooge/frontend/ThriftParser.scala | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8ec1fdd6e..7b38a5068 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,9 @@ Note that ``PHAB_ID=#`` and ``RB_ID=#`` correspond to associated messages in com Unreleased ---------- +* scrooge-generator: Respect the proper order of separators in function declarations. + ``PHAB_ID=D467476`` + 20.4.0 ------ diff --git a/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/frontend/ThriftParserSpec.scala b/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/frontend/ThriftParserSpec.scala index 9aae9fbfb..081eff007 100644 --- a/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/frontend/ThriftParserSpec.scala +++ b/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/frontend/ThriftParserSpec.scala @@ -472,7 +472,7 @@ enum Foo val code = """ /** cold hard cache */ service Cache { - void put(1: string name, 2: binary value); + void put(1: string name, 2: binary value) (mode="LRU"); binary get(1: string name) throws (1: NotFoundException ex); } """ @@ -490,7 +490,8 @@ enum Foo Field(2, SimpleID("value"), "value", TBinary, None, Requiredness.Default) ), Seq(), - None + None, + Map("mode" -> "LRU") ), Function( SimpleID("get"), diff --git a/scrooge-generator/src/main/scala/com/twitter/scrooge/frontend/ThriftParser.scala b/scrooge-generator/src/main/scala/com/twitter/scrooge/frontend/ThriftParser.scala index 73b5491d6..8df4799ea 100644 --- a/scrooge-generator/src/main/scala/com/twitter/scrooge/frontend/ThriftParser.scala +++ b/scrooge-generator/src/main/scala/com/twitter/scrooge/frontend/ThriftParser.scala @@ -273,7 +273,7 @@ class ThriftParser( (opt(comments) ~ (opt("oneway") ~ functionType)) ~ (simpleID <~ "(") ~ (rep( field ) <~ ")") ~ - (opt(throws) <~ opt(listSeparator)) ~ defaultedAnnotations ^^ { + opt(throws) ~ defaultedAnnotations <~ opt(listSeparator) ^^ { case comment ~ (oneway ~ ftype) ~ id ~ args ~ throws ~ annotations => Function( id,