inject no long for FBS generation to remove logs in flattests (#7926)

* inject no long for FBS generation to remove logs in flattests

* updated blaze rules

CMakeLists.txt

@@ -634,6 +634,7 @@ if(FLATBUFFERS_BUILD_TESTS)
   add_executable(flattests ${FlatBuffers_Tests_SRCS})
   target_link_libraries(flattests PRIVATE $<BUILD_INTERFACE:ProjectConfig>)
 
+  target_include_directories(flattests PUBLIC src)
   add_dependencies(flattests generated_code)
 
   if(FLATBUFFERS_CODE_SANITIZE)

include/flatbuffers/idl.h

@@ -1287,9 +1287,10 @@ extern bool GenerateSwift(const Parser &parser, const std::string &path,
 // Generate a schema file from the internal representation, useful after
 // parsing a .proto schema.
 extern std::string GenerateFBS(const Parser &parser,
-                               const std::string &file_name);
+                               const std::string &file_name,
+                               bool no_log);
 extern bool GenerateFBS(const Parser &parser, const std::string &path,
-                        const std::string &file_name);
+                        const std::string &file_name, bool no_log);
 
 // Generate a make rule for the generated TypeScript code.
 // See idl_gen_ts.cpp.

src/BUILD.bazel

@@ -5,20 +5,38 @@ package(
     default_visibility = ["//visibility:private"],
 )
 
+cc_library(
+    name = "code_generators",
+    srcs = ["code_generators.cpp"],
+    hdrs = [
+        "//:public_headers",
+    ],
+    strip_include_prefix = "/include",
+    visibility = ["//:__subpackages__"],
+)
+
+cc_library(
+    name = "generate_fbs",
+    srcs = ["idl_gen_fbs.cpp"],
+    hdrs = ["idl_gen_fbs.h"],
+    strip_include_prefix = "/src",
+    visibility = ["//:__subpackages__"],
+    deps = [":code_generators"],
+)
+
 # Public flatc library to compile flatbuffer files at runtime.
 cc_library(
     name = "flatbuffers",
     srcs = [
-        "code_generators.cpp",
-        "idl_gen_fbs.cpp",
-        "idl_gen_fbs.h",
         "idl_gen_text.cpp",
         "idl_gen_text.h",
         "idl_parser.cpp",
         "reflection.cpp",
         "util.cpp",
     ],
-    hdrs = ["//:public_headers"],
+    hdrs = [
+        "//:public_headers",
+    ],
     linkopts = select({
         # TODO: Bazel uses `clang` instead of `clang++` to link
         # C++ code on BSD. Temporarily adding these linker flags while
@@ -29,7 +47,11 @@ cc_library(
         "//conditions:default": [],
     }),
     strip_include_prefix = "/include",
-    visibility = ["//:__pkg__"],
+    visibility = ["//:__subpackages__"],
+    deps = [
+        ":code_generators",
+        ":generate_fbs",
+    ],
 )
 
 # Public flatc compiler library.

src/idl_gen_fbs.cpp

@@ -129,34 +129,43 @@ static bool HasGapInProtoId(const std::vector<FieldDef *> &fields) {
 }
 
 static bool ProtobufIdSanityCheck(const StructDef &struct_def,
-                                  IDLOptions::ProtoIdGapAction gap_action) {
+                                  IDLOptions::ProtoIdGapAction gap_action,
+                                  bool no_log = false) {
   const auto &fields = struct_def.fields.vec;
   if (HasNonPositiveFieldId(fields)) {
     // TODO: Use LogCompilerWarn
-    fprintf(stderr, "Field id in struct %s has a non positive number value\n",
+    if (!no_log) {
-            struct_def.name.c_str());
+      fprintf(stderr, "Field id in struct %s has a non positive number value\n",
+              struct_def.name.c_str());
+    }
     return false;
   }
 
   if (HasTwiceUsedId(fields)) {
     // TODO: Use LogCompilerWarn
-    fprintf(stderr, "Fields in struct %s have used an id twice\n",
+    if (!no_log) {
-            struct_def.name.c_str());
+      fprintf(stderr, "Fields in struct %s have used an id twice\n",
+              struct_def.name.c_str());
+    }
     return false;
   }
 
   if (HasFieldIdFromReservedIds(fields, struct_def.reserved_ids)) {
     // TODO: Use LogCompilerWarn
-    fprintf(stderr, "Fields in struct %s use id from reserved ids\n",
+    if (!no_log) {
-            struct_def.name.c_str());
+      fprintf(stderr, "Fields in struct %s use id from reserved ids\n",
+              struct_def.name.c_str());
+    }
     return false;
   }
 
   if (gap_action != IDLOptions::ProtoIdGapAction::NO_OP) {
     if (HasGapInProtoId(fields)) {
       // TODO: Use LogCompilerWarn
-      fprintf(stderr, "Fields in struct %s have gap between ids\n",
+      if (!no_log) {
-              struct_def.name.c_str());
+        fprintf(stderr, "Fields in struct %s have gap between ids\n",
+                struct_def.name.c_str());
+      }
       if (gap_action == IDLOptions::ProtoIdGapAction::ERROR) { return false; }
     }
   }
@@ -174,7 +183,8 @@ struct ProtobufToFbsIdMap {
 };
 
 static ProtobufToFbsIdMap MapProtoIdsToFieldsId(
-    const StructDef &struct_def, IDLOptions::ProtoIdGapAction gap_action) {
+    const StructDef &struct_def, IDLOptions::ProtoIdGapAction gap_action,
+    bool no_log) {
   const auto &fields = struct_def.fields.vec;
 
   if (!HasFieldWithId(fields)) {
@@ -183,7 +193,7 @@ static ProtobufToFbsIdMap MapProtoIdsToFieldsId(
     return result;
   }
 
-  if (!ProtobufIdSanityCheck(struct_def, gap_action)) { return {}; }
+  if (!ProtobufIdSanityCheck(struct_def, gap_action, no_log)) { return {}; }
 
   static constexpr int UNION_ID = -1;
   using ProtoIdFieldNamePair = std::pair<int, std::string>;
@@ -203,8 +213,10 @@ static ProtobufToFbsIdMap MapProtoIdsToFieldsId(
       }
     } else {
       // TODO: Use LogCompilerWarn
-      fprintf(stderr, "Fields id in struct %s is missing\n",
+      if (!no_log) {
-              struct_def.name.c_str());
+        fprintf(stderr, "Fields id in struct %s is missing\n",
+                struct_def.name.c_str());
+      }
       return {};
     }
   }
@@ -240,7 +252,8 @@ static void GenNameSpace(const Namespace &name_space, std::string *_schema,
 }
 
 // Generate a flatbuffer schema from the Parser's internal representation.
-std::string GenerateFBS(const Parser &parser, const std::string &file_name) {
+std::string GenerateFBS(const Parser &parser, const std::string &file_name,
+                        bool no_log = false) {
   // Proto namespaces may clash with table names, escape the ones that were
   // generated from a table:
   for (auto it = parser.namespaces_.begin(); it != parser.namespaces_.end();
@@ -315,8 +328,8 @@ 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;
-    const auto proto_fbs_ids =
+    const auto proto_fbs_ids = MapProtoIdsToFieldsId(
-        MapProtoIdsToFieldsId(struct_def, parser.opts.proto_id_gap_action);
+        struct_def, parser.opts.proto_id_gap_action, no_log);
     if (!proto_fbs_ids.successful) { return {}; }
 
     if (parser.opts.include_dependence_headers && struct_def.generated) {
@@ -362,13 +375,15 @@ std::string GenerateFBS(const Parser &parser, const std::string &file_name) {
 }
 
 bool GenerateFBS(const Parser &parser, const std::string &path,
-                 const std::string &file_name) {
+                 const std::string &file_name, bool no_log = false) {
-  const std::string fbs = GenerateFBS(parser, file_name);
+  const std::string fbs = GenerateFBS(parser, file_name, no_log);
   if (fbs.empty()) { return false; }
   // TODO: Use LogCompilerWarn
-  fprintf(stderr,
+  if (!no_log) {
-          "When you use --proto, that you should check for conformity "
+    fprintf(stderr,
-          "yourself, using the existing --conform");
+            "When you use --proto, that you should check for conformity "
+            "yourself, using the existing --conform");
+  }
   return SaveFile((path + file_name + ".fbs").c_str(), fbs, false);
 }
 
@@ -376,9 +391,11 @@ namespace {
 
 class FBSCodeGenerator : public CodeGenerator {
  public:
+  explicit FBSCodeGenerator(const bool no_log) : no_log_(no_log) {}
+
   Status GenerateCode(const Parser &parser, const std::string &path,
                       const std::string &filename) override {
-    if (!GenerateFBS(parser, path, filename)) { return Status::ERROR; }
+    if (!GenerateFBS(parser, path, filename, no_log_)) { return Status::ERROR; }
     return Status::OK;
   }
 
@@ -424,12 +441,15 @@ class FBSCodeGenerator : public CodeGenerator {
   IDLOptions::Language Language() const override { return IDLOptions::kProto; }
 
   std::string LanguageName() const override { return "proto"; }
+
+ protected:
+  const bool no_log_;
 };
 
 }  // namespace
 
-std::unique_ptr<CodeGenerator> NewFBSCodeGenerator() {
+std::unique_ptr<CodeGenerator> NewFBSCodeGenerator(const bool no_log) {
-  return std::unique_ptr<FBSCodeGenerator>(new FBSCodeGenerator());
+  return std::unique_ptr<FBSCodeGenerator>(new FBSCodeGenerator(no_log));
 }
 
 }  // namespace flatbuffers

src/idl_gen_fbs.h

@@ -21,7 +21,7 @@
 
 namespace flatbuffers {
 
-std::unique_ptr<CodeGenerator> NewFBSCodeGenerator();
+std::unique_ptr<CodeGenerator> NewFBSCodeGenerator(bool no_log = false);
 
 }  // namespace flatbuffers
 

tests/BUILD.bazel

@@ -113,6 +113,7 @@ cc_test(
         ":monster_test_cc_fbs",
         ":native_type_test_cc_fbs",
         "//:flatbuffers",
+        "//src:generate_fbs",
     ],
 )
 

tests/proto_test.cpp

@@ -1,5 +1,7 @@
 #include "proto_test.h"
 
+#include "flatbuffers/code_generator.h"
+#include "idl_gen_fbs.h"
 #include "test_assert.h"
 
 namespace flatbuffers {
@@ -15,7 +17,7 @@ void RunTest(const flatbuffers::IDLOptions &opts, const std::string &proto_path,
   TEST_EQ(parser.Parse(proto_file.c_str(), include_directories), true);
 
   // Generate fbs.
-  auto fbs = flatbuffers::GenerateFBS(parser, "test");
+  auto fbs = flatbuffers::GenerateFBS(parser, "test", true);
 
   // Ensure generated file is parsable.
   flatbuffers::Parser parser2;
@@ -25,7 +27,7 @@ void RunTest(const flatbuffers::IDLOptions &opts, const std::string &proto_path,
     flatbuffers::Parser import_parser(opts);
     TEST_EQ(import_parser.Parse(import_proto_file.c_str(), include_directories),
             true);
-    auto import_fbs = flatbuffers::GenerateFBS(import_parser, "test");
+    auto import_fbs = flatbuffers::GenerateFBS(import_parser, "test", true);
     // Since `imported.fbs` isn't in the filesystem AbsolutePath can't figure it
     // out by itself. We manually construct it so Parser works.
     std::string imported_fbs = flatbuffers::PosixPath(
@@ -222,6 +224,8 @@ void ParseCorruptedProto(const std::string &proto_path) {
 
   std::string proto_file;
 
+  std::unique_ptr<CodeGenerator> fbs_generator = NewFBSCodeGenerator(true);
+
   // Parse proto with non positive id.
   {
     flatbuffers::Parser parser(opts);
@@ -230,8 +234,8 @@ void ParseCorruptedProto(const std::string &proto_path) {
                               false, &proto_file),
         true);
     TEST_EQ(parser.Parse(proto_file.c_str(), include_directories), true);
-    auto fbs = flatbuffers::GenerateFBS(parser, "test");
+    TEST_NE(fbs_generator->GenerateCode(parser, "temp.fbs", "test"),
-    TEST_EQ(fbs.empty(), true);
+            CodeGenerator::Status::OK);
   }
 
   // Parse proto with twice id.
@@ -241,8 +245,8 @@ void ParseCorruptedProto(const std::string &proto_path) {
                                   false, &proto_file),
             true);
     TEST_EQ(parser.Parse(proto_file.c_str(), include_directories), true);
-    auto fbs = flatbuffers::GenerateFBS(parser, "test");
+    TEST_NE(fbs_generator->GenerateCode(parser, "temp.fbs", "test"),
-    TEST_EQ(fbs.empty(), true);
+            CodeGenerator::Status::OK);
   }
 
   // Parse proto with using reserved id.
@@ -252,8 +256,8 @@ void ParseCorruptedProto(const std::string &proto_path) {
                                   false, &proto_file),
             true);
     TEST_EQ(parser.Parse(proto_file.c_str(), include_directories), true);
-    auto fbs = flatbuffers::GenerateFBS(parser, "test");
+    TEST_NE(fbs_generator->GenerateCode(parser, "temp.fbs", "test"),
-    TEST_EQ(fbs.empty(), true);
+            CodeGenerator::Status::OK);
   }
 
   // Parse proto with error on gap.
@@ -264,8 +268,9 @@ void ParseCorruptedProto(const std::string &proto_path) {
                                   &proto_file),
             true);
     TEST_EQ(parser.Parse(proto_file.c_str(), include_directories), true);
-    auto fbs = flatbuffers::GenerateFBS(parser, "test");
+
-    TEST_EQ(fbs.empty(), true);
+    TEST_NE(fbs_generator->GenerateCode(parser, "temp.fbs", "test"),
+            CodeGenerator::Status::OK);
   }
 }
 

tests/test_assert.h

@@ -17,6 +17,7 @@
 #endif
 
 #define TEST_EQ(exp, val) TestEq(exp, val, "'" #exp "' != '" #val "'", __FILE__, __LINE__, "")
+#define TEST_NE(exp, val) TestNe(exp, val, "'" #exp "' == '" #val "'", __FILE__, __LINE__, "")
 #define TEST_ASSERT(val)  TestEq(true, !!(val), "'" "true" "' != '" #val "'", __FILE__, __LINE__, "")
 #define TEST_NOTNULL(val) TestEq(true, (val) != nullptr, "'" "nullptr" "' == '" #val "'", __FILE__, __LINE__, "")
 #define TEST_EQ_STR(exp, val) TestEqStr(exp, val, "'" #exp "' != '" #val "'", __FILE__, __LINE__, "")
@@ -106,4 +107,24 @@ inline void TestEq<std::string, std::string>(std::string expval,
   }
 }
 
+template<typename T, typename U>
+void TestNe(T expval, U val, const char *exp, const char *file, int line,
+            const char *func) {
+  if (static_cast<U>(expval) == val) {
+    TestFail(flatbuffers::NumToString(scalar_as_underlying(expval)).c_str(),
+             flatbuffers::NumToString(scalar_as_underlying(val)).c_str(), exp,
+             file, line, func);
+  }
+}
+
+template<>
+inline void TestNe<std::string, std::string>(std::string expval,
+                                             std::string val, const char *exp,
+                                             const char *file, int line,
+                                             const char *func) {
+  if (expval == val) {
+    TestFail(expval.c_str(), val.c_str(), exp, file, line, func);
+  }
+}
+
 #endif  // !TEST_ASSERT_H