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 <[email protected]> Differential Revision: https://phabricator.twitter.biz/D467476
twitter · Apr 16, 2020 · b72a08c · b72a08c
1 parent 8d995d7
commit b72a08c
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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,
-Original file line number
+Diff line change
@@ Expand Up @@
     Unreleased
     ----------
+    * scrooge-generator: Respect the proper order of separators in function declarations.
+      ``PHAB_ID=D467476``
 .4.0
     ------
@@ Expand Down @@