scrooge-generator: Stop Failing on Annotations inside Container Types

Problem

Scrooge is incapable of properly parsing Thrift IDL when a container
type (e.g. list, set, map) contains (an) annotation(s) inside the
container type itself. For example, if the type is 'list<i64>', the
'i64' component of the list can contain an annotation. If it has one,
scrooge chokes on it.

Solution

Scrooge now properly parses Thrift IDL that contains these types of
annotations. However, it currently silently discards them, and doesn't
take them into account when doing code generation. If desired, at a
later date we can build full support for them.

Result

Scrooge no longer fails to properly parse Thrift IDL that contains
annotations inside of container types.

JIRA Issues: CSL-10359

Differential Revision: https://phabricator.twitter.biz/D583284

CHANGELOG.rst

@@ -13,6 +13,11 @@ Unreleased
 * scrooge-generator: Java throws an exception when encountering incorrect field
   types in a struct while deserializing. ``PHAB_ID=D565373``
 
+* scrooge-generator: Scrooge no longer fails to parse Thrift IDL when annotations are used
+  on the individual components of a 'container type' (e.g. list, set, map). Those types of
+  annotations are not currently used by scrooge, instead they are skipped over and discarded,
+  but the IDL no longer fails to parse when they are encountered. ``PHAB_ID=D583284``
+
 20.10.0
 -------
 

scrooge-generator-tests/src/test/scala/com/twitter/scrooge/frontend/ThriftParserSpec.scala

@@ -96,17 +96,40 @@ struct MyStruct {}
       parser.parse("binary", parser.fieldType) must be(TBinary)
     }
 
-    "compound types" in {
+    "list types" in {
       parser.parse("list<i64>", parser.fieldType) must be(ListType(TI64, None))
       parser.parse("list<list<string>>", parser.fieldType) must be(
         ListType(ListType(TString, None), None)
       )
-      parser.parse("map<string, list<bool>>", parser.fieldType) must be(
-        MapType(TString, ListType(TBool, None), None)
+      // Annotations within container type definitions are parsed properly, but thrown away
+      parser.parse("list < i64 (something = \"else\") >", parser.fieldType) must be(
+        ListType(TI64, None))
+      parser.parse("list<list<string (a = \"b\")>(c = \"d\")>", parser.fieldType) must be(
+        ListType(ListType(TString, None), None)
       )
+    }
+
+    "set types" in {
       parser.parse("set<Monster>", parser.fieldType) must be(
         SetType(ReferenceType(Identifier("Monster")), None)
       )
+      parser.parse("set<bool (hello = \"goodbye\")>", parser.fieldType) must be(
+        SetType(TBool, None)
+      )
+    }
+
+    "map types" in {
+      parser.parse("map<string, list<bool>>", parser.fieldType) must be(
+        MapType(TString, ListType(TBool, None), None)
+      )
+      parser.parse(
+        "map<string (a=\"b\"), list<bool (c=\"d\")> (e=\"f\")>",
+        parser.fieldType) must be(
+        MapType(TString, ListType(TBool, None), None)
+      )
+    }
+
+    "compound types" in {
       parser.parse("Monster", parser.fieldType) must be(ReferenceType(Identifier("Monster")))
     }
 
@@ -833,8 +856,17 @@ enum Foo
         )
       )
 
-      // TODO(CSL-4127): support annotations on nested types
-      // parser.parse("""typedef list< double ( cpp.fixed_point = "16" ) > tiny_float_list""", parser.typedef) must be()
+      parser.parse(
+        """typedef list< double ( cpp.fixed_point = "16" ) > tiny_float_list""",
+        parser.typedef
+      ) must be(
+        Typedef(
+          SimpleID("tiny_float_list", None),
+          ListType(TDouble, None),
+          Map(),
+          Map()
+        )
+      )
 
       parser.parse(
         """

scrooge-generator/src/main/scala/com/twitter/scrooge/frontend/ThriftParser.scala

@@ -201,18 +201,25 @@ class ThriftParser(
 
   lazy val containerType: Parser[ContainerType] = mapType | setType | listType
 
+  // Annotations within container types are parsed correctly, but currently thrown away
   lazy val mapType: Parser[MapType] =
-    ("map" ~> opt(cppType) <~ "<") ~ (fieldType <~ ",") ~ (fieldType <~ ">") ^^ {
+    ("map" ~> opt(cppType) <~ "<") ~
+      (fieldType <~ defaultedAnnotations <~ ",") ~
+      (fieldType <~ defaultedAnnotations <~ ">") ^^ {
       case cpp ~ key ~ value => MapType(key, value, cpp)
     }
 
-  lazy val setType: Parser[SetType] = ("set" ~> opt(cppType)) ~ ("<" ~> fieldType <~ ">") ^^ {
-    case cpp ~ t => SetType(t, cpp)
-  }
+  // Annotations within container types are parsed correctly, but currently thrown away
+  lazy val setType: Parser[SetType] =
+    ("set" ~> opt(cppType)) ~ ("<" ~> fieldType <~ defaultedAnnotations <~ ">") ^^ {
+      case cpp ~ t => SetType(t, cpp)
+    }
 
-  lazy val listType: Parser[ListType] = ("list" ~ "<") ~> (fieldType <~ ">") ~ opt(cppType) ^^ {
-    case t ~ cpp => ListType(t, cpp)
-  }
+  // Annotations within container types are parsed correctly, but currently thrown away
+  lazy val listType: Parser[ListType] =
+    ("list" ~ "<") ~> (fieldType <~ defaultedAnnotations <~ ">") ~ opt(cppType) ^^ {
+      case t ~ cpp => ListType(t, cpp)
+    }
 
   // FFS. i'm very close to removing this and forcably breaking old thrift files.
   lazy val cppType: Parser[String] = "cpp_type" ~> stringLiteral ^^ { literal => literal.value }