From 13c05f4da39054d6a3918614554f7f2a83770fcd Mon Sep 17 00:00:00 2001 From: Michael Beardsworth Date: Mon, 23 Dec 2019 08:50:29 -0800 Subject: [PATCH] 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 | 13 +++-- tests/BUILD | 2 + tests/prototest/test_include.golden | 57 ++++++++++++++++++++ tests/prototest/test_union_include.golden | 61 +++++++++++++++++++++ tests/test.cpp | 66 +++++++++++++++++++++++ 5 files changed, 195 insertions(+), 4 deletions(-) create mode 100644 tests/prototest/test_include.golden create mode 100644 tests/prototest/test_union_include.golden diff --git a/src/idl_gen_fbs.cpp b/src/idl_gen_fbs.cpp index e5f3723cb..ed86d0dec 100644 --- a/src/idl_gen_fbs.cpp +++ b/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"; diff --git a/tests/BUILD b/tests/BUILD index 8ebd8b66b..7433f2d4e 100644 --- a/tests/BUILD +++ b/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", diff --git a/tests/prototest/test_include.golden b/tests/prototest/test_include.golden new file mode 100644 index 000000000..16b92ed8e --- /dev/null +++ b/tests/prototest/test_include.golden @@ -0,0 +1,57 @@ +// 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; +} + diff --git a/tests/prototest/test_union_include.golden b/tests/prototest/test_union_include.golden new file mode 100644 index 000000000..1ce3ac303 --- /dev/null +++ b/tests/prototest/test_union_include.golden @@ -0,0 +1,61 @@ +// 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; +} + diff --git a/tests/test.cpp b/tests/test.cpp index 149831da5..80c18768e 100644 --- a/tests/test.cpp +++ b/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 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();