Improve import handling for proto conversion (#5673)

* Keep include prefix when converting from proto.

This change preserves the include prefix when generating flatbuffers
from proto (with FBS_GEN_INCLUDES) defined.

* Improve handling of imports in proto conversion.

Previously, there was no runtime flag to make proto->fbs conversion keep
the import structure of a collection of files. This change makes proto
conversion respect the --no-gen-includes flag and skip the output of
"generated" symbols.

src/idl_gen_fbs.cpp

@@ -69,19 +69,22 @@ std::string GenerateFBS(const Parser &parser, const std::string &file_name) {
   schema += "// Generated from " + file_name + ".proto\n\n";
   if (parser.opts.include_dependence_headers) {
     // clang-format off
-    #ifdef FBS_GEN_INCLUDES  // TODO: currently all in one file.
     int num_includes = 0;
     for (auto it = parser.included_files_.begin();
          it != parser.included_files_.end(); ++it) {
       if (it->second.empty())
         continue;
-      auto basename = flatbuffers::StripPath(
-                        flatbuffers::StripExtension(it->second));
+      std::string basename;
+      if(parser.opts.keep_include_path) {
+        basename = flatbuffers::StripPath(
+                flatbuffers::StripExtension(it->second));
+      } else {
+        basename = flatbuffers::StripExtension(it->second);
+      }
       schema += "include \"" + basename + ".fbs\";\n";
       num_includes++;
     }
     if (num_includes) schema += "\n";
-    #endif
     // clang-format on
   }
   // Generate code for all the enum declarations.
@@ -89,6 +92,7 @@ std::string GenerateFBS(const Parser &parser, const std::string &file_name) {
   for (auto enum_def_it = parser.enums_.vec.begin();
        enum_def_it != parser.enums_.vec.end(); ++enum_def_it) {
     EnumDef &enum_def = **enum_def_it;
+    if (parser.opts.include_dependence_headers && enum_def.generated) { continue; }
     GenNameSpace(*enum_def.defined_namespace, &schema, &last_namespace);
     GenComment(enum_def.doc_comment, &schema, nullptr);
     if (enum_def.is_union)
@@ -110,6 +114,7 @@ std::string GenerateFBS(const Parser &parser, const std::string &file_name) {
   for (auto it = parser.structs_.vec.begin(); it != parser.structs_.vec.end();
        ++it) {
     StructDef &struct_def = **it;
+    if (parser.opts.include_dependence_headers && struct_def.generated) { continue; }
     GenNameSpace(*struct_def.defined_namespace, &schema, &last_namespace);
     GenComment(struct_def.doc_comment, &schema, nullptr);
     schema += "table " + struct_def.name + " {\n";

tests/BUILD

@@ -44,7 +44,9 @@ cc_test(
         ":prototest/imported.proto",
         ":prototest/test.golden",
         ":prototest/test.proto",
+        ":prototest/test_include.golden",
         ":prototest/test_union.golden",
+        ":prototest/test_union_include.golden",
         ":unicode_test.json",
         ":union_vector/union_vector.fbs",
         ":union_vector/union_vector.json",

tests/prototest/test_include.golden

+// Generated from test.proto
+
+include "imported.fbs";
+
+namespace proto.test;
+
+/// Enum doc comment.
+enum ProtoEnum : int {
+  NUL = 0,
+  FOO = 1,
+  /// Enum 2nd value doc comment misaligned.
+  BAR = 5,
+}
+
+/// 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_.Anonymous0;
+}
+
+namespace proto.test.ProtoMessage_;
+
+table OtherMessage {
+  a:double;
+  /// doc comment for b.
+  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_union_include.golden

+// Generated from test.proto
+
+include "imported.fbs";
+
+namespace proto.test;
+
+/// Enum doc comment.
+enum ProtoEnum : int {
+  NUL = 0,
+  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;
+
+/// 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

@@ -1127,6 +1127,71 @@ void ParseProtoTest() {
   TEST_EQ_STR(fbs_union.c_str(), goldenunionfile.c_str());
 }
 
+// Parse a .proto schema, output as .fbs
+void ParseProtoTestWithIncludes() {
+  // load the .proto and the golden file from disk
+  std::string protofile;
+  std::string goldenfile;
+  std::string goldenunionfile;
+  std::string importprotofile;
+  TEST_EQ(
+      flatbuffers::LoadFile((test_data_path + "prototest/test.proto").c_str(),
+                            false, &protofile),
+      true);
+  TEST_EQ(
+      flatbuffers::LoadFile((test_data_path + "prototest/imported.proto").c_str(),
+                            false, &importprotofile),
+      true);
+  TEST_EQ(
+      flatbuffers::LoadFile((test_data_path + "prototest/test_include.golden").c_str(),
+                            false, &goldenfile),
+      true);
+  TEST_EQ(flatbuffers::LoadFile(
+              (test_data_path + "prototest/test_union_include.golden").c_str(), false,
+              &goldenunionfile),
+          true);
+
+  flatbuffers::IDLOptions opts;
+  opts.include_dependence_headers = true;
+  opts.proto_mode = true;
+
+  // Parse proto.
+  flatbuffers::Parser parser(opts);
+  auto protopath = test_data_path + "prototest/";
+  const char *include_directories[] = { protopath.c_str(), nullptr };
+  TEST_EQ(parser.Parse(protofile.c_str(), include_directories), true);
+
+  // Generate fbs.
+  auto fbs = flatbuffers::GenerateFBS(parser, "test");
+
+  // Generate fbs from import.proto
+  flatbuffers::Parser import_parser(opts);
+  TEST_EQ(import_parser.Parse(importprotofile.c_str(), include_directories), true);
+  auto import_fbs = flatbuffers::GenerateFBS(import_parser, "test");
+
+  // Ensure generated file is parsable.
+  flatbuffers::Parser parser2;
+  TEST_EQ(parser2.Parse(import_fbs.c_str(), include_directories, "imported.fbs"), true);
+  TEST_EQ(parser2.Parse(fbs.c_str(), nullptr), true);
+  //printf("Golden\n%s\n", goldenfile.c_str());
+  printf("FBS\n%s\n", fbs.c_str());
+  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(import_fbs.c_str(), nullptr, "imported.fbs"), true);
+  TEST_EQ(parser4.Parse(fbs_union.c_str(), nullptr), true);
+  TEST_EQ_STR(fbs_union.c_str(), goldenunionfile.c_str());
+}
+
 template<typename T>
 void CompareTableFieldValue(flatbuffers::Table *table,
                             flatbuffers::voffset_t voffset, T val) {
@@ -3136,6 +3201,7 @@ int FlatBufferTests() {
     FixedLengthArrayJsonTest(true);
     ReflectionTest(flatbuf.data(), flatbuf.size());
     ParseProtoTest();
+    ParseProtoTestWithIncludes();
     EvolutionTest();
     UnionVectorTest();
     LoadVerifyBinaryTest();