Added way to test two schemas for safe evolution.

Change-Id: I1dfc867e6df5932ab61dad431eb3cb02f15d04df
Tested: on Linux.
Bug: 30202327

docs/source/Compiler.md

@@ -108,5 +108,9 @@ Additional options:
     to the reflection/reflection.fbs schema. Loading this binary file is the
     basis for reflection functionality.
 
+-   `--conform FILE` : Specify a schema the following schemas should be
+    an evolution of. Gives errors if not. Useful to check if schema
+    modifications don't break schema evolution rules.
+
 NOTE: short-form options for generators are deprecated, use the long form
 whenever possible.

include/flatbuffers/idl.h

@@ -311,6 +311,14 @@ struct EnumDef : public Definition {
   Type underlying_type;
 };
 
+inline bool EqualByName(const Type &a, const Type &b) {
+  return a.base_type == b.base_type && a.element == b.element &&
+         (a.struct_def == b.struct_def ||
+          a.struct_def->name == b.struct_def->name) &&
+         (a.enum_def == b.enum_def ||
+          a.enum_def->name == b.enum_def->name);
+}
+
 struct RPCCall {
   std::string name;
   SymbolTable<Value> attributes;
@@ -473,6 +481,10 @@ class Parser : public ParserState {
   // See reflection/reflection.fbs
   void Serialize();
 
+  // Checks that the schema represented by this parser is a safe evolution
+  // of the schema provided. Returns non-empty error on any problems.
+  std::string ConformTo(const Parser &base);
+
   FLATBUFFERS_CHECKED_ERROR CheckBitsFit(int64_t val, size_t bits);
 
 private:

src/flatc.cpp

@@ -84,14 +84,14 @@ const Generator generators[] = {
     flatbuffers::CPPMakeRule },
 };
 
-const char *program_name = nullptr;
-flatbuffers::Parser *parser = nullptr;
+const char *g_program_name = nullptr;
+flatbuffers::Parser *g_parser = nullptr;
 
 static void Error(const std::string &err, bool usage, bool show_exe_name) {
-  if (show_exe_name) printf("%s: ", program_name);
+  if (show_exe_name) printf("%s: ", g_program_name);
   printf("%s\n", err.c_str());
   if (usage) {
-    printf("usage: %s [OPTION]... FILE... [-- FILE...]\n", program_name);
+    printf("usage: %s [OPTION]... FILE... [-- FILE...]\n", g_program_name);
     for (size_t i = 0; i < sizeof(generators) / sizeof(generators[0]); ++i)
       printf("  %-12s %s %s.\n",
              generators[i].generator_opt_long,
@@ -128,19 +128,34 @@ static void Error(const std::string &err, bool usage, bool show_exe_name) {
       "                     This may crash flatc given a mismatched schema.\n"
       "  --proto            Input is a .proto, translate to .fbs.\n"
       "  --schema           Serialize schemas instead of JSON (use with -b)\n"
+      "  --conform FILE     Specify a schema the following schemas should be\n"
+      "                     an evolution of. Gives errors if not.\n"
       "FILEs may be schemas, or JSON files (conforming to preceding schema)\n"
       "FILEs after the -- must be binary flatbuffer format files.\n"
       "Output files are named using the base file name of the input,\n"
       "and written to the current directory or the path given by -o.\n"
       "example: %s -c -b schema1.fbs schema2.fbs data.json\n",
-      program_name);
+      g_program_name);
   }
-  if (parser) delete parser;
+  if (g_parser) delete g_parser;
   exit(1);
 }
 
+static void ParseFile(flatbuffers::Parser &parser, const std::string &filename,
+                      const std::string &contents,
+                      std::vector<const char *> &include_directories) {
+  auto local_include_directory = flatbuffers::StripFileName(filename);
+  include_directories.push_back(local_include_directory.c_str());
+  include_directories.push_back(nullptr);
+  if (!parser.Parse(contents.c_str(), &include_directories[0],
+                     filename.c_str()))
+    Error(parser.error_, false, false);
+  include_directories.pop_back();
+  include_directories.pop_back();
+}
+
 int main(int argc, const char *argv[]) {
-  program_name = argv[0];
+  g_program_name = argv[0];
   flatbuffers::IDLOptions opts;
   std::string output_path;
   const size_t num_generators = sizeof(generators) / sizeof(generators[0]);
@@ -152,6 +167,7 @@ int main(int argc, const char *argv[]) {
   std::vector<std::string> filenames;
   std::vector<const char *> include_directories;
   size_t binary_files_from = std::numeric_limits<size_t>::max();
+  std::string conform_to_schema;
   for (int argi = 1; argi < argc; argi++) {
     std::string arg = argv[argi];
     if (arg[0] == '-') {
@@ -163,6 +179,9 @@ int main(int argc, const char *argv[]) {
       } else if(arg == "-I") {
         if (++argi >= argc) Error("missing path following" + arg, true);
         include_directories.push_back(argv[argi]);
+      } else if(arg == "--conform") {
+        if (++argi >= argc) Error("missing path following" + arg, true);
+        conform_to_schema = argv[argi];
       } else if(arg == "--strict-json") {
         opts.strict_json = true;
       } else if(arg == "--no-js-exports") {
@@ -230,12 +249,20 @@ int main(int argc, const char *argv[]) {
   if (opts.proto_mode) {
     if (any_generator)
       Error("cannot generate code directly from .proto files", true);
-  } else if (!any_generator) {
+  } else if (!any_generator && conform_to_schema.empty()) {
     Error("no options: specify at least one generator.", true);
   }
 
+  flatbuffers::Parser conform_parser;
+  if (!conform_to_schema.empty()) {
+    std::string contents;
+    if (!flatbuffers::LoadFile(conform_to_schema.c_str(), true, &contents))
+      Error("unable to load schema: " + conform_to_schema);
+    ParseFile(conform_parser, conform_to_schema, contents, include_directories);
+  }
+
   // Now process the files:
-  parser = new flatbuffers::Parser(opts);
+  g_parser = new flatbuffers::Parser(opts);
   for (auto file_it = filenames.begin();
             file_it != filenames.end();
           ++file_it) {
@@ -246,8 +273,8 @@ int main(int argc, const char *argv[]) {
       bool is_binary = static_cast<size_t>(file_it - filenames.begin()) >=
                        binary_files_from;
       if (is_binary) {
-        parser->builder_.Clear();
-        parser->builder_.PushFlatBuffer(
+        g_parser->builder_.Clear();
+        g_parser->builder_.PushFlatBuffer(
           reinterpret_cast<const uint8_t *>(contents.c_str()),
           contents.length());
         if (!raw_binary) {
@@ -256,17 +283,17 @@ int main(int argc, const char *argv[]) {
           // does not contain a file identifier.
           // We'd expect that typically any binary used as a file would have
           // such an identifier, so by default we require them to match.
-          if (!parser->file_identifier_.length()) {
+          if (!g_parser->file_identifier_.length()) {
             Error("current schema has no file_identifier: cannot test if \"" +
                  *file_it +
                  "\" matches the schema, use --raw-binary to read this file"
                  " anyway.");
           } else if (!flatbuffers::BufferHasIdentifier(contents.c_str(),
-                                             parser->file_identifier_.c_str())) {
+                         g_parser->file_identifier_.c_str())) {
             Error("binary \"" +
                  *file_it +
                  "\" does not have expected file_identifier \"" +
-                 parser->file_identifier_ +
+                 g_parser->file_identifier_ +
                  "\", use --raw-binary to read this file anyway.");
           }
         }
@@ -275,36 +302,34 @@ int main(int argc, const char *argv[]) {
         if (contents.length() != strlen(contents.c_str())) {
           Error("input file appears to be binary: " + *file_it, true);
         }
-        if (flatbuffers::GetExtension(*file_it) == "fbs") {
+        auto is_schema = flatbuffers::GetExtension(*file_it) == "fbs";
+        if (is_schema) {
           // If we're processing multiple schemas, make sure to start each
           // one from scratch. If it depends on previous schemas it must do
           // so explicitly using an include.
-          delete parser;
-          parser = new flatbuffers::Parser(opts);
+          delete g_parser;
+          g_parser = new flatbuffers::Parser(opts);
+        }
+        ParseFile(*g_parser, *file_it, contents, include_directories);
+        if (is_schema && !conform_to_schema.empty()) {
+          auto err = g_parser->ConformTo(conform_parser);
+          if (!err.empty()) Error("schemas don\'t conform: " + err);
         }
-        auto local_include_directory = flatbuffers::StripFileName(*file_it);
-        include_directories.push_back(local_include_directory.c_str());
-        include_directories.push_back(nullptr);
-        if (!parser->Parse(contents.c_str(), &include_directories[0],
-                          file_it->c_str()))
-          Error(parser->error_, false, false);
         if (schema_binary) {
-          parser->Serialize();
-          parser->file_extension_ = reflection::SchemaExtension();
+          g_parser->Serialize();
+          g_parser->file_extension_ = reflection::SchemaExtension();
         }
-        include_directories.pop_back();
-        include_directories.pop_back();
       }
 
       std::string filebase = flatbuffers::StripPath(
                                flatbuffers::StripExtension(*file_it));
 
       for (size_t i = 0; i < num_generators; ++i) {
-        parser->opts.lang = generators[i].lang;
+        g_parser->opts.lang = generators[i].lang;
         if (generator_enabled[i]) {
           if (!print_make_rules) {
             flatbuffers::EnsureDirExists(output_path);
-            if (!generators[i].generate(*parser, output_path, filebase)) {
+            if (!generators[i].generate(*g_parser, output_path, filebase)) {
               Error(std::string("Unable to generate ") +
                     generators[i].lang_name +
                     " for " +
@@ -312,7 +337,7 @@ int main(int argc, const char *argv[]) {
             }
           } else {
             std::string make_rule = generators[i].make_rule(
-                *parser, output_path, *file_it);
+                *g_parser, output_path, *file_it);
             if (!make_rule.empty())
               printf("%s\n", flatbuffers::WordWrap(
                   make_rule, 80, " ", " \\").c_str());
@@ -320,13 +345,13 @@ int main(int argc, const char *argv[]) {
         }
       }
 
-      if (opts.proto_mode) GenerateFBS(*parser, output_path, filebase);
+      if (opts.proto_mode) GenerateFBS(*g_parser, output_path, filebase);
 
       // We do not want to generate code for the definitions in this file
       // in any files coming up next.
-      parser->MarkGenerated();
+      g_parser->MarkGenerated();
   }
 
-  delete parser;
+  delete g_parser;
   return 0;
 }

src/idl_parser.cpp

@@ -2093,4 +2093,57 @@ flatbuffers::Offset<flatbuffers::Vector<flatbuffers::Offset<
   }
 }
 
+std::string Parser::ConformTo(const Parser &base) {
+  for (auto sit = structs_.vec.begin(); sit != structs_.vec.end(); ++sit) {
+    auto &struct_def = **sit;
+    auto qualified_name =
+        struct_def.defined_namespace->GetFullyQualifiedName(struct_def.name);
+    auto struct_def_base = base.structs_.Lookup(qualified_name);
+    if (!struct_def_base) continue;
+    for (auto fit = struct_def.fields.vec.begin();
+             fit != struct_def.fields.vec.end(); ++fit) {
+      auto &field = **fit;
+      auto field_base = struct_def_base->fields.Lookup(field.name);
+      if (field_base) {
+        if (field.value.offset != field_base->value.offset)
+          return "offsets differ for field: " + field.name;
+        if (field.value.constant != field_base->value.constant)
+          return "defaults differ for field: " + field.name;
+        if (!EqualByName(field.value.type, field_base->value.type))
+          return "types differ for field: " + field.name;
+      } else {
+        // Doesn't have to exist, deleting fields is fine.
+        // But we should check if there is a field that has the same offset
+        // but is incompatible (in the case of field renaming).
+        for (auto fbit = struct_def_base->fields.vec.begin();
+                 fbit != struct_def_base->fields.vec.end(); ++fbit) {
+          field_base = *fbit;
+          if (field.value.offset == field_base->value.offset) {
+            if (!EqualByName(field.value.type, field_base->value.type))
+              return "field renamed to different type: " + field.name;
+            break;
+          }
+        }
+      }
+    }
+  }
+  for (auto eit = enums_.vec.begin(); eit != enums_.vec.end(); ++eit) {
+    auto &enum_def = **eit;
+    auto qualified_name =
+        enum_def.defined_namespace->GetFullyQualifiedName(enum_def.name);
+    auto enum_def_base = base.enums_.Lookup(qualified_name);
+    if (!enum_def_base) continue;
+    for (auto evit = enum_def.vals.vec.begin();
+             evit != enum_def.vals.vec.end(); ++evit) {
+      auto &enum_val = **evit;
+      auto enum_val_base = enum_def_base->vals.Lookup(enum_val.name);
+      if (enum_val_base) {
+        if (enum_val.value != enum_val_base->value)
+          return "values differ for enum: " + enum_val.name;
+      }
+    }
+  }
+  return "";
+}
+
 }  // namespace flatbuffers

tests/test.cpp

@@ -1060,6 +1060,24 @@ void ParseUnionTest() {
                        "{ X:{ A:1 }, X_type: T }"), true);
 }
 
+void ConformTest() {
+  flatbuffers::Parser parser;
+  TEST_EQ(parser.Parse("table T { A:int; } enum E:byte { A }"), true);
+
+  auto test_conform = [&](const char *test, const char *expected_err) {
+    flatbuffers::Parser parser2;
+    TEST_EQ(parser2.Parse(test), true);
+    auto err = parser2.ConformTo(parser);
+    TEST_NOTNULL(strstr(err.c_str(), expected_err));
+  };
+
+  test_conform("table T { A:byte; }", "types differ for field");
+  test_conform("table T { B:int; A:int; }", "offsets differ for field");
+  test_conform("table T { A:int = 1; }", "defaults differ for field");
+  test_conform("table T { B:float; }", "field renamed to different type");
+  test_conform("enum E:byte { B, A }", "values differ for enum");
+}
+
 int main(int /*argc*/, const char * /*argv*/[]) {
   // Run our various test suites:
 
@@ -1091,6 +1109,7 @@ int main(int /*argc*/, const char * /*argv*/[]) {
   UnicodeInvalidSurrogatesTest();
   UnknownFieldsTest();
   ParseUnionTest();
+  ConformTest();
 
   if (!testing_fails) {
     TEST_OUTPUT_LINE("ALL TESTS PASSED");