JSON Parser allows union type fields to come after unions.

This is useful because many JSON generators will sort the fields,
cause X_type to follow X.

Change-Id: I00ef3ac05418224fc05aee93e6b3b3597e73ffe3
Tested: on Linux.
Bug: 29221752

include/flatbuffers/idl.h

@@ -359,6 +359,19 @@ struct IDLOptions {
       lang(IDLOptions::kJava) {}
 };
 
+// This encapsulates where the parser is in the current source file.
+struct ParserState {
+  ParserState() : cursor_(nullptr), line_(1), token_(-1) {}
+
+ protected:
+  const char *cursor_;
+  int line_;  // the current line being parsed
+  int token_;
+
+  std::string attribute_;
+  std::vector<std::string> doc_comment_;
+};
+
 // A way to make error propagation less error prone by requiring values to be
 // checked.
 // Once you create a value of this type you must either:
@@ -400,14 +413,12 @@ class CheckedError {
 #define FLATBUFFERS_CHECKED_ERROR CheckedError
 #endif
 
-class Parser {
+class Parser : public ParserState {
  public:
   explicit Parser(const IDLOptions &options = IDLOptions())
     : root_struct_def_(nullptr),
       opts(options),
       source_(nullptr),
-      cursor_(nullptr),
-      line_(1),
       anonymous_counter(0) {
     // Just in case none are declared:
     namespaces_.push_back(new Namespace());
@@ -478,7 +489,8 @@ private:
                                      FieldDef **dest);
   FLATBUFFERS_CHECKED_ERROR ParseField(StructDef &struct_def);
   FLATBUFFERS_CHECKED_ERROR ParseAnyValue(Value &val, FieldDef *field,
-                                          size_t parent_fieldn);
+                                          size_t parent_fieldn,
+                                          const StructDef *parent_struct_def);
   FLATBUFFERS_CHECKED_ERROR ParseTable(const StructDef &struct_def,
                                        std::string *value, uoffset_t *ovalue);
   void SerializeStruct(const StructDef &struct_def, const Value &val);
@@ -538,13 +550,9 @@ private:
   IDLOptions opts;
 
  private:
-  const char *source_, *cursor_;
-  int line_;  // the current line being parsed
-  int token_;
-  std::string file_being_parsed_;
+  const char *source_;
 
-  std::string attribute_;
-  std::vector<std::string> doc_comment_;
+  std::string file_being_parsed_;
 
   std::vector<std::pair<Value, FieldDef *>> field_stack_;
 

include/flatbuffers/reflection.h

@@ -335,6 +335,8 @@ template<typename T, typename U> pointer_inside_vector<T, U> piv(T *ptr,
   return pointer_inside_vector<T, U>(ptr, vec);
 }
 
+inline const char *UnionTypeFieldSuffix() { return "_type"; }
+
 // Helper to figure out the actual table type a union refers to.
 inline const reflection::Object &GetUnionType(
     const reflection::Schema &schema, const reflection::Object &parent,
@@ -342,7 +344,7 @@ inline const reflection::Object &GetUnionType(
   auto enumdef = schema.enums()->Get(unionfield.type()->index());
   // TODO: this is clumsy and slow, but no other way to find it?
   auto type_field = parent.fields()->LookupByKey(
-            (unionfield.name()->str() + "_type").c_str());
+            (unionfield.name()->str() + UnionTypeFieldSuffix()).c_str());
   assert(type_field);
   auto union_type = GetFieldI<uint8_t>(table, *type_field);
   auto enumval = enumdef->values()->LookupByKey(union_type);

src/idl_gen_cpp.cpp

@@ -604,8 +604,8 @@ class CppGenerator : public BaseGenerator {
         switch (field.value.type.base_type) {
           case BASE_TYPE_UNION:
             code += prefix + "Verify" + field.value.type.enum_def->name;
-            code +=
-                "(verifier, " + field.name + "(), " + field.name + "_type())";
+            code += "(verifier, " + field.name + "(), " + field.name +
+                    UnionTypeFieldSuffix() + "())";
             break;
           case BASE_TYPE_STRUCT:
             if (!field.value.type.struct_def->fixed) {

src/idl_parser.cpp

@@ -583,9 +583,9 @@ CheckedError Parser::ParseField(StructDef &struct_def) {
   FieldDef *typefield = nullptr;
   if (type.base_type == BASE_TYPE_UNION) {
     // For union fields, add a second auto-generated field to hold the type,
-    // with _type appended as the name.
-    ECHECK(AddField(struct_def, name + "_type", type.enum_def->underlying_type,
-                    &typefield));
+    // with a special suffix.
+    ECHECK(AddField(struct_def, name + UnionTypeFieldSuffix(),
+                    type.enum_def->underlying_type, &typefield));
   }
 
   FieldDef *field;
@@ -686,17 +686,45 @@ CheckedError Parser::ParseField(StructDef &struct_def) {
 }
 
 CheckedError Parser::ParseAnyValue(Value &val, FieldDef *field,
-                                   size_t parent_fieldn) {
+                                   size_t parent_fieldn,
+                                   const StructDef *parent_struct_def) {
   switch (val.type.base_type) {
     case BASE_TYPE_UNION: {
       assert(field);
+      std::string constant;
       if (!parent_fieldn ||
-          field_stack_.back().second->value.type.base_type != BASE_TYPE_UTYPE)
-        return Error("missing type field before this union value: " +
-                     field->name);
+          field_stack_.back().second->value.type.base_type != BASE_TYPE_UTYPE) {
+        // We haven't seen the type field yet. Sadly a lot of JSON writers
+        // output these in alphabetical order, meaning it comes after this
+        // value. So we scan past the value to find it, then come back here.
+        auto type_name = field->name + UnionTypeFieldSuffix();
+        assert(parent_struct_def);
+        auto type_field = parent_struct_def->fields.Lookup(type_name);
+        assert(type_field);  // Guaranteed by ParseField().
+        // Remember where we are in the source file, so we can come back here.
+        auto backup = *static_cast<ParserState *>(this);
+        ECHECK(SkipAnyJsonValue());  // The table.
+        EXPECT(',');
+        auto next_name = attribute_;
+        if (Is(kTokenStringConstant)) {
+          NEXT();
+        } else {
+          EXPECT(kTokenIdentifier);
+        }
+        if (next_name != type_name)
+          return Error("missing type field after this union value: " +
+                       type_name);
+        EXPECT(':');
+        Value type_val = type_field->value;
+        ECHECK(ParseAnyValue(type_val, type_field, 0, nullptr));
+        constant = type_val.constant;
+        // Got the information we needed, now rewind:
+        *static_cast<ParserState *>(this) = backup;
+      } else {
+        constant = field_stack_.back().first.constant;
+      }
       uint8_t enum_idx;
-      ECHECK(atot(field_stack_.back().first.constant.c_str(), *this,
-                  &enum_idx));
+      ECHECK(atot(constant.c_str(), *this, &enum_idx));
       auto enum_val = val.type.enum_def->ReverseLookup(enum_idx);
       if (!enum_val) return Error("illegal type id for: " + field->name);
       ECHECK(ParseTable(*enum_val->struct_def, &val.constant, nullptr));
@@ -771,7 +799,7 @@ CheckedError Parser::ParseTable(const StructDef &struct_def, std::string *value,
         NEXT(); // Ignore this field.
       } else {
         Value val = field->value;
-        ECHECK(ParseAnyValue(val, field, fieldn));
+        ECHECK(ParseAnyValue(val, field, fieldn, &struct_def));
         size_t i = field_stack_.size();
         // Hardcoded insertion-sort with error-check.
         // If fields are specified in order, then this loop exits immediately.
@@ -870,7 +898,7 @@ CheckedError Parser::ParseVector(const Type &type, uoffset_t *ovalue) {
     if ((!opts.strict_json || !count) && Is(']')) { NEXT(); break; }
     Value val;
     val.type = type;
-    ECHECK(ParseAnyValue(val, nullptr, 0));
+    ECHECK(ParseAnyValue(val, nullptr, 0, nullptr));
     field_stack_.push_back(std::make_pair(val, nullptr));
     count++;
     if (Is(']')) { NEXT(); break; }
@@ -1324,7 +1352,8 @@ CheckedError Parser::ParseDecl() {
     }
   }
 
-  ECHECK(CheckClash(fields, struct_def, "_type", BASE_TYPE_UNION));
+  ECHECK(CheckClash(fields, struct_def, UnionTypeFieldSuffix(),
+                    BASE_TYPE_UNION));
   ECHECK(CheckClash(fields, struct_def, "Type", BASE_TYPE_UNION));
   ECHECK(CheckClash(fields, struct_def, "_length", BASE_TYPE_VECTOR));
   ECHECK(CheckClash(fields, struct_def, "Length", BASE_TYPE_VECTOR));

tests/test.cpp

@@ -783,7 +783,7 @@ void ErrorTest() {
   TestError("table X { Y:int; Y:int; }", "field already");
   TestError("struct X { Y:string; }", "only scalar");
   TestError("struct X { Y:int (deprecated); }", "deprecate");
-  TestError("union Z { X } table X { Y:Z; } root_type X; { Y: {",
+  TestError("union Z { X } table X { Y:Z; } root_type X; { Y: {}, A:1 }",
             "missing type field");
   TestError("union Z { X } table X { Y:Z; } root_type X; { Y_type: 99, Y: {",
             "type id");
@@ -951,6 +951,16 @@ void UnknownFieldsTest() {
   TEST_EQ(jsongen == "{str: \"test\",i: 10}", true);
 }
 
+void ParseUnionTest() {
+  // Unions must be parseable with the type field following the object.
+  flatbuffers::Parser parser;
+  TEST_EQ(parser.Parse("table T { A:int; }"
+                       "union U { T }"
+                       "table V { X:U; }"
+                       "root_type V;"
+                       "{ X:{ A:1 }, X_type: T }"), true);
+}
+
 int main(int /*argc*/, const char * /*argv*/[]) {
   // Run our various test suites:
 
@@ -979,6 +989,7 @@ int main(int /*argc*/, const char * /*argv*/[]) {
   UnicodeSurrogatesTest();
   UnicodeInvalidSurrogatesTest();
   UnknownFieldsTest();
+  ParseUnionTest();
 
   if (!testing_fails) {
     TEST_OUTPUT_LINE("ALL TESTS PASSED");