Add support for metadata attributes for enum values (#7567) (#7568)

* Add support for metadata attributes for enum values (#7567)

* Fix path lookup in flatc test

* Try a fix for Windows paths

* Convert path to string to fix Windows error

9 files changed, 193 insertions, 44 deletions

diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h
index 4cfd7ebf..230a6ef8 100644
--- a/include/flatbuffers/idl.h
+++ b/include/flatbuffers/idl.h
@@ -382,7 +382,14 @@ struct EnumVal {
   Offset<reflection::EnumVal> Serialize(FlatBufferBuilder *builder,
                                         const Parser &parser) const;
 
-  bool Deserialize(const Parser &parser, const reflection::EnumVal *val);
+  bool Deserialize(Parser &parser, const reflection::EnumVal *val);
+
+  flatbuffers::Offset<
+      flatbuffers::Vector<flatbuffers::Offset<reflection::KeyValue>>>
+  SerializeAttributes(FlatBufferBuilder *builder, const Parser &parser) const;
+
+  bool DeserializeAttributes(Parser &parser,
+                             const Vector<Offset<reflection::KeyValue>> *attrs);
 
   uint64_t GetAsUInt64() const { return static_cast<uint64_t>(value); }
   int64_t GetAsInt64() const { return value; }
@@ -392,6 +399,7 @@ struct EnumVal {
   std::string name;
   std::vector<std::string> doc_comment;
   Type union_type;
+  SymbolTable<Value> attributes;
 
  private:
   friend EnumDef;
diff --git a/include/flatbuffers/reflection_generated.h b/include/flatbuffers/reflection_generated.h
index 0f4d820d..ce7104ad 100644
--- a/include/flatbuffers/reflection_generated.h
+++ b/include/flatbuffers/reflection_generated.h
@@ -334,7 +334,8 @@ struct EnumVal FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table {
     VT_NAME = 4,
     VT_VALUE = 6,
     VT_UNION_TYPE = 10,
-    VT_DOCUMENTATION = 12
+    VT_DOCUMENTATION = 12,
+    VT_ATTRIBUTES = 14
   };
   const flatbuffers::String *name() const {
     return GetPointer<const flatbuffers::String *>(VT_NAME);
@@ -354,6 +355,9 @@ struct EnumVal FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table {
   const flatbuffers::Vector<flatbuffers::Offset<flatbuffers::String>> *documentation() const {
     return GetPointer<const flatbuffers::Vector<flatbuffers::Offset<flatbuffers::String>> *>(VT_DOCUMENTATION);
   }
+  const flatbuffers::Vector<flatbuffers::Offset<reflection::KeyValue>> *attributes() const {
+    return GetPointer<const flatbuffers::Vector<flatbuffers::Offset<reflection::KeyValue>> *>(VT_ATTRIBUTES);
+  }
   bool Verify(flatbuffers::Verifier &verifier) const {
     return VerifyTableStart(verifier) &&
            VerifyOffsetRequired(verifier, VT_NAME) &&
@@ -364,6 +368,9 @@ struct EnumVal FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table {
            VerifyOffset(verifier, VT_DOCUMENTATION) &&
            verifier.VerifyVector(documentation()) &&
            verifier.VerifyVectorOfStrings(documentation()) &&
+           VerifyOffset(verifier, VT_ATTRIBUTES) &&
+           verifier.VerifyVector(attributes()) &&
+           verifier.VerifyVectorOfTables(attributes()) &&
            verifier.EndTable();
   }
 };
@@ -384,6 +391,9 @@ struct EnumValBuilder {
   void add_documentation(flatbuffers::Offset<flatbuffers::Vector<flatbuffers::Offset<flatbuffers::String>>> documentation) {
     fbb_.AddOffset(EnumVal::VT_DOCUMENTATION, documentation);
   }
+  void add_attributes(flatbuffers::Offset<flatbuffers::Vector<flatbuffers::Offset<reflection::KeyValue>>> attributes) {
+    fbb_.AddOffset(EnumVal::VT_ATTRIBUTES, attributes);
+  }
   explicit EnumValBuilder(flatbuffers::FlatBufferBuilder &_fbb)
         : fbb_(_fbb) {
     start_ = fbb_.StartTable();
@@ -401,9 +411,11 @@ inline flatbuffers::Offset<EnumVal> CreateEnumVal(
     flatbuffers::Offset<flatbuffers::String> name = 0,
     int64_t value = 0,
     flatbuffers::Offset<reflection::Type> union_type = 0,
-    flatbuffers::Offset<flatbuffers::Vector<flatbuffers::Offset<flatbuffers::String>>> documentation = 0) {
+    flatbuffers::Offset<flatbuffers::Vector<flatbuffers::Offset<flatbuffers::String>>> documentation = 0,
+    flatbuffers::Offset<flatbuffers::Vector<flatbuffers::Offset<reflection::KeyValue>>> attributes = 0) {
   EnumValBuilder builder_(_fbb);
   builder_.add_value(value);
+  builder_.add_attributes(attributes);
   builder_.add_documentation(documentation);
   builder_.add_union_type(union_type);
   builder_.add_name(name);
@@ -415,15 +427,18 @@ inline flatbuffers::Offset<EnumVal> CreateEnumValDirect(
     const char *name = nullptr,
     int64_t value = 0,
     flatbuffers::Offset<reflection::Type> union_type = 0,
-    const std::vector<flatbuffers::Offset<flatbuffers::String>> *documentation = nullptr) {
+    const std::vector<flatbuffers::Offset<flatbuffers::String>> *documentation = nullptr,
+    std::vector<flatbuffers::Offset<reflection::KeyValue>> *attributes = nullptr) {
   auto name__ = name ? _fbb.CreateString(name) : 0;
   auto documentation__ = documentation ? _fbb.CreateVector<flatbuffers::Offset<flatbuffers::String>>(*documentation) : 0;
+  auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables<reflection::KeyValue>(attributes) : 0;
   return reflection::CreateEnumVal(
       _fbb,
       name__,
       value,
       union_type,
-      documentation__);
+      documentation__,
+      attributes__);
 }
 
 struct Enum FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table {
diff --git a/python/flatbuffers/reflection/EnumVal.py b/python/flatbuffers/reflection/EnumVal.py
index 62a32dc7..3592de08 100644
--- a/python/flatbuffers/reflection/EnumVal.py
+++ b/python/flatbuffers/reflection/EnumVal.py
@@ -73,7 +73,32 @@ class EnumVal(object):
         o = flatbuffers.number_types.UOffsetTFlags.py_type(self._tab.Offset(12))
         return o == 0
 
-def EnumValStart(builder): builder.StartObject(5)
+    # EnumVal
+    def Attributes(self, j):
+        o = flatbuffers.number_types.UOffsetTFlags.py_type(self._tab.Offset(14))
+        if o != 0:
+            x = self._tab.Vector(o)
+            x += flatbuffers.number_types.UOffsetTFlags.py_type(j) * 4
+            x = self._tab.Indirect(x)
+            from reflection.KeyValue import KeyValue
+            obj = KeyValue()
+            obj.Init(self._tab.Bytes, x)
+            return obj
+        return None
+
+    # EnumVal
+    def AttributesLength(self):
+        o = flatbuffers.number_types.UOffsetTFlags.py_type(self._tab.Offset(14))
+        if o != 0:
+            return self._tab.VectorLen(o)
+        return 0
+
+    # EnumVal
+    def AttributesIsNone(self):
+        o = flatbuffers.number_types.UOffsetTFlags.py_type(self._tab.Offset(14))
+        return o == 0
+
+def EnumValStart(builder): builder.StartObject(6)
 def Start(builder):
     return EnumValStart(builder)
 def EnumValAddName(builder, name): builder.PrependUOffsetTRelativeSlot(0, flatbuffers.number_types.UOffsetTFlags.py_type(name), 0)
@@ -91,6 +116,12 @@ def AddDocumentation(builder, documentation):
 def EnumValStartDocumentationVector(builder, numElems): return builder.StartVector(4, numElems, 4)
 def StartDocumentationVector(builder, numElems):
     return EnumValStartDocumentationVector(builder, numElems)
+def EnumValAddAttributes(builder, attributes): builder.PrependUOffsetTRelativeSlot(5, flatbuffers.number_types.UOffsetTFlags.py_type(attributes), 0)
+def AddAttributes(builder, attributes):
+    return EnumValAddAttributes(builder, attributes)
+def EnumValStartAttributesVector(builder, numElems): return builder.StartVector(4, numElems, 4)
+def StartAttributesVector(builder, numElems):
+    return EnumValStartAttributesVector(builder, numElems)
 def EnumValEnd(builder): return builder.EndObject()
 def End(builder):
     return EnumValEnd(builder)
\ No newline at end of file
diff --git a/reflection/reflection.fbs b/reflection/reflection.fbs
index 6dfeff66..b71a184b 100644
--- a/reflection/reflection.fbs
+++ b/reflection/reflection.fbs
@@ -56,6 +56,7 @@ table EnumVal {
     object:Object (deprecated);
     union_type:Type;
     documentation:[string];
+    attributes:[KeyValue];
 }
 
 table Enum {
diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp
index d8015cb6..9d9079a3 100644
--- a/src/idl_parser.cpp
+++ b/src/idl_parser.cpp
@@ -2450,14 +2450,17 @@ CheckedError Parser::ParseEnum(const bool is_union, EnumDef **dest,
         EXPECT(kTokenIntegerConstant);
       }
 
-      ECHECK(evb.AcceptEnumerator());
-
       if (opts.proto_mode && Is('[')) {
         NEXT();
         // ignore attributes on enums.
         while (token_ != ']') NEXT();
         NEXT();
+      } else {
+        // parse attributes in fbs schema
+        ECHECK(ParseMetaData(&ev.attributes));
       }
+
+      ECHECK(evb.AcceptEnumerator());
     }
     if (!Is(opts.proto_mode ? ';' : ',')) break;
     NEXT();
@@ -3633,6 +3636,44 @@ std::set<std::string> Parser::GetIncludedFilesRecursive(
 
 // Schema serialization functionality:
 
+static flatbuffers::Offset<
+    flatbuffers::Vector<flatbuffers::Offset<reflection::KeyValue>>>
+SerializeAttributesCommon(const SymbolTable<Value> &attributes,
+                          FlatBufferBuilder *builder, const Parser &parser) {
+  std::vector<flatbuffers::Offset<reflection::KeyValue>> attrs;
+  for (auto kv = attributes.dict.begin(); kv != attributes.dict.end(); ++kv) {
+    auto it = parser.known_attributes_.find(kv->first);
+    FLATBUFFERS_ASSERT(it != parser.known_attributes_.end());
+    if (parser.opts.binary_schema_builtins || !it->second) {
+      auto key = builder->CreateString(kv->first);
+      auto val = builder->CreateString(kv->second->constant);
+      attrs.push_back(reflection::CreateKeyValue(*builder, key, val));
+    }
+  }
+  if (attrs.size()) {
+    return builder->CreateVectorOfSortedTables(&attrs);
+  } else {
+    return 0;
+  }
+}
+
+static bool DeserializeAttributesCommon(
+    SymbolTable<Value> &attributes, Parser &parser,
+    const Vector<Offset<reflection::KeyValue>> *attrs) {
+  if (attrs == nullptr) return true;
+  for (uoffset_t i = 0; i < attrs->size(); ++i) {
+    auto kv = attrs->Get(i);
+    auto value = new Value();
+    if (kv->value()) { value->constant = kv->value()->str(); }
+    if (attributes.Add(kv->key()->str(), value)) {
+      delete value;
+      return false;
+    }
+    parser.known_attributes_[kv->key()->str()];
+  }
+  return true;
+}
+
 void Parser::Serialize() {
   builder_.Clear();
   AssignIndices(structs_.vec);
@@ -3918,21 +3959,35 @@ bool EnumDef::Deserialize(Parser &parser, const reflection::Enum *_enum) {
   return true;
 }
 
+flatbuffers::Offset<
+    flatbuffers::Vector<flatbuffers::Offset<reflection::KeyValue>>>
+EnumVal::SerializeAttributes(FlatBufferBuilder *builder,
+                             const Parser &parser) const {
+  return SerializeAttributesCommon(attributes, builder, parser);
+}
+
+bool EnumVal::DeserializeAttributes(
+    Parser &parser, const Vector<Offset<reflection::KeyValue>> *attrs) {
+  return DeserializeAttributesCommon(attributes, parser, attrs);
+}
+
 Offset<reflection::EnumVal> EnumVal::Serialize(FlatBufferBuilder *builder,
                                                const Parser &parser) const {
-  auto name__ = builder->CreateString(name);
-  auto type__ = union_type.Serialize(builder);
-  auto docs__ = parser.opts.binary_schema_comments
-                    ? builder->CreateVectorOfStrings(doc_comment)
-                    : 0;
-  return reflection::CreateEnumVal(*builder, name__, value, type__, docs__);
+  const auto name__ = builder->CreateString(name);
+  const auto type__ = union_type.Serialize(builder);
+  const auto attr__ = SerializeAttributes(builder, parser);
+  const auto docs__ = parser.opts.binary_schema_comments
+                          ? builder->CreateVectorOfStrings(doc_comment)
+                          : 0;
+  return reflection::CreateEnumVal(*builder, name__, value, type__, docs__,
+                                   attr__);
 }
 
-bool EnumVal::Deserialize(const Parser &parser,
-                          const reflection::EnumVal *val) {
+bool EnumVal::Deserialize(Parser &parser, const reflection::EnumVal *val) {
   name = val->name()->str();
   value = val->value();
   if (!union_type.Deserialize(parser, val->union_type())) return false;
+  if (!DeserializeAttributes(parser, val->attributes())) return false;
   DeserializeDoc(doc_comment, val->documentation());
   return true;
 }
@@ -3977,37 +4032,12 @@ flatbuffers::Offset<
     flatbuffers::Vector<flatbuffers::Offset<reflection::KeyValue>>>
 Definition::SerializeAttributes(FlatBufferBuilder *builder,
                                 const Parser &parser) const {
-  std::vector<flatbuffers::Offset<reflection::KeyValue>> attrs;
-  for (auto kv = attributes.dict.begin(); kv != attributes.dict.end(); ++kv) {
-    auto it = parser.known_attributes_.find(kv->first);
-    FLATBUFFERS_ASSERT(it != parser.known_attributes_.end());
-    if (parser.opts.binary_schema_builtins || !it->second) {
-      auto key = builder->CreateString(kv->first);
-      auto val = builder->CreateString(kv->second->constant);
-      attrs.push_back(reflection::CreateKeyValue(*builder, key, val));
-    }
-  }
-  if (attrs.size()) {
-    return builder->CreateVectorOfSortedTables(&attrs);
-  } else {
-    return 0;
-  }
+  return SerializeAttributesCommon(attributes, builder, parser);
 }
 
 bool Definition::DeserializeAttributes(
     Parser &parser, const Vector<Offset<reflection::KeyValue>> *attrs) {
-  if (attrs == nullptr) return true;
-  for (uoffset_t i = 0; i < attrs->size(); ++i) {
-    auto kv = attrs->Get(i);
-    auto value = new Value();
-    if (kv->value()) { value->constant = kv->value()->str(); }
-    if (attributes.Add(kv->key()->str(), value)) {
-      delete value;
-      return false;
-    }
-    parser.known_attributes_[kv->key()->str()];
-  }
-  return true;
+  return DeserializeAttributesCommon(attributes, parser, attrs);
 }
 
 /************************************************************************/
diff --git a/tests/flatc/enum_val_attributes.fbs b/tests/flatc/enum_val_attributes.fbs
new file mode 100644
index 00000000..33b67d7f
--- /dev/null
+++ b/tests/flatc/enum_val_attributes.fbs
@@ -0,0 +1,8 @@
+attribute display_name;
+
+enum ValAttributes : int
+{
+    Val1 = 0 (display_name: "Value 1"),
+    Val2 (display_name: "Value 2"),
+    Val3 (deprecated, display_name: "Value 3 (deprecated)"),
+}
\ No newline at end of file
diff --git a/tests/flatc/flatc_schema_tests.py b/tests/flatc/flatc_schema_tests.py
new file mode 100644
index 00000000..e79bb3ce
--- /dev/null
+++ b/tests/flatc/flatc_schema_tests.py
@@ -0,0 +1,43 @@
+# Copyright 2022 Google Inc. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from flatc_test import *
+import json
+
+
+class SchemaTests:
+    def EnumValAttributes(self):
+        # Generate .bfbs schema first
+        flatc(["--schema", "--binary", "--bfbs-builtins", "enum_val_attributes.fbs"])
+        assert_file_exists("enum_val_attributes.bfbs")
+
+        # Then turn it into JSON
+        flatc(["--json", "--strict-json", str(reflection_fbs_path()), "--", "enum_val_attributes.bfbs"])
+
+        # The attributes should be present in JSON
+        schema_json = json.loads(get_file_contents("enum_val_attributes.json"))
+
+        assert schema_json["enums"][0]["name"] == "ValAttributes"
+        assert schema_json["enums"][0]["values"][0]["name"] == "Val1"
+        assert schema_json["enums"][0]["values"][0]["attributes"][0]["key"] == "display_name"
+        assert schema_json["enums"][0]["values"][0]["attributes"][0]["value"] == "Value 1"
+
+        assert schema_json["enums"][0]["values"][1]["name"] == "Val2"
+        assert schema_json["enums"][0]["values"][1]["attributes"][0]["key"] == "display_name"
+        assert schema_json["enums"][0]["values"][1]["attributes"][0]["value"] == "Value 2"
+
+        assert schema_json["enums"][0]["values"][2]["name"] == "Val3"
+        assert schema_json["enums"][0]["values"][2]["attributes"][0]["key"] == "deprecated"
+        assert schema_json["enums"][0]["values"][2]["attributes"][1]["key"] == "display_name"
+        assert schema_json["enums"][0]["values"][2]["attributes"][1]["value"] == "Value 3 (deprecated)"
diff --git a/tests/flatc/flatc_test.py b/tests/flatc/flatc_test.py
index fcc32c6c..2e51743e 100755
--- a/tests/flatc/flatc_test.py
+++ b/tests/flatc/flatc_test.py
@@ -51,6 +51,10 @@ def flatc(options, cwd=script_path):
     subprocess.check_call(cmd, cwd=str(cwd))
 
 
+def reflection_fbs_path():
+    return Path(root_path).joinpath("reflection", "reflection.fbs")
+
+
 def make_absolute(filename, path=script_path):
     return str(Path(path, filename).absolute())
 
@@ -67,6 +71,14 @@ def assert_file_doesnt_exists(filename, path=script_path):
     return file
 
 
+def get_file_contents(filename, path=script_path):
+    file = Path(path, filename)
+    contents = ""
+    with open(file) as file:
+        contents = file.read()
+    return contents
+
+
 def assert_file_contains(file, needles):
     with open(file) as file:
         contents = file.read()
diff --git a/tests/flatc/main.py b/tests/flatc/main.py
index 1b6bee90..3bc23184 100755
--- a/tests/flatc/main.py
+++ b/tests/flatc/main.py
@@ -19,8 +19,9 @@ import sys
 from flatc_test import run_all
 from flatc_cpp_tests import CppTests
 from flatc_ts_tests import TsTests
+from flatc_schema_tests import SchemaTests
 
-passing, failing = run_all(CppTests, TsTests)
+passing, failing = run_all(CppTests, TsTests, SchemaTests)
 
 print("")
 print("{0} of {1} tests passed".format(passing, passing + failing))