From e58c182443ac53c317c12fc8b4017d7816576a3d Mon Sep 17 00:00:00 2001
From: Austin Schuh <AustinSchuh@users.noreply.github.com>
Date: Thu, 19 Nov 2020 17:16:45 -0800
Subject: [PATCH] Add --require-explicit-ids to require explicit ids (#6277)

* Add --require-explicit-ids to require explicit ids

We just got bit by a well intentioned developer forgetting that field
order by default is the field index.  3 people missed it in review.

I'm looking at ways to make it harder to mess up.  We are requesting
that developers explicitly id all fields in tables.  Automatic (opt in
for others) enforcement of this will help the effort succeed.  This
patch adds a command line flag which lets the user require ids on all
fields in tables.

* Added docs to Compiler.md as well
---
 docs/source/Compiler.md   |  2 ++
 include/flatbuffers/idl.h |  4 ++++
 src/flatc.cpp             |  3 +++
 src/idl_parser.cpp        | 15 +++++++++++----
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/docs/source/Compiler.md b/docs/source/Compiler.md
index 4636428c7..973c4116e 100644
--- a/docs/source/Compiler.md
+++ b/docs/source/Compiler.md
@@ -218,6 +218,8 @@ Additional options:
 
 -   `--root-type T` : Select or override the default root_type.
 
+-   `--require-explicit-ids` : When parsing schemas, require explicit ids (id: x).
+
 -   `--force-defaults` : Emit default values in binary output from JSON.
 
 -   `--force-empty` : When serializing from object API representation, force
diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h
index a28745f0e..e073d4162 100644
--- a/include/flatbuffers/idl.h
+++ b/include/flatbuffers/idl.h
@@ -601,6 +601,9 @@ struct IDLOptions {
 
   MiniReflect mini_reflect;
 
+  // If set, require all fields in a table to be explicitly numbered.
+  bool require_explicit_ids;
+
   // The corresponding language bit will be set if a language is included
   // for code generation.
   unsigned long lang_to_generate;
@@ -661,6 +664,7 @@ struct IDLOptions {
         filename_extension(),
         lang(IDLOptions::kJava),
         mini_reflect(IDLOptions::kNone),
+        require_explicit_ids(false),
         lang_to_generate(0),
         set_empty_strings_to_null(true),
         set_empty_vectors_to_null(true) {}
diff --git a/src/flatc.cpp b/src/flatc.cpp
index 4a9df5f84..1233ff937 100644
--- a/src/flatc.cpp
+++ b/src/flatc.cpp
@@ -163,6 +163,7 @@ std::string FlatCompiler::GetUsageString(const char *program_name) const {
     "  --reflect-types        Add minimal type reflection to code generation.\n"
     "  --reflect-names        Add minimal type/name reflection.\n"
     "  --root-type T          Select or override the default root_type\n"
+    "  --require-explicit-ids When parsing schemas, require explicit ids (id: x).\n"
     "  --force-defaults       Emit default values in binary output from JSON\n"
     "  --force-empty          When serializing from object API representation,\n"
     "                         force strings and vectors to empty rather than null.\n"
@@ -345,6 +346,8 @@ int FlatCompiler::Compile(int argc, const char **argv) {
         opts.mini_reflect = IDLOptions::kTypes;
       } else if (arg == "--reflect-names") {
         opts.mini_reflect = IDLOptions::kTypesAndNames;
+      } else if (arg == "--require-explicit-ids") {
+        opts.require_explicit_ids = true;
       } else if (arg == "--root-type") {
         if (++argi >= argc) Error("missing type following: " + arg, true);
         opts.root_type = argv[argi];
diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp
index a87fbce4c..3f0407734 100644
--- a/src/idl_parser.cpp
+++ b/src/idl_parser.cpp
@@ -2379,11 +2379,18 @@ CheckedError Parser::ParseDecl() {
       if ((*it)->attributes.Lookup("id")) num_id_fields++;
     }
     // If any fields have ids..
-    if (num_id_fields) {
+    if (num_id_fields || opts.require_explicit_ids) {
       // Then all fields must have them.
-      if (num_id_fields != fields.size())
-        return Error(
-            "either all fields or no fields must have an 'id' attribute");
+      if (num_id_fields != fields.size()) {
+        if (opts.require_explicit_ids) {
+          return Error(
+              "all fields must have an 'id' attribute when "
+              "--require-explicit-ids is used");
+        } else {
+          return Error(
+              "either all fields or no fields must have an 'id' attribute");
+        }
+      }
       // Simply sort by id, then the fields are the same as if no ids had
       // been specified.
       std::sort(fields.begin(), fields.end(), compareFieldDefs);