Protobufs: Added '--oneof-union' option. (#4647)

* Added '--oneof-union' option.

Used with the .proto -> .fbs converter, will translate protobuff oneofs to flatbuffer unions.
Updated proto test to check both methods of converting oneofs.

* Added '--oneof-union' option.

Used with the .proto -> .fbs converter, will translate protobuff oneofs to flatbuffer unions.
Updated proto test to check both methods of converting oneofs.

* FlatBuffers: Moved MakeCamel() into idl_parser.cpp

Removes library dependency on Java/C# generator code.

BUILD

@@ -136,6 +136,7 @@ cc_test(
         ":tests/prototest/imported.proto",
         ":tests/prototest/test.golden",
         ":tests/prototest/test.proto",
+        ":tests/prototest/test_union.golden",
         ":tests/union_vector/union_vector.fbs",
     ],
     includes = ["include/"],

include/flatbuffers/idl.h

@@ -367,6 +367,7 @@ struct IDLOptions {
   bool mutable_buffer;
   bool one_file;
   bool proto_mode;
+  bool proto_oneof_union;
   bool generate_all;
   bool skip_unexpected_fields_in_json;
   bool generate_name_strings;
@@ -426,6 +427,7 @@ struct IDLOptions {
         mutable_buffer(false),
         one_file(false),
         proto_mode(false),
+        proto_oneof_union(false),
         generate_all(false),
         skip_unexpected_fields_in_json(false),
         generate_name_strings(false),
@@ -661,6 +663,9 @@ class Parser : public ParserState {
   FLATBUFFERS_CHECKED_ERROR ParseNamespace();
   FLATBUFFERS_CHECKED_ERROR StartStruct(const std::string &name,
                                         StructDef **dest);
+  FLATBUFFERS_CHECKED_ERROR StartEnum(const std::string &name,
+                                      bool is_union,
+                                      EnumDef **dest);
   FLATBUFFERS_CHECKED_ERROR ParseDecl();
   FLATBUFFERS_CHECKED_ERROR ParseService();
   FLATBUFFERS_CHECKED_ERROR ParseProtoFields(StructDef *struct_def,

src/flatc.cpp

@@ -101,6 +101,7 @@ std::string FlatCompiler::GetUsageString(const char *program_name) const {
     "  --raw-binary       Allow binaries without file_indentifier to be read.\n"
     "                     This may crash flatc given a mismatched schema.\n"
     "  --proto            Input is a .proto, translate to .fbs.\n"
+    "  --oneof-union      Translate .proto oneofs to flatbuffer unions.\n"
     "  --grpc             Generate GRPC interfaces for the specified languages\n"
     "  --schema           Serialize schemas instead of JSON (use with -b)\n"
     "  --bfbs-comments    Add doc comments to the binary schema files.\n"
@@ -236,6 +237,8 @@ int FlatCompiler::Compile(int argc, const char **argv) {
         binary_files_from = filenames.size();
       } else if (arg == "--proto") {
         opts.proto_mode = true;
+      } else if (arg == "--oneof-union") {
+        opts.proto_oneof_union = true;
       } else if (arg == "--schema") {
         schema_binary = true;
       } else if (arg == "-M") {

src/idl_gen_fbs.cpp

@@ -91,13 +91,19 @@ std::string GenerateFBS(const Parser &parser, const std::string &file_name) {
     EnumDef &enum_def = **enum_def_it;
     GenNameSpace(*enum_def.defined_namespace, &schema, &last_namespace);
     GenComment(enum_def.doc_comment, &schema, nullptr);
-    schema += "enum " + enum_def.name + " : ";
+    if (enum_def.is_union)
+      schema += "union " + enum_def.name;
+    else
+      schema += "enum " + enum_def.name + " : ";
     schema += GenType(enum_def.underlying_type, true) + " {\n";
     for (auto it = enum_def.vals.vec.begin(); it != enum_def.vals.vec.end();
          ++it) {
       auto &ev = **it;
       GenComment(ev.doc_comment, &schema, nullptr, "  ");
-      schema += "  " + ev.name + " = " + NumToString(ev.value) + ",\n";
+      if (enum_def.is_union)
+        schema += "  " + GenType(ev.union_type) + ",\n";
+      else
+        schema += "  " + ev.name + " = " + NumToString(ev.value) + ",\n";
     }
     schema += "}\n\n";
   }

src/idl_gen_general.cpp

@@ -27,21 +27,6 @@
 
 namespace flatbuffers {
 
-// Convert an underscore_based_indentifier in to camelCase.
-// Also uppercases the first character if first is true.
-std::string MakeCamel(const std::string &in, bool first) {
-  std::string s;
-  for (size_t i = 0; i < in.length(); i++) {
-    if (!i && first)
-      s += static_cast<char>(toupper(in[0]));
-    else if (in[i] == '_' && i + 1 < in.length())
-      s += static_cast<char>(toupper(in[++i]));
-    else
-      s += in[i];
-  }
-  return s;
-}
-
 // These arrays need to correspond to the IDLOptions::k enum.
 
 struct LanguageParameters {

src/idl_parser.cpp

@@ -76,6 +76,21 @@ static bool ValidateUTF8(const std::string &str) {
   return true;
 }
 
+// Convert an underscore_based_indentifier in to camelCase.
+// Also uppercases the first character if first is true.
+std::string MakeCamel(const std::string &in, bool first) {
+  std::string s;
+  for (size_t i = 0; i < in.length(); i++) {
+    if (!i && first)
+      s += static_cast<char>(toupper(in[0]));
+    else if (in[i] == '_' && i + 1 < in.length())
+      s += static_cast<char>(toupper(in[++i]));
+    else
+      s += in[i];
+  }
+  return s;
+}
+
 void Parser::Message(const std::string &msg) {
   error_ = file_being_parsed_.length() ? AbsolutePath(file_being_parsed_) : "";
   // clang-format off
@@ -1467,42 +1482,29 @@ CheckedError Parser::ParseEnum(bool is_union, EnumDef **dest) {
   NEXT();
   std::string enum_name = attribute_;
   EXPECT(kTokenIdentifier);
-  auto &enum_def = *new EnumDef();
-  enum_def.name = enum_name;
-  enum_def.file = file_being_parsed_;
-  enum_def.doc_comment = enum_comment;
-  enum_def.is_union = is_union;
-  enum_def.defined_namespace = current_namespace_;
-  if (enums_.Add(current_namespace_->GetFullyQualifiedName(enum_name),
-                 &enum_def))
-    return Error("enum already exists: " + enum_name);
-  if (is_union) {
-    enum_def.underlying_type.base_type = BASE_TYPE_UTYPE;
-    enum_def.underlying_type.enum_def = &enum_def;
-  } else {
-    if (opts.proto_mode) {
-      enum_def.underlying_type.base_type = BASE_TYPE_INT;
+  EnumDef *enum_def;
+  ECHECK(StartEnum(enum_name, is_union, &enum_def));
+  enum_def->doc_comment = enum_comment;
+  if (!is_union && !opts.proto_mode) {
+    // Give specialized error message, since this type spec used to
+    // be optional in the first FlatBuffers release.
+    if (!Is(':')) {
+      return Error(
+          "must specify the underlying integer type for this"
+          " enum (e.g. \': short\', which was the default).");
     } else {
-      // Give specialized error message, since this type spec used to
-      // be optional in the first FlatBuffers release.
-      if (!Is(':')) {
-        return Error(
-            "must specify the underlying integer type for this"
-            " enum (e.g. \': short\', which was the default).");
-      } else {
-        NEXT();
-      }
-      // Specify the integer type underlying this enum.
-      ECHECK(ParseType(enum_def.underlying_type));
-      if (!IsInteger(enum_def.underlying_type.base_type))
-        return Error("underlying enum type must be integral");
+      NEXT();
     }
+    // Specify the integer type underlying this enum.
+    ECHECK(ParseType(enum_def->underlying_type));
+    if (!IsInteger(enum_def->underlying_type.base_type))
+      return Error("underlying enum type must be integral");
     // Make this type refer back to the enum it was derived from.
-    enum_def.underlying_type.enum_def = &enum_def;
+    enum_def->underlying_type.enum_def = enum_def;
   }
-  ECHECK(ParseMetaData(&enum_def.attributes));
+  ECHECK(ParseMetaData(&enum_def->attributes));
   EXPECT('{');
-  if (is_union) enum_def.vals.Add("NONE", new EnumVal("NONE", 0));
+  if (is_union) enum_def->vals.Add("NONE", new EnumVal("NONE", 0));
   for (;;) {
     if (opts.proto_mode && attribute_ == "option") {
       ECHECK(ParseProtoOption());
@@ -1520,11 +1522,12 @@ CheckedError Parser::ParseEnum(bool is_union, EnumDef **dest) {
           std::replace(value_name.begin(), value_name.end(), '.', '_');
         }
       }
-      auto prevsize = enum_def.vals.vec.size();
-      auto value =
-          !enum_def.vals.vec.empty() ? enum_def.vals.vec.back()->value + 1 : 0;
+      auto prevsize = enum_def->vals.vec.size();
+      auto value = !enum_def->vals.vec.empty()
+          ? enum_def->vals.vec.back()->value + 1
+          : 0;
       auto &ev = *new EnumVal(value_name, value);
-      if (enum_def.vals.Add(value_name, &ev))
+      if (enum_def->vals.Add(value_name, &ev))
         return Error("enum value already exists: " + value_name);
       ev.doc_comment = value_comment;
       if (is_union) {
@@ -1534,7 +1537,7 @@ CheckedError Parser::ParseEnum(bool is_union, EnumDef **dest) {
           if (ev.union_type.base_type != BASE_TYPE_STRUCT &&
               ev.union_type.base_type != BASE_TYPE_STRING)
             return Error("union value type may only be table/struct/string");
-          enum_def.uses_type_aliases = true;
+          enum_def->uses_type_aliases = true;
         } else {
           ev.union_type = Type(BASE_TYPE_STRUCT, LookupCreateStruct(full_name));
         }
@@ -1544,7 +1547,7 @@ CheckedError Parser::ParseEnum(bool is_union, EnumDef **dest) {
         ev.value = StringToInt(attribute_.c_str());
         EXPECT(kTokenIntegerConstant);
         if (!opts.proto_mode && prevsize &&
-            enum_def.vals.vec[prevsize - 1]->value >= ev.value)
+            enum_def->vals.vec[prevsize - 1]->value >= ev.value)
           return Error("enum values must be specified in ascending order");
       }
       if (is_union) {
@@ -1563,18 +1566,18 @@ CheckedError Parser::ParseEnum(bool is_union, EnumDef **dest) {
     if (Is('}')) break;
   }
   EXPECT('}');
-  if (enum_def.attributes.Lookup("bit_flags")) {
-    for (auto it = enum_def.vals.vec.begin(); it != enum_def.vals.vec.end();
+  if (enum_def->attributes.Lookup("bit_flags")) {
+    for (auto it = enum_def->vals.vec.begin(); it != enum_def->vals.vec.end();
          ++it) {
       if (static_cast<size_t>((*it)->value) >=
-          SizeOf(enum_def.underlying_type.base_type) * 8)
+          SizeOf(enum_def->underlying_type.base_type) * 8)
         return Error("bit flag out of range of underlying integral type");
       (*it)->value = 1LL << (*it)->value;
     }
   }
-  if (dest) *dest = &enum_def;
-  types_.Add(current_namespace_->GetFullyQualifiedName(enum_def.name),
-             new Type(BASE_TYPE_UNION, nullptr, &enum_def));
+  if (dest) *dest = enum_def;
+  types_.Add(current_namespace_->GetFullyQualifiedName(enum_def->name),
+             new Type(BASE_TYPE_UNION, nullptr, enum_def));
   return NoError();
 }
 
@@ -1864,6 +1867,24 @@ CheckedError Parser::ParseProtoDecl() {
   return NoError();
 }
 
+CheckedError Parser::StartEnum(const std::string &enum_name, bool is_union,
+                               EnumDef **dest) {
+  auto &enum_def = *new EnumDef();
+  enum_def.name = enum_name;
+  enum_def.file = file_being_parsed_;
+  enum_def.doc_comment = doc_comment_;
+  enum_def.is_union = is_union;
+  enum_def.defined_namespace = current_namespace_;
+  if (enums_.Add(current_namespace_->GetFullyQualifiedName(enum_name),
+                 &enum_def))
+    return Error("enum already exists: " + enum_name);
+  enum_def.underlying_type.base_type = is_union ? BASE_TYPE_UTYPE
+                                                : BASE_TYPE_INT;
+  enum_def.underlying_type.enum_def = &enum_def;
+  if (dest) *dest = &enum_def;
+  return NoError();
+}
+
 CheckedError Parser::ParseProtoFields(StructDef *struct_def, bool isextend,
                                       bool inside_oneof) {
   EXPECT('{');
@@ -1910,12 +1931,19 @@ CheckedError Parser::ParseProtoFields(StructDef *struct_def, bool isextend,
         }
       }
       StructDef *anonymous_struct = nullptr;
+      EnumDef *oneof_union = nullptr;
       Type type;
       if (IsIdent("group") || oneof) {
         if (!oneof) NEXT();
-        auto name = "Anonymous" + NumToString(anonymous_counter++);
-        ECHECK(StartStruct(name, &anonymous_struct));
-        type = Type(BASE_TYPE_STRUCT, anonymous_struct);
+        if (oneof && opts.proto_oneof_union) {
+          auto name = MakeCamel(attribute_, true) + "Union";
+          ECHECK(StartEnum(name, true, &oneof_union));
+          type = Type(BASE_TYPE_UNION, nullptr, oneof_union);
+        } else {
+          auto name = "Anonymous" + NumToString(anonymous_counter++);
+          ECHECK(StartStruct(name, &anonymous_struct));
+          type = Type(BASE_TYPE_STRUCT, anonymous_struct);
+        }
       } else {
         ECHECK(ParseTypeFromProtoType(&type));
       }
@@ -1974,6 +2002,27 @@ CheckedError Parser::ParseProtoFields(StructDef *struct_def, bool isextend,
       if (anonymous_struct) {
         ECHECK(ParseProtoFields(anonymous_struct, false, oneof));
         if (Is(';')) NEXT();
+      } else if (oneof_union) {
+        // Parse into a temporary StructDef, then transfer fields into an
+        // EnumDef describing the oneof as a union.
+        StructDef oneof_struct;
+        ECHECK(ParseProtoFields(&oneof_struct, false, oneof));
+        if (Is(';')) NEXT();
+        for (auto field_it = oneof_struct.fields.vec.begin();
+             field_it != oneof_struct.fields.vec.end(); ++field_it) {
+          const auto &oneof_field = **field_it;
+          const auto &oneof_type = oneof_field.value.type;
+          if (oneof_type.base_type != BASE_TYPE_STRUCT ||
+              !oneof_type.struct_def || oneof_type.struct_def->fixed)
+            return Error("oneof '" + name +
+                "' cannot be mapped to a union because member '" +
+                oneof_field.name + "' is not a table type.");
+          auto enum_val = new EnumVal(oneof_type.struct_def->name,
+                                      oneof_union->vals.vec.size());
+          enum_val->union_type = oneof_type;
+          enum_val->doc_comment = oneof_field.doc_comment;
+          oneof_union->vals.Add(oneof_field.name, enum_val);
+        }
       } else {
         EXPECT(';');
       }

tests/prototest/test.golden

@@ -36,6 +36,8 @@ table ProtoMessage {
   n:proto.test.ProtoMessage_.OtherMessage;
   o:[string];
   z:proto.test.ImportedMessage;
+  /// doc comment for r.
+  r:proto.test.ProtoMessage_.Anonymous0;
 }
 
 namespace proto.test.ProtoMessage_;
@@ -46,3 +48,11 @@ table OtherMessage {
   b:float = 3.14149;
 }
 
+table Anonymous0 {
+  /// doc comment for s.
+  s:proto.test.ImportedMessage;
+  /// doc comment for t on 2
+  /// lines.
+  t:proto.test.ProtoMessage_.OtherMessage;
+}
+

tests/prototest/test.proto

@@ -42,4 +42,12 @@ message ProtoMessage {
   optional OtherMessage n = 12;
   repeated string o = 14;
   optional ImportedMessage z = 16;
+  /// doc comment for r.
+  oneof r {
+    /// doc comment for s.
+    ImportedMessage s = 17;
+    /// doc comment for t on 2
+    /// lines.
+    OtherMessage t = 18;
+  }
 }

tests/prototest/test_union.golden

+// Generated from test.proto
+
+namespace proto.test;
+
+/// Enum doc comment.
+enum ProtoEnum : int {
+  FOO = 1,
+  /// Enum 2nd value doc comment misaligned.
+  BAR = 5,
+}
+
+namespace proto.test.ProtoMessage_;
+
+union RUnion {
+  /// doc comment for s.
+  proto.test.ImportedMessage,
+  /// doc comment for t on 2
+  /// lines.
+  proto.test.ProtoMessage_.OtherMessage,
+}
+
+namespace proto.test;
+
+table ImportedMessage {
+  a:int;
+}
+
+/// 2nd table doc comment with
+/// many lines.
+table ProtoMessage {
+  c:int = 16;
+  d:long;
+  p:uint;
+  e:ulong;
+  /// doc comment for f.
+  f:int = -1;
+  g:long;
+  h:uint;
+  q:ulong;
+  i:int;
+  j:long;
+  /// doc comment for k.
+  k:bool;
+  /// doc comment for l on 2
+  /// lines
+  l:string (required);
+  m:[ubyte];
+  n:proto.test.ProtoMessage_.OtherMessage;
+  o:[string];
+  z:proto.test.ImportedMessage;
+  /// doc comment for r.
+  r:proto.test.ProtoMessage_.RUnion;
+}
+
+namespace proto.test.ProtoMessage_;
+
+table OtherMessage {
+  a:double;
+  /// doc comment for b.
+  b:float = 3.14149;
+}
+

tests/test.cpp

@@ -863,6 +863,7 @@ void ParseProtoTest() {
   // load the .proto and the golden file from disk
   std::string protofile;
   std::string goldenfile;
+  std::string goldenunionfile;
   TEST_EQ(
       flatbuffers::LoadFile((test_data_path + "prototest/test.proto").c_str(),
                             false, &protofile),
@@ -871,6 +872,11 @@ void ParseProtoTest() {
       flatbuffers::LoadFile((test_data_path + "prototest/test.golden").c_str(),
                             false, &goldenfile),
       true);
+  TEST_EQ(
+      flatbuffers::LoadFile((test_data_path +
+                            "prototest/test_union.golden").c_str(),
+                            false, &goldenunionfile),
+      true);
 
   flatbuffers::IDLOptions opts;
   opts.include_dependence_headers = false;
@@ -889,6 +895,19 @@ void ParseProtoTest() {
   flatbuffers::Parser parser2;
   TEST_EQ(parser2.Parse(fbs.c_str(), nullptr), true);
   TEST_EQ_STR(fbs.c_str(), goldenfile.c_str());
+
+  // Parse proto with --oneof-union option.
+  opts.proto_oneof_union = true;
+  flatbuffers::Parser parser3(opts);
+  TEST_EQ(parser3.Parse(protofile.c_str(), include_directories), true);
+
+  // Generate fbs.
+  auto fbs_union = flatbuffers::GenerateFBS(parser3, "test");
+
+  // Ensure generated file is parsable.
+  flatbuffers::Parser parser4;
+  TEST_EQ(parser4.Parse(fbs_union.c_str(), nullptr), true);
+  TEST_EQ_STR(fbs_union.c_str(), goldenunionfile.c_str());
 }
 
 template<typename T>