diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 84e057cef..e50a9b8ae 100644 --- a/CHANGELOG.rst +++ b/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 ------- 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 6c845f82d..31d5d2cef 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 @@ -96,17 +96,40 @@ struct MyStruct {} parser.parse("binary", parser.fieldType) must be(TBinary) } - "compound types" in { + "list types" in { parser.parse("list", parser.fieldType) must be(ListType(TI64, None)) parser.parse("list>", parser.fieldType) must be( ListType(ListType(TString, None), None) ) - parser.parse("map>", 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(c = \"d\")>", parser.fieldType) must be( + ListType(ListType(TString, None), None) ) + } + + "set types" in { parser.parse("set", parser.fieldType) must be( SetType(ReferenceType(Identifier("Monster")), None) ) + parser.parse("set", parser.fieldType) must be( + SetType(TBool, None) + ) + } + + "map types" in { + parser.parse("map>", parser.fieldType) must be( + MapType(TString, ListType(TBool, None), None) + ) + parser.parse( + "map (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( """ 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 f6c1b500e..6c2533dc3 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 @@ -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 }