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 <derekbailey@google.com>

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,

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 {
+    bool operator()(const Definition *a, const Definition *b) const {
-      return *a < *b;
+      return *a->defined_namespace < *b->defined_namespace;
     }
   };
-  std::set<const Namespace *, NamespacePtrLess> tracked_imported_namespaces_;
+  std::set<const Definition *, NamespacePtrLess> 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,
+    return WrapInNameSpaceAndTrack(&enum_def, namer_.Type(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,
+              WrapInNameSpaceAndTrack(&enum_def, NativeName(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,
+      return "*" +
-                                           NativeName(*type.enum_def));
+             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) + " \"" +
+          if ((*it)->defined_namespace->components.empty()) {
-                  NamespaceImportPath(*it) + "\"\n";
+            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;
+    if (CurrentNameSpace() == def->defined_namespace) return name;
-
+    tracked_imported_namespaces_.insert(def);
-    tracked_imported_namespaces_.insert(ns);
+    if (def->defined_namespace->components.empty())
-    return NamespaceImportName(ns) + "." + name;
+      return def->name + "." + name;
-  }
+    else
-
+      return NamespaceImportName(def->defined_namespace) + "." + name;
-  std::string WrapInNameSpaceAndTrack(const Definition &def) {
-    return WrapInNameSpaceAndTrack(def.defined_namespace, def.name);
   }
 
   const Namespace *CurrentNameSpace() const { return cur_name_space_; }

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;
   }
 

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

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()
+}

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)
 

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);
+}

tests/include_test/sub/no_namespace.fbs

@@ -0,0 +1,3 @@
+table Pizza {
+  size: int;
+}

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()
+}