Python: Escape enum member names if they correspond to a Python keyword (#4772)

* Python: Generated enum member names are now escaped if they correspond to a Python keyword.

* Keyword list in Python generator is now a const char* instead of an std::string array.

* Moved static functions and keyword list of Python generator into the PythonGenerator class.

* Python generator escapes keyword identifiers in all definitions.

src/idl_gen_python.cpp

@@ -23,21 +23,58 @@
 #include "flatbuffers/idl.h"
 #include "flatbuffers/util.h"
 
+#include <unordered_set>
+
 namespace flatbuffers {
 namespace python {
 
-static std::string GenGetter(const Type &type);
-static std::string GenMethod(const FieldDef &field);
-static void GenStructBuilder(const StructDef &struct_def,
-                             std::string *code_ptr);
-static void GenReceiver(const StructDef &struct_def, std::string *code_ptr);
-static std::string GenTypeBasic(const Type &type);
-static std::string GenTypeGet(const Type &type);
-static std::string TypeName(const FieldDef &field);
-
 // Hardcode spaces per indentation.
 const std::string Indent = "    ";
 
+class PythonGenerator : public BaseGenerator {
+ public:
+  PythonGenerator(const Parser &parser, const std::string &path,
+                  const std::string &file_name)
+      : BaseGenerator(parser, path, file_name, "" /* not used */,
+                      "" /* not used */){
+    static const char * const keywords[] = {
+      "False",
+      "None",
+      "True",
+      "and",
+      "as",
+      "assert",
+      "break",
+      "class",
+      "continue",
+      "def",
+      "del",
+      "elif",
+      "else",
+      "except",
+      "finally",
+      "for",
+      "from",
+      "global",
+      "if",
+      "import",
+      "in",
+      "is",
+      "lambda",
+      "nonlocal",
+      "not",
+      "or",
+      "pass",
+      "raise",
+      "return",
+      "try",
+      "while",
+      "with",
+      "yield"
+    };
+    for (auto kw = keywords; *kw; kw++) keywords_.insert(*kw);
+  }
+
   // Most field accessors need to retrieve and test the field offset first,
   // this is the prefix code for that.
   std::string OffsetPrefix(const FieldDef &field) {
@@ -48,55 +85,67 @@ std::string OffsetPrefix(const FieldDef &field) {
   }
 
   // Begin a class declaration.
-static void BeginClass(const StructDef &struct_def, std::string *code_ptr) {
+  void BeginClass(const StructDef &struct_def, std::string *code_ptr) {
     std::string &code = *code_ptr;
-  code += "class " + struct_def.name + "(object):\n";
+    code += "class " + NormalizedName(struct_def) + "(object):\n";
     code += Indent + "__slots__ = ['_tab']";
     code += "\n\n";
   }
 
   // Begin enum code with a class declaration.
-static void BeginEnum(const std::string class_name, std::string *code_ptr) {
+  void BeginEnum(const std::string class_name, std::string *code_ptr) {
     std::string &code = *code_ptr;
     code += "class " + class_name + "(object):\n";
   }
 
+  std::string EscapeKeyword(const std::string &name) const {
+    return keywords_.find(name) == keywords_.end() ? name : name + "_";
+  }
+
+  std::string NormalizedName(const Definition &definition) const {
+    return EscapeKeyword(definition.name);
+  }
+
+  std::string NormalizedName(const EnumVal &ev) const {
+    return EscapeKeyword(ev.name);
+  }
+
   // A single enum member.
-static void EnumMember(const EnumVal ev, std::string *code_ptr) {
+  void EnumMember(const EnumVal ev, std::string *code_ptr) {
     std::string &code = *code_ptr;
     code += Indent;
-  code += ev.name;
+    code += NormalizedName(ev);
     code += " = ";
     code += NumToString(ev.value) + "\n";
   }
 
   // End enum code.
-static void EndEnum(std::string *code_ptr) {
+  void EndEnum(std::string *code_ptr) {
     std::string &code = *code_ptr;
     code += "\n";
   }
 
   // Initialize a new struct or table from existing data.
-static void NewRootTypeFromBuffer(const StructDef &struct_def,
+  void NewRootTypeFromBuffer(const StructDef &struct_def,
                              std::string *code_ptr) {
     std::string &code = *code_ptr;
 
     code += Indent + "@classmethod\n";
     code += Indent + "def GetRootAs";
-  code += struct_def.name;
+    code += NormalizedName(struct_def);
     code += "(cls, buf, offset):";
     code += "\n";
     code += Indent + Indent;
     code += "n = flatbuffers.encode.Get";
     code += "(flatbuffers.packer.uoffset, buf, offset)\n";
-  code += Indent + Indent + "x = " + struct_def.name + "()\n";
+    code += Indent + Indent + "x = " + NormalizedName(struct_def) + "()\n";
     code += Indent + Indent + "x.Init(buf, n + offset)\n";
     code += Indent + Indent + "return x\n";
     code += "\n";
   }
 
   // Initialize an existing object with other data, to avoid an allocation.
-static void InitializeExisting(const StructDef &struct_def,
+  void InitializeExisting(const StructDef &struct_def,
                           std::string *code_ptr) {
     std::string &code = *code_ptr;
 
@@ -107,38 +156,38 @@ static void InitializeExisting(const StructDef &struct_def,
   }
 
   // Get the length of a vector.
-static void GetVectorLen(const StructDef &struct_def, const FieldDef &field,
+  void GetVectorLen(const StructDef &struct_def, const FieldDef &field,
                     std::string *code_ptr) {
     std::string &code = *code_ptr;
 
     GenReceiver(struct_def, code_ptr);
-  code += MakeCamel(field.name) + "Length(self";
+    code += MakeCamel(NormalizedName(field)) + "Length(self";
     code += "):" + OffsetPrefix(field);
     code += Indent + Indent + Indent + "return self._tab.VectorLen(o)\n";
     code += Indent + Indent + "return 0\n\n";
   }
 
   // Get the value of a struct's scalar.
-static void GetScalarFieldOfStruct(const StructDef &struct_def,
+  void GetScalarFieldOfStruct(const StructDef &struct_def,
                               const FieldDef &field,
                               std::string *code_ptr) {
     std::string &code = *code_ptr;
     std::string getter = GenGetter(field.value.type);
     GenReceiver(struct_def, code_ptr);
-  code += MakeCamel(field.name);
+    code += MakeCamel(NormalizedName(field));
     code += "(self): return " + getter;
     code += "self._tab.Pos + flatbuffers.number_types.UOffsetTFlags.py_type(";
     code += NumToString(field.value.offset) + "))\n";
   }
 
   // Get the value of a table's scalar.
-static void GetScalarFieldOfTable(const StructDef &struct_def,
+  void GetScalarFieldOfTable(const StructDef &struct_def,
                              const FieldDef &field,
                              std::string *code_ptr) {
     std::string &code = *code_ptr;
     std::string getter = GenGetter(field.value.type);
     GenReceiver(struct_def, code_ptr);
-  code += MakeCamel(field.name);
+    code += MakeCamel(NormalizedName(field));
     code += "(self):";
     code += OffsetPrefix(field);
     getter += "o + self._tab.Pos)";
@@ -158,12 +207,12 @@ static void GetScalarFieldOfTable(const StructDef &struct_def,
 
   // Get a struct by initializing an existing struct.
   // Specific to Struct.
-static void GetStructFieldOfStruct(const StructDef &struct_def,
+  void GetStructFieldOfStruct(const StructDef &struct_def,
                               const FieldDef &field,
                               std::string *code_ptr) {
     std::string &code = *code_ptr;
     GenReceiver(struct_def, code_ptr);
-  code += MakeCamel(field.name);
+    code += MakeCamel(NormalizedName(field));
     code += "(self, obj):\n";
     code += Indent + Indent + "obj.Init(self._tab.Bytes, self._tab.Pos + ";
     code += NumToString(field.value.offset) + ")";
@@ -172,12 +221,12 @@ static void GetStructFieldOfStruct(const StructDef &struct_def,
 
   // Get a struct by initializing an existing struct.
   // Specific to Table.
-static void GetStructFieldOfTable(const StructDef &struct_def,
+  void GetStructFieldOfTable(const StructDef &struct_def,
                              const FieldDef &field,
                              std::string *code_ptr) {
     std::string &code = *code_ptr;
     GenReceiver(struct_def, code_ptr);
-  code += MakeCamel(field.name);
+    code += MakeCamel(NormalizedName(field));
     code += "(self):";
     code += OffsetPrefix(field);
     if (field.value.type.struct_def->fixed) {
@@ -195,11 +244,11 @@ static void GetStructFieldOfTable(const StructDef &struct_def,
   }
 
   // Get the value of a string.
-static void GetStringField(const StructDef &struct_def, const FieldDef &field,
+  void GetStringField(const StructDef &struct_def, const FieldDef &field,
                       std::string *code_ptr) {
     std::string &code = *code_ptr;
     GenReceiver(struct_def, code_ptr);
-  code += MakeCamel(field.name);
+    code += MakeCamel(NormalizedName(field));
     code += "(self):";
     code += OffsetPrefix(field);
     code += Indent + Indent + Indent + "return " + GenGetter(field.value.type);
@@ -208,11 +257,11 @@ static void GetStringField(const StructDef &struct_def, const FieldDef &field,
   }
 
   // Get the value of a union from an object.
-static void GetUnionField(const StructDef &struct_def, const FieldDef &field,
+  void GetUnionField(const StructDef &struct_def, const FieldDef &field,
                      std::string *code_ptr) {
     std::string &code = *code_ptr;
     GenReceiver(struct_def, code_ptr);
-  code += MakeCamel(field.name) + "(self):";
+    code += MakeCamel(NormalizedName(field)) + "(self):";
     code += OffsetPrefix(field);
 
     // TODO(rw): this works and is not the good way to it:
@@ -230,14 +279,14 @@ static void GetUnionField(const StructDef &struct_def, const FieldDef &field,
   }
 
   // Get the value of a vector's struct member.
-static void GetMemberOfVectorOfStruct(const StructDef &struct_def,
+  void GetMemberOfVectorOfStruct(const StructDef &struct_def,
                                  const FieldDef &field,
                                  std::string *code_ptr) {
     std::string &code = *code_ptr;
     auto vectortype = field.value.type.VectorType();
 
     GenReceiver(struct_def, code_ptr);
-  code += MakeCamel(field.name);
+    code += MakeCamel(NormalizedName(field));
     code += "(self, j):" + OffsetPrefix(field);
     code += Indent + Indent + Indent + "x = self._tab.Vector(o)\n";
     code += Indent + Indent + Indent;
@@ -256,14 +305,14 @@ static void GetMemberOfVectorOfStruct(const StructDef &struct_def,
 
   // Get the value of a vector's non-struct member. Uses a named return
   // argument to conveniently set the zero value for the result.
-static void GetMemberOfVectorOfNonStruct(const StructDef &struct_def,
+  void GetMemberOfVectorOfNonStruct(const StructDef &struct_def,
                                     const FieldDef &field,
                                     std::string *code_ptr) {
     std::string &code = *code_ptr;
     auto vectortype = field.value.type.VectorType();
 
     GenReceiver(struct_def, code_ptr);
-  code += MakeCamel(field.name);
+    code += MakeCamel(NormalizedName(field));
     code += "(self, j):";
     code += OffsetPrefix(field);
     code += Indent + Indent + Indent + "a = self._tab.Vector(o)\n";
@@ -281,7 +330,7 @@ static void GetMemberOfVectorOfNonStruct(const StructDef &struct_def,
 
   // Returns a non-struct vector as a numpy array. Much faster
   // than iterating over the vector element by element.
-static void GetVectorOfNonStructAsNumpy(const StructDef &struct_def,
+  void GetVectorOfNonStructAsNumpy(const StructDef &struct_def,
                                    const FieldDef &field,
                                    std::string *code_ptr) {
     std::string &code = *code_ptr;
@@ -292,7 +341,7 @@ static void GetVectorOfNonStructAsNumpy(const StructDef &struct_def,
     if (!(IsScalar(vectortype.base_type))) { return; }
 
     GenReceiver(struct_def, code_ptr);
-  code += MakeCamel(field.name) + "AsNumpy(self):";
+    code += MakeCamel(NormalizedName(field)) + "AsNumpy(self):";
     code += OffsetPrefix(field);
 
     code += Indent + Indent + Indent;
@@ -310,18 +359,18 @@ static void GetVectorOfNonStructAsNumpy(const StructDef &struct_def,
   }
 
   // Begin the creator function signature.
-static void BeginBuilderArgs(const StructDef &struct_def,
+  void BeginBuilderArgs(const StructDef &struct_def,
                         std::string *code_ptr) {
     std::string &code = *code_ptr;
 
     code += "\n";
-  code += "def Create" + struct_def.name;
+    code += "def Create" + NormalizedName(struct_def);
     code += "(builder";
   }
 
   // Recursively generate arguments for a constructor, to deal with nested
   // structs.
-static void StructBuilderArgs(const StructDef &struct_def,
+  void StructBuilderArgs(const StructDef &struct_def,
                          const char *nameprefix, std::string *code_ptr) {
     for (auto it = struct_def.fields.vec.begin();
         it != struct_def.fields.vec.end(); ++it) {
@@ -331,24 +380,24 @@ static void StructBuilderArgs(const StructDef &struct_def,
         // don't clash, and to make it obvious these arguments are constructing
         // a nested struct, prefix the name with the field name.
         StructBuilderArgs(*field.value.type.struct_def,
-                        (nameprefix + (field.name + "_")).c_str(), code_ptr);
+                          (nameprefix + (NormalizedName(field) + "_")).c_str(), code_ptr);
       } else {
         std::string &code = *code_ptr;
         code += (std::string) ", " + nameprefix;
-      code += MakeCamel(field.name, false);
+        code += MakeCamel(NormalizedName(field), false);
       }
     }
   }
 
   // End the creator function signature.
-static void EndBuilderArgs(std::string *code_ptr) {
+  void EndBuilderArgs(std::string *code_ptr) {
     std::string &code = *code_ptr;
     code += "):\n";
   }
 
   // Recursively generate struct construction statements and instert manual
   // padding.
-static void StructBuilderBody(const StructDef &struct_def,
+  void StructBuilderBody(const StructDef &struct_def,
                          const char *nameprefix, std::string *code_ptr) {
     std::string &code = *code_ptr;
     code += "    builder.Prep(" + NumToString(struct_def.minalign) + ", ";
@@ -360,24 +409,24 @@ static void StructBuilderBody(const StructDef &struct_def,
         code += "    builder.Pad(" + NumToString(field.padding) + ")\n";
       if (IsStruct(field.value.type)) {
         StructBuilderBody(*field.value.type.struct_def,
-                        (nameprefix + (field.name + "_")).c_str(), code_ptr);
+                          (nameprefix + (NormalizedName(field) + "_")).c_str(), code_ptr);
       } else {
         code += "    builder.Prepend" + GenMethod(field) + "(";
-      code += nameprefix + MakeCamel(field.name, false) + ")\n";
+        code += nameprefix + MakeCamel(NormalizedName(field), false) + ")\n";
       }
     }
   }
 
-static void EndBuilderBody(std::string *code_ptr) {
+  void EndBuilderBody(std::string *code_ptr) {
     std::string &code = *code_ptr;
     code += "    return builder.Offset()\n";
   }
 
   // Get the value of a table's starting offset.
-static void GetStartOfTable(const StructDef &struct_def,
+  void GetStartOfTable(const StructDef &struct_def,
                        std::string *code_ptr) {
     std::string &code = *code_ptr;
-  code += "def " + struct_def.name + "Start";
+    code += "def " + NormalizedName(struct_def) + "Start";
     code += "(builder): ";
     code += "builder.StartObject(";
     code += NumToString(struct_def.fields.vec.size());
@@ -385,13 +434,13 @@ static void GetStartOfTable(const StructDef &struct_def,
   }
 
   // Set the value of a table's field.
-static void BuildFieldOfTable(const StructDef &struct_def,
+  void BuildFieldOfTable(const StructDef &struct_def,
                          const FieldDef &field, const size_t offset,
                          std::string *code_ptr) {
     std::string &code = *code_ptr;
-  code += "def " + struct_def.name + "Add" + MakeCamel(field.name);
+    code += "def " + NormalizedName(struct_def) + "Add" + MakeCamel(NormalizedName(field));
     code += "(builder, ";
-  code += MakeCamel(field.name, false);
+    code += MakeCamel(NormalizedName(field), false);
     code += "): ";
     code += "builder.Prepend";
     code += GenMethod(field) + "Slot(";
@@ -399,20 +448,20 @@ static void BuildFieldOfTable(const StructDef &struct_def,
     if (!IsScalar(field.value.type.base_type) && (!struct_def.fixed)) {
       code += "flatbuffers.number_types.UOffsetTFlags.py_type";
       code += "(";
-    code += MakeCamel(field.name, false) + ")";
+      code += MakeCamel(NormalizedName(field), false) + ")";
     } else {
-    code += MakeCamel(field.name, false);
+      code += MakeCamel(NormalizedName(field), false);
     }
     code += ", " + field.value.constant;
     code += ")\n";
   }
 
   // Set the value of one of the members of a table's vector.
-static void BuildVectorOfTable(const StructDef &struct_def,
+  void BuildVectorOfTable(const StructDef &struct_def,
                           const FieldDef &field, std::string *code_ptr) {
     std::string &code = *code_ptr;
-  code += "def " + struct_def.name + "Start";
+    code += "def " + NormalizedName(struct_def) + "Start";
-  code += MakeCamel(field.name);
+    code += MakeCamel(NormalizedName(field));
     code += "Vector(builder, numElems): return builder.StartVector(";
     auto vector_type = field.value.type.VectorType();
     auto alignment = InlineAlignment(vector_type);
@@ -423,23 +472,23 @@ static void BuildVectorOfTable(const StructDef &struct_def,
   }
 
   // Get the offset of the end of a table.
-static void GetEndOffsetOnTable(const StructDef &struct_def,
+  void GetEndOffsetOnTable(const StructDef &struct_def,
                            std::string *code_ptr) {
     std::string &code = *code_ptr;
-  code += "def " + struct_def.name + "End";
+    code += "def " + NormalizedName(struct_def) + "End";
     code += "(builder): ";
     code += "return builder.EndObject()\n";
   }
 
   // Generate the receiver for function signatures.
-static void GenReceiver(const StructDef &struct_def, std::string *code_ptr) {
+  void GenReceiver(const StructDef &struct_def, std::string *code_ptr) {
     std::string &code = *code_ptr;
-  code += Indent + "# " + struct_def.name + "\n";
+    code += Indent + "# " + NormalizedName(struct_def) + "\n";
     code += Indent + "def ";
   }
 
   // Generate a struct field, conditioned on its child type(s).
-static void GenStructAccessor(const StructDef &struct_def,
+  void GenStructAccessor(const StructDef &struct_def,
                          const FieldDef &field, std::string *code_ptr) {
     GenComment(field.doc_comment, code_ptr, nullptr, "# ");
     if (IsScalar(field.value.type.base_type)) {
@@ -478,7 +527,7 @@ static void GenStructAccessor(const StructDef &struct_def,
   }
 
   // Generate table constructors, conditioned on its members' types.
-static void GenTableBuilders(const StructDef &struct_def,
+  void GenTableBuilders(const StructDef &struct_def,
                         std::string *code_ptr) {
     GetStartOfTable(struct_def, code_ptr);
 
@@ -498,7 +547,7 @@ static void GenTableBuilders(const StructDef &struct_def,
   }
 
   // Generate struct or table methods.
-static void GenStruct(const StructDef &struct_def, std::string *code_ptr) {
+  void GenStruct(const StructDef &struct_def, std::string *code_ptr) {
     if (struct_def.generated) return;
 
     GenComment(struct_def.doc_comment, code_ptr, nullptr, "# ");
@@ -529,11 +578,11 @@ static void GenStruct(const StructDef &struct_def, std::string *code_ptr) {
   }
 
   // Generate enum declarations.
-static void GenEnum(const EnumDef &enum_def, std::string *code_ptr) {
+  void GenEnum(const EnumDef &enum_def, std::string *code_ptr) {
     if (enum_def.generated) return;
 
     GenComment(enum_def.doc_comment, code_ptr, nullptr, "# ");
-  BeginEnum(enum_def.name, code_ptr);
+    BeginEnum(NormalizedName(enum_def), code_ptr);
     for (auto it = enum_def.vals.vec.begin(); it != enum_def.vals.vec.end();
         ++it) {
       auto &ev = **it;
@@ -544,7 +593,7 @@ static void GenEnum(const EnumDef &enum_def, std::string *code_ptr) {
   }
 
   // Returns the function name that is able to read a value of the given type.
-static std::string GenGetter(const Type &type) {
+  std::string GenGetter(const Type &type) {
     switch (type.base_type) {
       case BASE_TYPE_STRING: return "self._tab.String(";
       case BASE_TYPE_UNION: return "self._tab.Union(";
@@ -556,13 +605,13 @@ static std::string GenGetter(const Type &type) {
   }
 
   // Returns the method name for use with add/put calls.
-static std::string GenMethod(const FieldDef &field) {
+  std::string GenMethod(const FieldDef &field) {
     return IsScalar(field.value.type.base_type)
               ? MakeCamel(GenTypeBasic(field.value.type))
               : (IsStruct(field.value.type) ? "Struct" : "UOffsetTRelative");
   }
 
-static std::string GenTypeBasic(const Type &type) {
+  std::string GenTypeBasic(const Type &type) {
     static const char *ctypename[] = {
     // clang-format off
       #define FLATBUFFERS_TD(ENUM, IDLTYPE, \
@@ -575,7 +624,7 @@ static std::string GenTypeBasic(const Type &type) {
     return ctypename[type.base_type];
   }
 
-static std::string GenTypePointer(const Type &type) {
+  std::string GenTypePointer(const Type &type) {
     switch (type.base_type) {
       case BASE_TYPE_STRING: return "string";
       case BASE_TYPE_VECTOR: return GenTypeGet(type.VectorType());
@@ -586,16 +635,16 @@ static std::string GenTypePointer(const Type &type) {
     }
   }
 
-static std::string GenTypeGet(const Type &type) {
+  std::string GenTypeGet(const Type &type) {
     return IsScalar(type.base_type) ? GenTypeBasic(type) : GenTypePointer(type);
   }
 
-static std::string TypeName(const FieldDef &field) {
+  std::string TypeName(const FieldDef &field) {
     return GenTypeGet(field.value.type);
   }
 
   // Create a struct with a builder and the struct's arguments.
-static void GenStructBuilder(const StructDef &struct_def,
+  void GenStructBuilder(const StructDef &struct_def,
                               std::string *code_ptr) {
     BeginBuilderArgs(struct_def, code_ptr);
     StructBuilderArgs(struct_def, "", code_ptr);
@@ -605,12 +654,6 @@ static void GenStructBuilder(const StructDef &struct_def,
     EndBuilderBody(code_ptr);
   }
 
-class PythonGenerator : public BaseGenerator {
- public:
-  PythonGenerator(const Parser &parser, const std::string &path,
-                  const std::string &file_name)
-      : BaseGenerator(parser, path, file_name, "" /* not used */,
-                      "" /* not used */){};
   bool generate() {
     if (!generateEnums()) return false;
     if (!generateStructs()) return false;
@@ -667,9 +710,11 @@ class PythonGenerator : public BaseGenerator {
     BeginFile(LastNamespacePart(*def.defined_namespace), needs_imports, &code);
     code += classcode;
     std::string filename =
-        NamespaceDir(*def.defined_namespace) + def.name + ".py";
+        NamespaceDir(*def.defined_namespace) + NormalizedName(def) + ".py";
     return SaveFile(filename.c_str(), code, false);
   }
+ private:
+  std::unordered_set<std::string> keywords_;
 };
 
 }  // namespace python