From 77c9a8803256502c8cfa568a745cc44954cc7ec9 Mon Sep 17 00:00:00 2001 From: tmimmanuel <14046872+tmimmanuel@users.noreply.github.com> Date: Mon, 16 Mar 2026 12:55:17 +0100 Subject: [PATCH] Fix inconsistent Python union creator function naming --- scripts/generate_code.py | 5 ++ src/idl_gen_python.cpp | 6 +- tests/PythonTest.sh | 1 + tests/py_test.py | 48 ++++++++++ tests/union_name_test.fbs | 21 +++++ tests/union_name_test/Bar.py | 93 +++++++++++++++++++ tests/union_name_test/Container.py | 120 +++++++++++++++++++++++++ tests/union_name_test/Foo.py | 90 +++++++++++++++++++ tests/union_name_test/__init__.py | 0 tests/union_name_test/my_test_union.py | 20 +++++ 10 files changed, 401 insertions(+), 3 deletions(-) create mode 100644 tests/union_name_test.fbs create mode 100644 tests/union_name_test/Bar.py create mode 100644 tests/union_name_test/Container.py create mode 100644 tests/union_name_test/Foo.py create mode 100644 tests/union_name_test/__init__.py create mode 100644 tests/union_name_test/my_test_union.py diff --git a/scripts/generate_code.py b/scripts/generate_code.py index e14b96dcda3..b754ec58f66 100755 --- a/scripts/generate_code.py +++ b/scripts/generate_code.py @@ -422,6 +422,11 @@ def glob(path, pattern): schema="nested_union_test.fbs", ) +flatc( + ["--python", "--gen-object-api"], + schema="union_name_test.fbs", +) + flatc( NO_INCL_OPTS + CPP_OPTS, schema="default_vectors_strings_test.fbs", diff --git a/src/idl_gen_python.cpp b/src/idl_gen_python.cpp index cd96081ecd4..fa3787088b1 100644 --- a/src/idl_gen_python.cpp +++ b/src/idl_gen_python.cpp @@ -2095,12 +2095,12 @@ class PythonGenerator : public BaseGenerator { const auto field_method = namer_.Method(field); const auto struct_var = namer_.Variable(struct_def); const EnumDef& enum_def = *field.value.type.enum_def; - auto union_type = namer_.Type(enum_def); + auto union_fn = namer_.Function(enum_def); if (parser_.opts.include_dependence_headers) { - union_type = namer_.NamespacedType(enum_def) + "." + union_type; + union_fn = namer_.NamespacedType(enum_def) + "." + union_fn; } - code += GenIndents(2) + "self." + field_field + " = " + union_type + + code += GenIndents(2) + "self." + field_field + " = " + union_fn + "Creator(" + "self." + field_field + "Type, " + struct_var + "." + field_method + "())"; } diff --git a/tests/PythonTest.sh b/tests/PythonTest.sh index 90bee57244e..e7806bd429f 100755 --- a/tests/PythonTest.sh +++ b/tests/PythonTest.sh @@ -28,6 +28,7 @@ ${test_dir}/../flatc -p -o ${gen_code_path} -I include_test monster_extra.fbs -- ${test_dir}/../flatc -p -o ${gen_code_path} -I include_test arrays_test.fbs --gen-object-api --python-typing ${test_dir}/../flatc -p -o ${gen_code_path} -I include_test nested_union_test.fbs --gen-object-api --python-typing --python-decode-obj-api-strings ${test_dir}/../flatc -p -o ${gen_code_path} -I include_test service_test.fbs --grpc --grpc-python-typed-handlers --python-typing --no-python-gen-numpy --gen-onefile +${test_dir}/../flatc -p -o ${gen_code_path} union_name_test.fbs --gen-object-api # Syntax: run_tests # diff --git a/tests/py_test.py b/tests/py_test.py index fb483816b85..0be6177855f 100644 --- a/tests/py_test.py +++ b/tests/py_test.py @@ -55,6 +55,10 @@ import monster_test_generated # the one-file version import optional_scalars import optional_scalars.ScalarStuff +import union_name_test.Container # refers to generated code +import union_name_test.Foo # refers to generated code +import union_name_test.Bar # refers to generated code +import union_name_test.my_test_union # refers to generated code def create_namespace_shortcut(is_onefile): @@ -3020,6 +3024,50 @@ def test_nested_union_tables(self): ) +class TestUnionCreatorNaming(unittest.TestCase): + """Tests that union creator functions use consistent naming (issue #8843). + + Uses a schema with a snake_case union name (my_test_union) to verify that + the generated creator function name matches between definition and call site. + """ + + def test_union_creator_pack_unpack(self): + """Pack and UnPack a table with a non-UpperCamel union name.""" + containerT = union_name_test.Container.ContainerT() + containerT.uType = union_name_test.my_test_union.my_test_union.Foo + containerT.u = union_name_test.Foo.FooT() + containerT.u.val = 42 + + b = flatbuffers.Builder(0) + b.Finish(containerT.Pack(b)) + + container = union_name_test.Container.Container.GetRootAs( + b.Bytes, b.Head() + ) + containerT2 = union_name_test.Container.ContainerT.InitFromObj(container) + + self.assertEqual(containerT2.uType, union_name_test.my_test_union.my_test_union.Foo) + self.assertEqual(containerT2.u.val, 42) + + def test_union_creator_with_bar(self): + """Test the other union variant to ensure all branches work.""" + containerT = union_name_test.Container.ContainerT() + containerT.uType = union_name_test.my_test_union.my_test_union.Bar + containerT.u = union_name_test.Bar.BarT() + containerT.u.name = "hello" + + b = flatbuffers.Builder(0) + b.Finish(containerT.Pack(b)) + + container = union_name_test.Container.Container.GetRootAs( + b.Bytes, b.Head() + ) + containerT2 = union_name_test.Container.ContainerT.InitFromObj(container) + + self.assertEqual(containerT2.uType, union_name_test.my_test_union.my_test_union.Bar) + self.assertEqual(containerT2.u.name, b"hello") + + class TestBuilderClear(unittest.TestCase): def test_consistency(self): diff --git a/tests/union_name_test.fbs b/tests/union_name_test.fbs new file mode 100644 index 00000000000..0452f7ad1e0 --- /dev/null +++ b/tests/union_name_test.fbs @@ -0,0 +1,21 @@ +// Test for union creator naming consistency (issue #8843). +// Uses non-UpperCamel union name to verify that Function() naming +// is used consistently for both definition and call site. + +namespace union_name_test; + +table Foo { + val: int; +} + +table Bar { + name: string; +} + +union my_test_union { Foo, Bar } + +table Container { + u: my_test_union; +} + +root_type Container; diff --git a/tests/union_name_test/Bar.py b/tests/union_name_test/Bar.py new file mode 100644 index 00000000000..eacf02dda2b --- /dev/null +++ b/tests/union_name_test/Bar.py @@ -0,0 +1,93 @@ +# automatically generated by the FlatBuffers compiler, do not modify + +# namespace: union_name_test + +import flatbuffers +from flatbuffers.compat import import_numpy +np = import_numpy() + +class Bar(object): + __slots__ = ['_tab'] + + @classmethod + def GetRootAs(cls, buf, offset=0): + n = flatbuffers.encode.Get(flatbuffers.packer.uoffset, buf, offset) + x = Bar() + x.Init(buf, n + offset) + return x + + @classmethod + def GetRootAsBar(cls, buf, offset=0): + """This method is deprecated. Please switch to GetRootAs.""" + return cls.GetRootAs(buf, offset) + # Bar + def Init(self, buf, pos): + self._tab = flatbuffers.table.Table(buf, pos) + + # Bar + def Name(self): + o = flatbuffers.number_types.UOffsetTFlags.py_type(self._tab.Offset(4)) + if o != 0: + return self._tab.String(o + self._tab.Pos) + return None + +def BarStart(builder): + builder.StartObject(1) + +def Start(builder): + BarStart(builder) + +def BarAddName(builder, name): + builder.PrependUOffsetTRelativeSlot(0, flatbuffers.number_types.UOffsetTFlags.py_type(name), 0) + +def AddName(builder, name): + BarAddName(builder, name) + +def BarEnd(builder): + return builder.EndObject() + +def End(builder): + return BarEnd(builder) + + +class BarT(object): + + # BarT + def __init__( + self, + name = None, + ): + self.name = name # type: Optional[str] + + @classmethod + def InitFromBuf(cls, buf, pos): + bar = Bar() + bar.Init(buf, pos) + return cls.InitFromObj(bar) + + @classmethod + def InitFromPackedBuf(cls, buf, pos=0): + n = flatbuffers.encode.Get(flatbuffers.packer.uoffset, buf, pos) + return cls.InitFromBuf(buf, pos+n) + + @classmethod + def InitFromObj(cls, bar): + x = BarT() + x._UnPack(bar) + return x + + # BarT + def _UnPack(self, bar): + if bar is None: + return + self.name = bar.Name() + + # BarT + def Pack(self, builder): + if self.name is not None: + name = builder.CreateString(self.name) + BarStart(builder) + if self.name is not None: + BarAddName(builder, name) + bar = BarEnd(builder) + return bar diff --git a/tests/union_name_test/Container.py b/tests/union_name_test/Container.py new file mode 100644 index 00000000000..c39580b02ed --- /dev/null +++ b/tests/union_name_test/Container.py @@ -0,0 +1,120 @@ +# automatically generated by the FlatBuffers compiler, do not modify + +# namespace: union_name_test + +import flatbuffers +from flatbuffers.compat import import_numpy +np = import_numpy() + +class Container(object): + __slots__ = ['_tab'] + + @classmethod + def GetRootAs(cls, buf, offset=0): + n = flatbuffers.encode.Get(flatbuffers.packer.uoffset, buf, offset) + x = Container() + x.Init(buf, n + offset) + return x + + @classmethod + def GetRootAsContainer(cls, buf, offset=0): + """This method is deprecated. Please switch to GetRootAs.""" + return cls.GetRootAs(buf, offset) + # Container + def Init(self, buf, pos): + self._tab = flatbuffers.table.Table(buf, pos) + + # Container + def UType(self): + o = flatbuffers.number_types.UOffsetTFlags.py_type(self._tab.Offset(4)) + if o != 0: + return self._tab.Get(flatbuffers.number_types.Uint8Flags, o + self._tab.Pos) + return 0 + + # Container + def U(self): + o = flatbuffers.number_types.UOffsetTFlags.py_type(self._tab.Offset(6)) + if o != 0: + from flatbuffers.table import Table + obj = Table(bytearray(), 0) + self._tab.Union(obj, o) + return obj + return None + +def ContainerStart(builder): + builder.StartObject(2) + +def Start(builder): + ContainerStart(builder) + +def ContainerAddUType(builder, uType): + builder.PrependUint8Slot(0, uType, 0) + +def AddUType(builder, uType): + ContainerAddUType(builder, uType) + +def ContainerAddU(builder, u): + builder.PrependUOffsetTRelativeSlot(1, flatbuffers.number_types.UOffsetTFlags.py_type(u), 0) + +def AddU(builder, u): + ContainerAddU(builder, u) + +def ContainerEnd(builder): + return builder.EndObject() + +def End(builder): + return ContainerEnd(builder) + +import union_name_test.Bar +import union_name_test.Foo +import union_name_test.my_test_union +try: + from typing import Union +except: + pass + +class ContainerT(object): + + # ContainerT + def __init__( + self, + uType = 0, + u = None, + ): + self.uType = uType # type: int + self.u = u # type: Union[None, 'union_name_test.Foo.FooT', 'union_name_test.Bar.BarT'] + + @classmethod + def InitFromBuf(cls, buf, pos): + container = Container() + container.Init(buf, pos) + return cls.InitFromObj(container) + + @classmethod + def InitFromPackedBuf(cls, buf, pos=0): + n = flatbuffers.encode.Get(flatbuffers.packer.uoffset, buf, pos) + return cls.InitFromBuf(buf, pos+n) + + @classmethod + def InitFromObj(cls, container): + x = ContainerT() + x._UnPack(container) + return x + + # ContainerT + def _UnPack(self, container): + if container is None: + return + self.uType = container.UType() + self.u = union_name_test.my_test_union.MyTestUnionCreator(self.uType, container.U()) + + # ContainerT + def Pack(self, builder): + if self.u is not None: + u = self.u.Pack(builder) + ContainerStart(builder) + ContainerAddUType(builder, self.uType) + if self.u is not None: + ContainerAddU(builder, u) + container = ContainerEnd(builder) + return container diff --git a/tests/union_name_test/Foo.py b/tests/union_name_test/Foo.py new file mode 100644 index 00000000000..f55f6e111ea --- /dev/null +++ b/tests/union_name_test/Foo.py @@ -0,0 +1,90 @@ +# automatically generated by the FlatBuffers compiler, do not modify + +# namespace: union_name_test + +import flatbuffers +from flatbuffers.compat import import_numpy +np = import_numpy() + +class Foo(object): + __slots__ = ['_tab'] + + @classmethod + def GetRootAs(cls, buf, offset=0): + n = flatbuffers.encode.Get(flatbuffers.packer.uoffset, buf, offset) + x = Foo() + x.Init(buf, n + offset) + return x + + @classmethod + def GetRootAsFoo(cls, buf, offset=0): + """This method is deprecated. Please switch to GetRootAs.""" + return cls.GetRootAs(buf, offset) + # Foo + def Init(self, buf, pos): + self._tab = flatbuffers.table.Table(buf, pos) + + # Foo + def Val(self): + o = flatbuffers.number_types.UOffsetTFlags.py_type(self._tab.Offset(4)) + if o != 0: + return self._tab.Get(flatbuffers.number_types.Int32Flags, o + self._tab.Pos) + return 0 + +def FooStart(builder): + builder.StartObject(1) + +def Start(builder): + FooStart(builder) + +def FooAddVal(builder, val): + builder.PrependInt32Slot(0, val, 0) + +def AddVal(builder, val): + FooAddVal(builder, val) + +def FooEnd(builder): + return builder.EndObject() + +def End(builder): + return FooEnd(builder) + + +class FooT(object): + + # FooT + def __init__( + self, + val = 0, + ): + self.val = val # type: int + + @classmethod + def InitFromBuf(cls, buf, pos): + foo = Foo() + foo.Init(buf, pos) + return cls.InitFromObj(foo) + + @classmethod + def InitFromPackedBuf(cls, buf, pos=0): + n = flatbuffers.encode.Get(flatbuffers.packer.uoffset, buf, pos) + return cls.InitFromBuf(buf, pos+n) + + @classmethod + def InitFromObj(cls, foo): + x = FooT() + x._UnPack(foo) + return x + + # FooT + def _UnPack(self, foo): + if foo is None: + return + self.val = foo.Val() + + # FooT + def Pack(self, builder): + FooStart(builder) + FooAddVal(builder, self.val) + foo = FooEnd(builder) + return foo diff --git a/tests/union_name_test/__init__.py b/tests/union_name_test/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/union_name_test/my_test_union.py b/tests/union_name_test/my_test_union.py new file mode 100644 index 00000000000..e62f316c993 --- /dev/null +++ b/tests/union_name_test/my_test_union.py @@ -0,0 +1,20 @@ +# automatically generated by the FlatBuffers compiler, do not modify + +# namespace: union_name_test + +class my_test_union(object): + NONE = 0 + Foo = 1 + Bar = 2 + +def MyTestUnionCreator(unionType, table): + from flatbuffers.table import Table + if not isinstance(table, Table): + return None + if unionType == my_test_union.Foo: + import union_name_test.Foo + return union_name_test.Foo.FooT.InitFromBuf(table.Bytes, table.Pos) + if unionType == my_test_union.Bar: + import union_name_test.Bar + return union_name_test.Bar.BarT.InitFromBuf(table.Bytes, table.Pos) + return None