From 1cba8b2b49a808a6b1bbd72fb0b3cbc27fb79f46 Mon Sep 17 00:00:00 2001 From: Saman <100295082+enum-class@users.noreply.github.com> Date: Tue, 22 Nov 2022 16:21:25 -0500 Subject: [PATCH] Fix go generator undefined Package name, also throwing exception (#7632) * Fix go generator undefined Package, also throw exception in specific examples. * Add test for go generator import problem * Add new version of generated go file. Fix conflict. * Add executable permission to generate_code.py script. * Improve test quality, remove unwanted generated files, better naming * Fix comments * clang format Co-authored-by: Derek Bailey --- scripts/generate_code.py | 14 ++++ src/idl_gen_go.cpp | 57 +++++++------ src/namer.h | 2 +- tests/GoTest.sh | 16 +--- tests/Pizza.go | 78 ++++++++++++++++++ tests/go_test.go | 23 ++++++ tests/include_test/order.fbs | 8 ++ tests/include_test/sub/no_namespace.fbs | 3 + tests/order/Food.go | 102 ++++++++++++++++++++++++ 9 files changed, 264 insertions(+), 39 deletions(-) create mode 100644 tests/Pizza.go create mode 100644 tests/include_test/order.fbs create mode 100644 tests/include_test/sub/no_namespace.fbs create mode 100644 tests/order/Food.go diff --git a/scripts/generate_code.py b/scripts/generate_code.py index 1a8d2f1e8..c98169329 100755 --- a/scripts/generate_code.py +++ b/scripts/generate_code.py @@ -193,6 +193,20 @@ flatc( include="include_test", ) +flatc( + NO_INCL_OPTS + + ["--go"], + schema="include_test/foo.fbs", + include="include_test/sub", +) + +flatc( + NO_INCL_OPTS + + ["--go"], + schema="include_test/sub/header.fbs", + include="include_test", +) + flatc( NO_INCL_OPTS + TS_OPTS, diff --git a/src/idl_gen_go.cpp b/src/idl_gen_go.cpp index d5d3c43d0..a5e0c364f 100644 --- a/src/idl_gen_go.cpp +++ b/src/idl_gen_go.cpp @@ -79,7 +79,7 @@ static Namer::Config GoDefaultConfig() { /*filename_extension=*/".go" }; } -} // namespace +} // namespace class GoGenerator : public BaseGenerator { public: @@ -152,11 +152,11 @@ class GoGenerator : public BaseGenerator { const IdlNamer namer_; struct NamespacePtrLess { - bool operator()(const Namespace *a, const Namespace *b) const { - return *a < *b; + bool operator()(const Definition *a, const Definition *b) const { + return *a->defined_namespace < *b->defined_namespace; } }; - std::set tracked_imported_namespaces_; + std::set tracked_imported_namespaces_; bool needs_math_import_ = false; // Most field accessors need to retrieve and test the field offset first, @@ -180,8 +180,7 @@ class GoGenerator : public BaseGenerator { // Construct the name of the type for this enum. std::string GetEnumTypeName(const EnumDef &enum_def) { - return WrapInNameSpaceAndTrack(enum_def.defined_namespace, - namer_.Type(enum_def)); + return WrapInNameSpaceAndTrack(&enum_def, namer_.Type(enum_def)); } // Create a type for the enum values. @@ -907,13 +906,13 @@ class GoGenerator : public BaseGenerator { if (ev.IsZero()) continue; code += "\tcase " + namer_.EnumVariant(enum_def, ev) + ":\n"; code += "\t\tvar x " + - WrapInNameSpaceAndTrack(*ev.union_type.struct_def) + + WrapInNameSpaceAndTrack(ev.union_type.struct_def, + ev.union_type.struct_def->name) + "\n"; code += "\t\tx.Init(table.Bytes, table.Pos)\n"; code += "\t\treturn &" + - WrapInNameSpaceAndTrack(enum_def.defined_namespace, - NativeName(enum_def)) + + WrapInNameSpaceAndTrack(&enum_def, NativeName(enum_def)) + "{ Type: " + namer_.EnumVariant(enum_def, ev) + ", Value: x.UnPack() }\n"; } @@ -1074,7 +1073,8 @@ class GoGenerator : public BaseGenerator { code += "\tfor j := 0; j < " + length + "; j++ {\n"; if (field.value.type.element == BASE_TYPE_STRUCT) { code += "\t\tx := " + - WrapInNameSpaceAndTrack(*field.value.type.struct_def) + + WrapInNameSpaceAndTrack(field.value.type.struct_def, + field.value.type.struct_def->name) + "{}\n"; code += "\t\trcv." + field_field + "(&x, j)\n"; } @@ -1241,7 +1241,8 @@ class GoGenerator : public BaseGenerator { switch (type.base_type) { case BASE_TYPE_STRING: return "[]byte"; case BASE_TYPE_VECTOR: return GenTypeGet(type.VectorType()); - case BASE_TYPE_STRUCT: return WrapInNameSpaceAndTrack(*type.struct_def); + case BASE_TYPE_STRUCT: + return WrapInNameSpaceAndTrack(type.struct_def, type.struct_def->name); case BASE_TYPE_UNION: // fall through default: return "*flatbuffers.Table"; @@ -1325,11 +1326,11 @@ class GoGenerator : public BaseGenerator { } else if (IsVector(type)) { return "[]" + NativeType(type.VectorType()); } else if (type.base_type == BASE_TYPE_STRUCT) { - return "*" + WrapInNameSpaceAndTrack(type.struct_def->defined_namespace, + return "*" + WrapInNameSpaceAndTrack(type.struct_def, NativeName(*type.struct_def)); } else if (type.base_type == BASE_TYPE_UNION) { - return "*" + WrapInNameSpaceAndTrack(type.enum_def->defined_namespace, - NativeName(*type.enum_def)); + return "*" + + WrapInNameSpaceAndTrack(type.enum_def, NativeName(*type.enum_def)); } FLATBUFFERS_ASSERT(0); return std::string(); @@ -1365,8 +1366,13 @@ class GoGenerator : public BaseGenerator { code += "\n"; for (auto it = tracked_imported_namespaces_.begin(); it != tracked_imported_namespaces_.end(); ++it) { - code += "\t" + NamespaceImportName(*it) + " \"" + - NamespaceImportPath(*it) + "\"\n"; + if ((*it)->defined_namespace->components.empty()) { + code += "\t" + (*it)->name + " \"" + (*it)->name + "\"\n"; + } else { + code += "\t" + NamespaceImportName((*it)->defined_namespace) + + " \"" + NamespaceImportPath((*it)->defined_namespace) + + "\"\n"; + } } } code += ")\n\n"; @@ -1387,7 +1393,8 @@ class GoGenerator : public BaseGenerator { Namespace &ns = go_namespace_.components.empty() ? *def.defined_namespace : go_namespace_; std::string code = ""; - BeginFile(LastNamespacePart(ns), needs_imports, is_enum, &code); + BeginFile(ns.components.empty() ? def.name : LastNamespacePart(ns), + needs_imports, is_enum, &code); code += classcode; // Strip extra newlines at end of file to make it gofmt-clean. while (code.length() > 2 && code.substr(code.length() - 2) == "\n\n") { @@ -1412,16 +1419,14 @@ class GoGenerator : public BaseGenerator { // Ensure that a type is prefixed with its go package import name if it is // used outside of its namespace. - std::string WrapInNameSpaceAndTrack(const Namespace *ns, + std::string WrapInNameSpaceAndTrack(const Definition *def, const std::string &name) { - if (CurrentNameSpace() == ns) return name; - - tracked_imported_namespaces_.insert(ns); - return NamespaceImportName(ns) + "." + name; - } - - std::string WrapInNameSpaceAndTrack(const Definition &def) { - return WrapInNameSpaceAndTrack(def.defined_namespace, def.name); + if (CurrentNameSpace() == def->defined_namespace) return name; + tracked_imported_namespaces_.insert(def); + if (def->defined_namespace->components.empty()) + return def->name + "." + name; + else + return NamespaceImportName(def->defined_namespace) + "." + name; } const Namespace *CurrentNameSpace() const { return cur_name_space_; } diff --git a/src/namer.h b/src/namer.h index 8fd8354e1..6a7fadcd1 100644 --- a/src/namer.h +++ b/src/namer.h @@ -196,7 +196,7 @@ class Namer { result += ConvertCase(*d, config_.directories, Case::kUpperCamel); result.push_back(kPathSeparator); } - if (skip_trailing_seperator) result.pop_back(); + if (skip_trailing_seperator && !result.empty()) result.pop_back(); return result; } diff --git a/tests/GoTest.sh b/tests/GoTest.sh index 85253c177..8e73af241 100755 --- a/tests/GoTest.sh +++ b/tests/GoTest.sh @@ -20,26 +20,18 @@ go_path=${test_dir}/go_gen go_src=${go_path}/src # Emit Go code for the example schemas in the test dir: -../flatc -g --gen-object-api -I include_test monster_test.fbs optional_scalars.fbs +../flatc -g --gen-object-api -I include_test -o ${go_src} monster_test.fbs optional_scalars.fbs +../flatc -g --gen-object-api -I include_test/sub -o ${go_src} include_test/order.fbs +../flatc -g --gen-object-api -o ${go_src}/Pizza include_test/sub/no_namespace.fbs # Go requires a particular layout of files in order to link multiple packages. # Copy flatbuffer Go files to their own package directories to compile the # test binary: -mkdir -p ${go_src}/MyGame/Example -mkdir -p ${go_src}/MyGame/Example2 mkdir -p ${go_src}/github.com/google/flatbuffers/go mkdir -p ${go_src}/flatbuffers_test -mkdir -p ${go_src}/optional_scalars -cp -a MyGame/*.go ./go_gen/src/MyGame/ -cp -a MyGame/Example/*.go ./go_gen/src/MyGame/Example/ -cp -a MyGame/Example2/*.go ./go_gen/src/MyGame/Example2/ -# do not compile the gRPC generated files, which are not tested by go_test.go -# below, but have their own test. -rm ./go_gen/src/MyGame/Example/*_grpc.go cp -a ../go/* ./go_gen/src/github.com/google/flatbuffers/go cp -a ./go_test.go ./go_gen/src/flatbuffers_test/ -cp -a optional_scalars/*.go ./go_gen/src/optional_scalars # https://stackoverflow.com/a/63545857/7024978 # We need to turn off go modules for this script @@ -72,7 +64,7 @@ else exit 1 fi -NOT_FMT_FILES=$(gofmt -l MyGame) +NOT_FMT_FILES=$(gofmt -l .) if [[ ${NOT_FMT_FILES} != "" ]]; then echo "These files are not well gofmt'ed:" echo diff --git a/tests/Pizza.go b/tests/Pizza.go new file mode 100644 index 000000000..08df9e16b --- /dev/null +++ b/tests/Pizza.go @@ -0,0 +1,78 @@ +// Code generated by the FlatBuffers compiler. DO NOT EDIT. + +package Pizza + +import ( + flatbuffers "github.com/google/flatbuffers/go" +) + +type PizzaT struct { + Size int32 `json:"size"` +} + +func (t *PizzaT) Pack(builder *flatbuffers.Builder) flatbuffers.UOffsetT { + if t == nil { return 0 } + PizzaStart(builder) + PizzaAddSize(builder, t.Size) + return PizzaEnd(builder) +} + +func (rcv *Pizza) UnPackTo(t *PizzaT) { + t.Size = rcv.Size() +} + +func (rcv *Pizza) UnPack() *PizzaT { + if rcv == nil { return nil } + t := &PizzaT{} + rcv.UnPackTo(t) + return t +} + +type Pizza struct { + _tab flatbuffers.Table +} + +func GetRootAsPizza(buf []byte, offset flatbuffers.UOffsetT) *Pizza { + n := flatbuffers.GetUOffsetT(buf[offset:]) + x := &Pizza{} + x.Init(buf, n+offset) + return x +} + +func GetSizePrefixedRootAsPizza(buf []byte, offset flatbuffers.UOffsetT) *Pizza { + n := flatbuffers.GetUOffsetT(buf[offset+flatbuffers.SizeUint32:]) + x := &Pizza{} + x.Init(buf, n+offset+flatbuffers.SizeUint32) + return x +} + +func (rcv *Pizza) Init(buf []byte, i flatbuffers.UOffsetT) { + rcv._tab.Bytes = buf + rcv._tab.Pos = i +} + +func (rcv *Pizza) Table() flatbuffers.Table { + return rcv._tab +} + +func (rcv *Pizza) Size() int32 { + o := flatbuffers.UOffsetT(rcv._tab.Offset(4)) + if o != 0 { + return rcv._tab.GetInt32(o + rcv._tab.Pos) + } + return 0 +} + +func (rcv *Pizza) MutateSize(n int32) bool { + return rcv._tab.MutateInt32Slot(4, n) +} + +func PizzaStart(builder *flatbuffers.Builder) { + builder.StartObject(1) +} +func PizzaAddSize(builder *flatbuffers.Builder, size int32) { + builder.PrependInt32Slot(0, size, 0) +} +func PizzaEnd(builder *flatbuffers.Builder) flatbuffers.UOffsetT { + return builder.EndObject() +} diff --git a/tests/go_test.go b/tests/go_test.go index a04ef2c9b..d454b5647 100644 --- a/tests/go_test.go +++ b/tests/go_test.go @@ -17,6 +17,8 @@ package main import ( + order "order" + pizza "Pizza" mygame "MyGame" // refers to generated code example "MyGame/Example" // refers to generated code "encoding/json" @@ -98,6 +100,24 @@ func TestTextParsing(t *testing.T) { } } +func CheckNoNamespaceImport(fail func(string, ...interface{})) { + const size = 13 + // Order a pizza with specific size + builder := flatbuffers.NewBuilder(0) + ordered_pizza := pizza.PizzaT{Size: size} + food := order.FoodT{Pizza: &ordered_pizza} + builder.Finish(food.Pack(builder)) + + // Receive order + received_food := order.GetRootAsFood(builder.FinishedBytes(), 0) + received_pizza := received_food.Pizza(nil).UnPack() + + // Check if received pizza is equal to ordered pizza + if !reflect.DeepEqual(ordered_pizza, *received_pizza) { + fail(FailString("no namespace import", ordered_pizza, received_pizza)) + } +} + // TestAll runs all checks, failing if any errors occur. func TestAll(t *testing.T) { // Verify that the Go FlatBuffers runtime library generates the @@ -160,6 +180,9 @@ func TestAll(t *testing.T) { // Check a parent namespace import CheckParentNamespace(t.Fatalf) + // Check a no namespace import + CheckNoNamespaceImport(t.Fatalf) + // Check size-prefixed flatbuffers CheckSizePrefixedBuffer(t.Fatalf) diff --git a/tests/include_test/order.fbs b/tests/include_test/order.fbs new file mode 100644 index 000000000..4588d5fd1 --- /dev/null +++ b/tests/include_test/order.fbs @@ -0,0 +1,8 @@ +include "no_namespace.fbs"; + +namespace order; + +table Food { + pizza: Pizza (id: 0); + pizza_test:Pizza(id:1); +} diff --git a/tests/include_test/sub/no_namespace.fbs b/tests/include_test/sub/no_namespace.fbs new file mode 100644 index 000000000..5f1052d17 --- /dev/null +++ b/tests/include_test/sub/no_namespace.fbs @@ -0,0 +1,3 @@ +table Pizza { + size: int; +} diff --git a/tests/order/Food.go b/tests/order/Food.go new file mode 100644 index 000000000..298d63120 --- /dev/null +++ b/tests/order/Food.go @@ -0,0 +1,102 @@ +// Code generated by the FlatBuffers compiler. DO NOT EDIT. + +package order + +import ( + flatbuffers "github.com/google/flatbuffers/go" + + Pizza "Pizza" +) + +type FoodT struct { + Pizza *Pizza.PizzaT `json:"pizza"` + PizzaTest *Pizza.PizzaT `json:"pizza_test"` +} + +func (t *FoodT) Pack(builder *flatbuffers.Builder) flatbuffers.UOffsetT { + if t == nil { return 0 } + pizzaOffset := t.Pizza.Pack(builder) + pizzaTestOffset := t.PizzaTest.Pack(builder) + FoodStart(builder) + FoodAddPizza(builder, pizzaOffset) + FoodAddPizzaTest(builder, pizzaTestOffset) + return FoodEnd(builder) +} + +func (rcv *Food) UnPackTo(t *FoodT) { + t.Pizza = rcv.Pizza(nil).UnPack() + t.PizzaTest = rcv.PizzaTest(nil).UnPack() +} + +func (rcv *Food) UnPack() *FoodT { + if rcv == nil { return nil } + t := &FoodT{} + rcv.UnPackTo(t) + return t +} + +type Food struct { + _tab flatbuffers.Table +} + +func GetRootAsFood(buf []byte, offset flatbuffers.UOffsetT) *Food { + n := flatbuffers.GetUOffsetT(buf[offset:]) + x := &Food{} + x.Init(buf, n+offset) + return x +} + +func GetSizePrefixedRootAsFood(buf []byte, offset flatbuffers.UOffsetT) *Food { + n := flatbuffers.GetUOffsetT(buf[offset+flatbuffers.SizeUint32:]) + x := &Food{} + x.Init(buf, n+offset+flatbuffers.SizeUint32) + return x +} + +func (rcv *Food) Init(buf []byte, i flatbuffers.UOffsetT) { + rcv._tab.Bytes = buf + rcv._tab.Pos = i +} + +func (rcv *Food) Table() flatbuffers.Table { + return rcv._tab +} + +func (rcv *Food) Pizza(obj *Pizza.Pizza) *Pizza.Pizza { + o := flatbuffers.UOffsetT(rcv._tab.Offset(4)) + if o != 0 { + x := rcv._tab.Indirect(o + rcv._tab.Pos) + if obj == nil { + obj = new(Pizza.Pizza) + } + obj.Init(rcv._tab.Bytes, x) + return obj + } + return nil +} + +func (rcv *Food) PizzaTest(obj *Pizza.Pizza) *Pizza.Pizza { + o := flatbuffers.UOffsetT(rcv._tab.Offset(6)) + if o != 0 { + x := rcv._tab.Indirect(o + rcv._tab.Pos) + if obj == nil { + obj = new(Pizza.Pizza) + } + obj.Init(rcv._tab.Bytes, x) + return obj + } + return nil +} + +func FoodStart(builder *flatbuffers.Builder) { + builder.StartObject(2) +} +func FoodAddPizza(builder *flatbuffers.Builder, pizza flatbuffers.UOffsetT) { + builder.PrependUOffsetTSlot(0, flatbuffers.UOffsetT(pizza), 0) +} +func FoodAddPizzaTest(builder *flatbuffers.Builder, pizzaTest flatbuffers.UOffsetT) { + builder.PrependUOffsetTSlot(1, flatbuffers.UOffsetT(pizzaTest), 0) +} +func FoodEnd(builder *flatbuffers.Builder) flatbuffers.UOffsetT { + return builder.EndObject() +}