From 7b4a810969c6cb063fb28eea4738d9991833adb8 Mon Sep 17 00:00:00 2001
From: Dmytro Shteflyuk <kpumuk@apache.org>
Date: Mon, 11 May 2026 19:05:03 -0400
Subject: [PATCH] THRIFT-6011: Make compiled Go code formatting compatible with
 gofmt

---
 .github/workflows/sca.yml                     |  47 +-
 .../thrift/generate/go_validator_generator.cc |  40 +-
 .../cpp/src/thrift/generate/t_go_generator.cc | 534 +++++++++++++-----
 .../cpp/src/thrift/generate/t_go_generator.h  |   5 +
 4 files changed, 464 insertions(+), 162 deletions(-)

diff --git a/.github/workflows/sca.yml b/.github/workflows/sca.yml
index 59bc4b57a39..2009e2d0113 100644
--- a/.github/workflows/sca.yml
+++ b/.github/workflows/sca.yml
@@ -202,12 +202,53 @@ jobs:
         with:
           go-version: ${{ matrix.go }}
 
-      - name: Run gofmt
+      - name: Install dependencies
+        run: |
+          sudo apt-get update -yq
+          # shellcheck disable=SC2086
+          sudo apt-get install -y --no-install-recommends g++ $BUILD_DEPS
+
+      - name: Configure
+        run: |
+          ./bootstrap.sh
+          # shellcheck disable=SC2086
+          ./configure $(echo $CONFIG_ARGS_FOR_SCA | sed 's/without-go/with-go/')
+
+      - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
+        with:
+          name: thrift-compiler
+          path: compiler/cpp
+
+      - name: Run gofmt on checked-in Go files
         # Args:
         # -s: Simplify code
-        # -d: Display changes, and exit non-zero when changes present
+        # -l: List files that would change
         # -e: Report all errors instead of only the first 10
-        run: gofmt -s -d -e $(git ls-files | grep "\.go$")
+        run: |
+          git ls-files '*.go' > /tmp/go-files.txt
+          xargs -r gofmt -s -l -e < /tmp/go-files.txt > /tmp/gofmt-files.txt
+          cat /tmp/gofmt-files.txt
+          test ! -s /tmp/gofmt-files.txt
+
+      - name: Run gofmt on generated Go files
+        run: |
+          chmod a+x compiler/cpp/thrift
+          thrift_compiler="$PWD/compiler/cpp/thrift"
+
+          make -C lib/go/test THRIFT="$thrift_compiler" gopath
+          make -C test/go THRIFT="$thrift_compiler" gopath
+          make -C tutorial/go THRIFT="$thrift_compiler" all-local
+
+          {
+            find lib/go/test/gopath/src \
+              -path lib/go/test/gopath/src/dontexportrwtest -prune -o \
+              -name '*.go' -type f -print
+            find test/go/src/gen tutorial/go/gen-go -name '*.go' -type f -print
+          } > /tmp/generated-go-files.txt
+
+          xargs -r gofmt -l -e < /tmp/generated-go-files.txt > /tmp/generated-gofmt-files.txt
+          cat /tmp/generated-gofmt-files.txt
+          test ! -s /tmp/generated-gofmt-files.txt
 
   lib-python:
     needs: compiler
diff --git a/compiler/cpp/src/thrift/generate/go_validator_generator.cc b/compiler/cpp/src/thrift/generate/go_validator_generator.cc
index 0adff2ee984..8cf828a6755 100644
--- a/compiler/cpp/src/thrift/generate/go_validator_generator.cc
+++ b/compiler/cpp/src/thrift/generate/go_validator_generator.cc
@@ -239,11 +239,11 @@ void go_validator_generator::generate_enum_field_validator(std::ostream& out,
       }
     } else if (key == "vt.defined_only") {
       if (values[0]->get_bool()) {
-        out << indent() << "if (" << context.tgt << ").String() == \"<UNSET>\" ";
+        out << indent() << "if (" << context.tgt << ").String() == \"<UNSET>\"";
       } else {
         continue;
       }
-      out << "{" << '\n';
+      out << " {" << '\n';
       indent_up();
       out << indent()
           << "return thrift.NewValidationException(thrift.VALIDATION_FAILED, \"" + key + "\", \""
@@ -276,7 +276,7 @@ void go_validator_generator::generate_bool_field_validator(std::ostream& out,
         }
       }
     }
-    out << "{" << '\n';
+    out << " {" << '\n';
     indent_up();
     out << indent()
         << "return thrift.NewValidationException(thrift.VALIDATION_FAILED, \"" + key + "\", \""
@@ -308,7 +308,7 @@ void go_validator_generator::generate_double_field_validator(std::ostream& out,
       } else {
         out << values[0]->get_double();
       }
-      out << "{" << '\n';
+      out << " {" << '\n';
       indent_up();
       out << indent()
           << "return thrift.NewValidationException(thrift.VALIDATION_FAILED, \"" + key + "\", \""
@@ -354,7 +354,7 @@ void go_validator_generator::generate_double_field_validator(std::ostream& out,
         } else {
           out << values[0]->get_double();
         }
-        out << "{" << '\n';
+        out << " {" << '\n';
       }
 
       indent_up();
@@ -390,7 +390,7 @@ void go_validator_generator::generate_double_field_validator(std::ostream& out,
         } else {
           out << values[0]->get_double();
         }
-        out << "{" << '\n';
+        out << " {" << '\n';
       }
       indent_up();
       out << indent()
@@ -460,7 +460,7 @@ void go_validator_generator::generate_integer_field_validator(std::ostream& out,
       } else {
         out << values[0]->get_int();
       }
-      out << "{" << '\n';
+      out << " {" << '\n';
       indent_up();
       out << indent()
           << "return thrift.NewValidationException(thrift.VALIDATION_FAILED, \"" + key + "\", \""
@@ -531,7 +531,7 @@ void go_validator_generator::generate_integer_field_validator(std::ostream& out,
         } else {
           out << values[0]->get_int();
         }
-        out << "{" << '\n';
+        out << " {" << '\n';
       }
       indent_up();
       out << indent()
@@ -608,7 +608,7 @@ void go_validator_generator::generate_integer_field_validator(std::ostream& out,
         } else {
           out << values[0]->get_int();
         }
-        out << "{" << '\n';
+        out << " {" << '\n';
       }
       indent_up();
       out << indent()
@@ -679,7 +679,7 @@ void go_validator_generator::generate_string_field_validator(std::ostream& out,
       }
       out << ")";
     } else if (key == "vt.pattern") {
-      out << indent() << "if ok, _ := regexp.MatchString(" << target << ",";
+      out << indent() << "if ok, _ := regexp.MatchString(" << target << ", ";
       if (values[0]->is_field_reference()) {
         out << "string(";
         out << get_field_reference_name(values[0]->get_field_reference());
@@ -687,9 +687,9 @@ void go_validator_generator::generate_string_field_validator(std::ostream& out,
       } else {
         out << "\"" << values[0]->get_string() << "\"";
       }
-      out << "); ok ";
+      out << "); ok";
     } else if (key == "vt.prefix") {
-      out << indent() << "if !strings.HasPrefix(" << target << ",";
+      out << indent() << "if !strings.HasPrefix(" << target << ", ";
       if (values[0]->is_field_reference()) {
         out << "string(";
         out << get_field_reference_name(values[0]->get_field_reference());
@@ -699,7 +699,7 @@ void go_validator_generator::generate_string_field_validator(std::ostream& out,
       }
       out << ")";
     } else if (key == "vt.suffix") {
-      out << indent() << "if !strings.HasSuffix(" << target << ",";
+      out << indent() << "if !strings.HasSuffix(" << target << ", ";
       if (values[0]->is_field_reference()) {
         out << "string(";
         out << get_field_reference_name(values[0]->get_field_reference());
@@ -709,7 +709,7 @@ void go_validator_generator::generate_string_field_validator(std::ostream& out,
       }
       out << ")";
     } else if (key == "vt.contains") {
-      out << indent() << "if !strings.Contains(" << target << ",";
+      out << indent() << "if !strings.Contains(" << target << ", ";
       if (values[0]->is_field_reference()) {
         out << "string(";
         out << get_field_reference_name(values[0]->get_field_reference());
@@ -719,7 +719,7 @@ void go_validator_generator::generate_string_field_validator(std::ostream& out,
       }
       out << ")";
     } else if (key == "vt.not_contains") {
-      out << indent() << "if strings.Contains(" << target << ",";
+      out << indent() << "if strings.Contains(" << target << ", ";
       if (values[0]->is_field_reference()) {
         out << "string(";
         out << get_field_reference_name(values[0]->get_field_reference());
@@ -729,7 +729,7 @@ void go_validator_generator::generate_string_field_validator(std::ostream& out,
       }
       out << ")";
     }
-    out << "{" << '\n';
+    out << " {" << '\n';
     indent_up();
     out << indent()
         << "return thrift.NewValidationException(thrift.VALIDATION_FAILED, \"" + key + "\", \""
@@ -764,7 +764,7 @@ void go_validator_generator::generate_list_field_validator(std::ostream& out,
       } else {
         out << values[0]->get_int();
       }
-      out << "{" << '\n';
+      out << " {" << '\n';
       indent_up();
       out << indent()
           << "return thrift.NewValidationException(thrift.VALIDATION_FAILED, \"" + key + "\", \""
@@ -773,7 +773,7 @@ void go_validator_generator::generate_list_field_validator(std::ostream& out,
       indent_down();
       out << indent() << "}" << '\n';
     } else if (key == "vt.elem") {
-      out << indent() << "for i := 0; i < len(" << context.tgt << ");i++ {" << '\n';
+      out << indent() << "for i := 0; i < len(" << context.tgt << "); i++ {" << '\n';
       indent_up();
       std::string src = GenID("_elem");
       out << indent() << src << " := " << context.tgt << "[i]" << '\n';
@@ -815,7 +815,7 @@ void go_validator_generator::generate_map_field_validator(std::ostream& out,
       } else {
         out << values[0]->get_int();
       }
-      out << "{" << '\n';
+      out << " {" << '\n';
       indent_up();
       out << indent()
           << "return thrift.NewValidationException(thrift.VALIDATION_FAILED, \"" + key + "\", \""
@@ -891,7 +891,7 @@ void go_validator_generator::generate_struct_field_validator(std::ostream& out,
       } else if (values[0]->is_field_reference()) {
         out << indent() << "if !";
         out << get_field_reference_name(values[0]->get_field_reference());
-        out << "{" << '\n';
+        out << " {" << '\n';
         indent_up();
         out << indent() << "if err := " << context.tgt << ".Validate(); err != nil {" << '\n';
         indent_up();
diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc b/compiler/cpp/src/thrift/generate/t_go_generator.cc
index bef28553632..805df860abf 100644
--- a/compiler/cpp/src/thrift/generate/t_go_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_go_generator.cc
@@ -119,6 +119,13 @@ static bool type_need_reference(t_type* type) {
   return true;
 }
 
+static string indexable_go_expr(const string& expr) {
+  if (!expr.empty() && expr[0] == '*') {
+    return "(" + expr + ")";
+  }
+  return expr;
+}
+
 // returns false if field could not use comparison to default value as !IsSet*
 bool t_go_generator::is_pointer_field(t_field* tfield, bool in_container_value) {
   (void)in_container_value;
@@ -419,6 +426,17 @@ std::string t_go_generator::variable_name_to_go_name(const std::string& value) {
   return value2 + "_a1";
 }
 
+std::string t_go_generator::package_name_to_go_name(const std::string& value) {
+  // Go package names only need to avoid keywords; "error" remains a valid package name.
+  if (value == "error") {
+    return value;
+  }
+  if (std::all_of(value.begin(), value.end(), ::islower)) {
+    return variable_name_to_go_name(value);
+  }
+  return value;
+}
+
 /**
  * Prepares for file generation by opening up the necessary file output
  * streams.
@@ -430,6 +448,7 @@ void t_go_generator::init_generator() {
   string module = get_real_go_module(program_);
   string target = module;
   package_dir_ = get_out_dir();
+  last_const_block_ = 0;
 
   // This set is taken from https://github.com/golang/lint/blob/master/lint.go#L692
   commonInitialisms.insert("API");
@@ -491,7 +510,7 @@ void t_go_generator::init_generator() {
     if (pos == string::npos) {
       package_dir_ += "/";
       package_dir_ += module;
-      package_name_ = module;
+      package_name_ = package_name_to_go_name(module);
       module.clear();
     } else {
       package_dir_ += "/";
@@ -515,10 +534,12 @@ void t_go_generator::init_generator() {
 
   // Print header
   f_types_ << go_autogen_comment() << go_package() << render_includes(false);
+  types_file_has_declaration_ = true;
 
   f_consts_ << go_autogen_comment() << go_package() << render_includes(true);
+  f_consts_ << '\n';
 
-  f_const_values_ << '\n' << "func init() {" << '\n';
+  f_const_values_ << "func init() {" << '\n';
 
   // Create file for the GoUnusedProtection__ variable
   string f_unused_prot_name_ = package_dir_ + "/" + "GoUnusedProtection__.go";
@@ -532,18 +553,18 @@ string t_go_generator::render_included_programs(string& unused_prot) {
   const vector<t_program*>& includes = program_->get_includes();
   string result = "";
   string local_namespace = get_real_go_module(program_);
-  std::set<std::string> included;
+  std::map<std::string, const t_program*> included;
   for (auto include : includes) {
     std::string includeModule = get_real_go_module(include);
     if (!local_namespace.empty() && local_namespace == includeModule) {
       continue;
     }
 
-    if (!included.insert(includeModule).second) {
-        continue;
-    }
+    included.emplace(includeModule, include);
+  }
 
-    result += render_program_import(include, unused_prot);
+  for (auto const& include : included) {
+    result += render_program_import(include.second, unused_prot);
   }
   return result;
 }
@@ -565,7 +586,7 @@ string t_go_generator::render_program_import(const t_program* program, string& u
   auto it = package_identifiers_.find(go_module);
   auto last_component = go_module.substr(found);
   if (it == package_identifiers_.end()) {
-    auto value = last_component;
+    auto value = variable_name_to_go_name(last_component);
     // This final path component has already been used, let's construct a more unique alias
     if (package_identifiers_set_.find(value) != package_identifiers_set_.end()) {
       // TODO: This would produce more readable code if it appended trailing go_module
@@ -583,16 +604,6 @@ string t_go_generator::render_program_import(const t_program* program, string& u
   if (last_component.compare(package_identifier) != 0) {
     result += package_identifier + " ";
   }
-  string s;
-
-  for (auto const& e : package_identifiers_set_)
-  {
-      s += e;
-      s += ',';
-  }
-
-  s.pop_back();
-
   result += "\"" + gen_package_prefix_ + go_path + "\"\n";
   unused_protection += "var _ = " + package_identifier + ".GoUnusedProtection__\n";
   return result;
@@ -651,21 +662,22 @@ string t_go_generator::render_system_packages(std::vector<string>& system_packag
  * If consts include the additional imports.
  */
 string t_go_generator::render_includes(bool consts) {
-  const vector<t_program*>& includes = program_->get_includes();
-  string result = "";
-  string unused_prot = "";
+  string result;
+  string unused_prot;
   result += go_imports_begin(consts);
-  result += render_included_programs(unused_prot);
-
-  if (includes.size() > 0) {
-    result += "\n";
+  std::vector<string> thrift_package;
+  thrift_package.push_back("thrift \"" + gen_thrift_import_ + "\"");
+  result += "\n" + render_system_packages(thrift_package);
+  string included_programs = render_included_programs(unused_prot);
+  if (!included_programs.empty()) {
+    result += "\n" + included_programs;
   }
 
   return result + go_imports_end() + unused_prot;
 }
 
 string t_go_generator::render_import_protection() {
-  return string("var GoUnusedProtection__ int;\n\n");
+  return string("var GoUnusedProtection__ int\n");
 }
 
 /**
@@ -708,13 +720,11 @@ string t_go_generator::go_imports_begin(bool consts) {
   system_packages.push_back("fmt");
   system_packages.push_back("iter");
   system_packages.push_back("log/slog");
-  system_packages.push_back("time");
-  // For the thrift import, always do rename import to make sure it's called thrift.
-  system_packages.push_back("thrift \"" + gen_thrift_import_ + "\"");
 
   // validator import
-  system_packages.push_back("strings");
   system_packages.push_back("regexp");
+  system_packages.push_back("strings");
+  system_packages.push_back("time");
   return "import (\n" + render_system_packages(system_packages);
 }
 
@@ -725,7 +735,7 @@ string t_go_generator::go_imports_begin(bool consts) {
  * This will have to do in lieu of more intelligent import statement construction
  */
 string t_go_generator::go_imports_end() {
-  string import_end = string(
+  return string(
       ")\n\n"
       "// (needed to ensure safety because of naive import list construction.)\n"
       "var _ = bytes.Equal\n"
@@ -736,17 +746,27 @@ string t_go_generator::go_imports_end() {
       "var _ = slog.Log\n"
       "var _ = time.Now\n"
       "var _ = thrift.ZERO\n"
+      "\n"
       "// (needed by validator.)\n"
       "var _ = strings.Contains\n"
-      "var _ = regexp.MatchString\n\n");
-  return import_end;
+      "var _ = regexp.MatchString\n");
+}
+
+void t_go_generator::begin_types_declaration() {
+  if (types_file_has_declaration_) {
+    f_types_ << '\n';
+  }
+  types_file_has_declaration_ = true;
 }
 
 /**
  * Closes the type files
  */
 void t_go_generator::close_generator() {
-  f_const_values_ << "}" << '\n' << '\n';
+  f_const_values_ << "}" << '\n';
+  if (!get_program()->get_consts().empty()) {
+    f_consts_ << '\n';
+  }
   f_consts_ << f_const_values_.str();
 
   // Close types and constants files
@@ -762,7 +782,6 @@ void t_go_generator::close_generator() {
  * @param ttypedef The type definition
  */
 void t_go_generator::generate_typedef(t_typedef* ttypedef) {
-  generate_go_docstring(f_types_, ttypedef);
   string new_type_name(publicize(ttypedef->get_symbolic()));
   string base_type(type_to_go_type(ttypedef->get_type()));
 
@@ -770,13 +789,16 @@ void t_go_generator::generate_typedef(t_typedef* ttypedef) {
     return;
   }
 
+  begin_types_declaration();
+  generate_go_docstring(f_types_, ttypedef);
   generate_deprecation_comment(f_types_, ttypedef->annotations_);
-  f_types_ << "type " << new_type_name << " " << base_type << '\n' << '\n';
+  f_types_ << "type " << new_type_name << " " << base_type << '\n';
   // Generate a convenience function that converts an instance of a type
   // (which may be a constant) into a pointer to an instance of a type.
+  f_types_ << '\n';
   generate_deprecation_comment(f_types_, ttypedef->annotations_);
   f_types_ << "func " << new_type_name << "Ptr(v " << new_type_name << ") *" << new_type_name
-           << " { return &v }" << '\n' << '\n';
+           << " { return &v }" << '\n';
 }
 
 /**
@@ -786,11 +808,12 @@ void t_go_generator::generate_typedef(t_typedef* ttypedef) {
  * @param tenum The enumeration
  */
 void t_go_generator::generate_enum(t_enum* tenum) {
+  begin_types_declaration();
   std::ostringstream to_string_mapping, from_string_mapping, known_values_mapping;
   std::string tenum_name(publicize(tenum->get_name()));
   generate_go_docstring(f_types_, tenum);
   generate_deprecation_comment(f_types_, tenum->annotations_);
-  f_types_ << "type " << tenum_name << " int64" << '\n' << '\n' << "const (" << '\n';
+  f_types_ << "type " << tenum_name << " int64" << '\n' << '\n';
 
   known_values_mapping << indent() << "var known" << tenum_name << "Values" << " = []" << tenum_name << "{" << '\n';
 
@@ -809,29 +832,43 @@ void t_go_generator::generate_enum(t_enum* tenum) {
   vector<t_enum_value*> constants = tenum->get_constants();
   vector<t_enum_value*>::iterator c_iter;
   int value = -1;
+  size_t max_enum_name_len = 0;
+  for (c_iter = constants.begin(); c_iter != constants.end(); ++c_iter) {
+    max_enum_name_len = (std::max)(max_enum_name_len,
+                                   (tenum_name + "_" + (*c_iter)->get_name()).size());
+  }
 
+  if (constants.empty()) {
+    f_types_ << "const ()" << '\n' << '\n';
+  } else {
+    f_types_ << "const (" << '\n';
+  }
   indent_up();
   for (c_iter = constants.begin(); c_iter != constants.end(); ++c_iter) {
     value = (*c_iter)->get_value();
 
     string iter_std_name(escape_string((*c_iter)->get_name()));
     string iter_name((*c_iter)->get_name());
+    string go_enum_name = tenum_name + "_" + iter_name;
     generate_deprecation_comment(f_types_, (*c_iter)->annotations_);
-    f_types_ << indent() << tenum_name << "_" << iter_name << ' ' << tenum_name << " = "
+    f_types_ << indent() << go_enum_name << string(max_enum_name_len - go_enum_name.size() + 1, ' ') << tenum_name << " = "
              << value << '\n';
-    known_values_mapping << indent() << tenum_name << "_" << iter_name << "," << '\n';
+    known_values_mapping << indent() << go_enum_name << "," << '\n';
     // Dictionaries to/from string names of enums
-    to_string_mapping << indent() << "case " << tenum_name << "_" << iter_name << ": return \""
-                      << iter_std_name << "\"" << '\n';
+    to_string_mapping << indent() << "case " << go_enum_name << ":" << '\n';
+    indent_up();
+    to_string_mapping << indent() << "return \"" << iter_std_name << "\"" << '\n';
+    indent_down();
 
     if (iter_std_name != escape_string(iter_name)) {
       from_string_mapping << indent() << "case \"" << iter_std_name << "\", \""
-                          << escape_string(iter_name) << "\": return " << tenum_name << "_"
-                          << iter_name << ", nil" << '\n';
+                          << escape_string(iter_name) << "\":" << '\n';
     } else {
-      from_string_mapping << indent() << "case \"" << iter_std_name << "\": return " << tenum_name
-                          << "_" << iter_name << ", nil" << '\n';
+      from_string_mapping << indent() << "case \"" << iter_std_name << "\":" << '\n';
     }
+    indent_up();
+    from_string_mapping << indent() << "return " << go_enum_name << ", nil" << '\n';
+    indent_down();
   }
 
   to_string_mapping << indent() << "}" << '\n';
@@ -845,7 +882,13 @@ void t_go_generator::generate_enum(t_enum* tenum) {
   indent_down();
   from_string_mapping << indent() << "}" << '\n';
 
-  known_values_mapping << indent() << "}" << '\n' << '\n';
+  if (constants.empty()) {
+    known_values_mapping.str(std::string());
+    known_values_mapping << indent() << "var known" << tenum_name << "Values" << " = []" << tenum_name << "{}" << '\n'
+                         << '\n';
+  } else {
+    known_values_mapping << indent() << "}" << '\n' << '\n';
+  }
   known_values_mapping << indent() << "func " << tenum_name << "Values() iter.Seq[" << tenum_name << "] {" << '\n';
   indent_up();
   known_values_mapping << indent() << "return func(yield func(" << tenum_name << ") bool) {" << '\n';
@@ -864,19 +907,22 @@ void t_go_generator::generate_enum(t_enum* tenum) {
   indent_down();
   known_values_mapping << indent() << "}" << '\n';
 
-  f_types_ << ")" << '\n' << '\n'
-           << known_values_mapping.str() << '\n'
+  if (!constants.empty()) {
+    f_types_ << ")" << '\n' << '\n';
+  }
+  f_types_ << known_values_mapping.str()
            << to_string_mapping.str() << '\n'
-           << from_string_mapping.str() << '\n' << '\n';
+           << from_string_mapping.str() << '\n';
 
   // Generate a convenience function that converts an instance of an enum
   // (which may be a constant) into a pointer to an instance of that enum
   // type.
   generate_deprecation_comment(f_types_, tenum->annotations_);
   f_types_ << "func " << tenum_name << "Ptr(v " << tenum_name << ") *" << tenum_name
-           << " { return &v }" << '\n' << '\n';
+           << " { return &v }" << '\n';
 
   // Generate MarshalText
+  f_types_ << '\n';
   f_types_ << "func (p " << tenum_name << ") MarshalText() ([]byte, error) {" << '\n';
   indent_up();
   f_types_ << indent() << "return []byte(p.String()), nil" << '\n';
@@ -921,8 +967,7 @@ void t_go_generator::generate_enum(t_enum* tenum) {
   f_types_ << indent() << "}" << '\n';
   f_types_ << indent() << "return int64(*p), nil" << '\n';
   indent_down();
-  f_types_ << "}" << '\n' << '\n';
-
+  f_types_ << "}" << '\n';
 }
 
 /**
@@ -933,10 +978,20 @@ void t_go_generator::generate_const(t_const* tconst) {
   string name = publicize(tconst->get_name());
   t_const_value* value = tconst->get_value();
   if (type->is_enum() || (type->is_base_type() && ((t_base_type*)type)->get_base() != t_base_type::TYPE_UUID)) {
+    if (last_const_block_ == 2) {
+      f_consts_ << '\n';
+    }
+    last_const_block_ = 1;
     indent(f_consts_) << "const " << name << " = " << render_const_value(type, value, name) << '\n';
   } else {
-    f_const_values_ << indent() << name << " = " << render_const_value(type, value, name) << '\n'
-                    << '\n';
+    if (last_const_block_ == 1) {
+      f_consts_ << '\n';
+    }
+    last_const_block_ = 2;
+    indent_up();
+    string rendered_value = render_const_value(type, value, name);
+    indent_down();
+    f_const_values_ << "\t" << name << " = " << rendered_value << '\n';
 
     f_consts_ << indent() << "var " << name << " " << type_to_go_type(type) << '\n';
   }
@@ -1115,13 +1170,21 @@ string t_go_generator::render_const_value(t_type* type, t_const_value* value, co
       out << ")";
     }
   } else if (type->is_struct() || type->is_xception()) {
+    const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
+    if (val.empty()) {
+      out << "&" << publicize(type_name(type)) << "{}";
+      return out.str();
+    }
     out << "&" << publicize(type_name(type)) << "{";
     indent_up();
     const vector<t_field*>& fields = ((t_struct*)type)->get_members();
     vector<t_field*>::const_iterator f_iter;
-    const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
     map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
 
+    vector<string> field_names;
+    vector<string> field_values;
+    vector<bool> multiline_values;
+
     for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
       t_type* field_type = nullptr;
       bool is_optional = false;
@@ -1135,24 +1198,63 @@ string t_go_generator::render_const_value(t_type* type, t_const_value* value, co
       if (field_type == nullptr) {
         throw "type error: " + type->get_name() + " has no field " + v_iter->first->get_string();
       }
-      out << '\n' << indent() << publicize(v_iter->first->get_string()) << ": "
-          << render_const_value(field_type, v_iter->second, name, is_optional) << "," << '\n';
+      string rendered_value = render_const_value(field_type, v_iter->second, name, is_optional);
+      field_names.push_back(publicize(v_iter->first->get_string()));
+      field_values.push_back(rendered_value);
+      multiline_values.push_back(rendered_value.find('\n') != string::npos);
+    }
+
+    for (size_t i = 0; i < field_names.size();) {
+      if (multiline_values[i]) {
+        out << '\n' << indent() << field_names[i] << ": " << field_values[i] << ",";
+        ++i;
+        continue;
+      }
+
+      size_t group_end = i;
+      size_t max_field_name_len = 0;
+      while (group_end < field_names.size() && !multiline_values[group_end]) {
+        max_field_name_len = (std::max)(max_field_name_len, field_names[group_end].size());
+        ++group_end;
+      }
+      for (; i < group_end; ++i) {
+        out << '\n' << indent() << field_names[i] << ":"
+            << string(max_field_name_len - field_names[i].size() + 1, ' ') << field_values[i] << ",";
+      }
     }
 
     indent_down();
-    out << "}";
+    out << '\n' << indent() << "}";
 
   } else if (type->is_map()) {
     t_type* ktype = ((t_map*)type)->get_key_type();
     t_type* vtype = ((t_map*)type)->get_val_type();
     const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
+    if (val.empty()) {
+      out << "map[" << type_to_go_key_type(ktype) << "]" << type_to_go_type(vtype) << "{}";
+      return out.str();
+    }
     out << "map[" << type_to_go_key_type(ktype) << "]" << type_to_go_type(vtype) << "{" << '\n';
     indent_up();
     map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
+    size_t max_key_len = 0;
+    for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
+      string rendered_value = render_const_value(vtype, v_iter->second, name);
+      if (rendered_value.find('\n') == string::npos) {
+        max_key_len = (std::max)(max_key_len, render_const_value(ktype, v_iter->first, name).size());
+      }
+    }
 
     for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
-      out << indent() << render_const_value(ktype, v_iter->first, name) << ": "
-          << render_const_value(vtype, v_iter->second, name) << "," << '\n';
+      string key = render_const_value(ktype, v_iter->first, name);
+      string rendered_value = render_const_value(vtype, v_iter->second, name);
+      out << indent() << key << ":";
+      if (rendered_value.find('\n') == string::npos) {
+        out << string(max_key_len - key.size() + 1, ' ');
+      } else {
+        out << " ";
+      }
+      out << rendered_value << "," << '\n';
     }
 
     indent_down();
@@ -1160,12 +1262,16 @@ string t_go_generator::render_const_value(t_type* type, t_const_value* value, co
   } else if (type->is_list()) {
     t_type* etype = ((t_list*)type)->get_elem_type();
     const vector<t_const_value*>& val = value->get_list();
+    if (val.empty()) {
+      out << "[]" << type_to_go_type(etype) << "{}";
+      return out.str();
+    }
     out << "[]" << type_to_go_type(etype) << "{" << '\n';
     indent_up();
     vector<t_const_value*>::const_iterator v_iter;
 
     for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
-      out << indent() << render_const_value(etype, *v_iter, name) << ", ";
+      out << indent() << render_const_value(etype, *v_iter, name) << "," << '\n';
     }
 
     indent_down();
@@ -1173,12 +1279,16 @@ string t_go_generator::render_const_value(t_type* type, t_const_value* value, co
   } else if (type->is_set()) {
     t_type* etype = ((t_set*)type)->get_elem_type();
     const vector<t_const_value*>& val = value->get_list();
+    if (val.empty()) {
+      out << "[]" << type_to_go_type(etype) << "{}";
+      return out.str();
+    }
     out << "[]" << type_to_go_type(etype) << "{" << '\n';
     indent_up();
     vector<t_const_value*>::const_iterator v_iter;
 
     for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
-      out << indent() << render_const_value(etype, *v_iter, name) << ", ";
+      out << indent() << render_const_value(etype, *v_iter, name) << "," << '\n';
     }
 
     indent_down();
@@ -1211,15 +1321,17 @@ void t_go_generator::generate_xception(t_struct* txception) {
  * Generates a go struct
  */
 void t_go_generator::generate_go_struct(t_struct* tstruct, bool is_exception) {
+  begin_types_declaration();
   generate_go_struct_definition(f_types_, tstruct, is_exception);
   // generate Validate function
+  f_types_ << '\n';
   std::string tstruct_name(publicize(tstruct->get_name(), false));
   f_types_ << "func (p *" << tstruct_name << ") Validate() error {" << '\n';
   indent_up();
   go_validator_generator(this).generate_struct_validator(f_types_, tstruct);
   f_types_ << indent() << "return nil" << '\n';
   indent_down();
-  f_types_ << "}" << '\n' << '\n';
+  f_types_ << "}" << '\n';
 }
 
 void t_go_generator::get_publicized_name_and_def_value(t_field* tfield,
@@ -1237,21 +1349,43 @@ void t_go_generator::generate_go_struct_initializer(ostream& out,
   out << publicize(type_name(tstruct), is_args_or_result) << "{";
   indent_up();
   const vector<t_field*>& members = tstruct->get_members();
-  bool empty = true;
+  vector<string> field_names;
+  vector<string> field_values;
+  vector<bool> multiline_values;
   for (auto member : members) {
     bool pointer_field = is_pointer_field(member);
     string publicized_name;
     t_const_value* def_value;
     get_publicized_name_and_def_value(member, &publicized_name, &def_value);
     if (!pointer_field && def_value != nullptr && !omit_initialization(member)) {
-      empty = false;
-      out << '\n' << indent() << publicized_name << ": "
-          << render_field_initial_value(member, member->get_name(), pointer_field) << ",";
+      string rendered_value = render_field_initial_value(member, member->get_name(), pointer_field);
+      field_names.push_back(publicized_name);
+      field_values.push_back(rendered_value);
+      multiline_values.push_back(rendered_value.find('\n') != string::npos);
+    }
+  }
+
+  for (size_t i = 0; i < field_names.size();) {
+    if (multiline_values[i]) {
+      out << '\n' << indent() << field_names[i] << ": " << field_values[i] << ",";
+      ++i;
+      continue;
+    }
+
+    size_t group_end = i;
+    size_t max_field_name_len = 0;
+    while (group_end < field_names.size() && !multiline_values[group_end]) {
+      max_field_name_len = (std::max)(max_field_name_len, field_names[group_end].size());
+      ++group_end;
+    }
+    for (; i < group_end; ++i) {
+      out << '\n' << indent() << field_names[i] << ":"
+          << string(max_field_name_len - field_names[i].size() + 1, ' ') << field_values[i] << ",";
     }
   }
 
   indent_down();
-  if (!empty) {
+  if (!field_names.empty()) {
     out << '\n' << indent();
   }
   out << "}" << '\n';
@@ -1303,12 +1437,17 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
   int num_setable = 0;
   if (sorted_members.empty() || (sorted_members[0]->get_key() >= 0)) {
     int sorted_keys_pos = 0;
+    for (m_iter = sorted_members.begin(); m_iter != sorted_members.end(); ++m_iter) {
+      if (tstruct->is_union())
+        (*m_iter)->set_req(t_field::T_OPTIONAL);
+    }
 
+    size_t max_field_name_len = 0;
+    size_t max_field_type_len = 0;
+    auto group_end = sorted_members.begin();
     for (m_iter = sorted_members.begin(); m_iter != sorted_members.end(); ++m_iter) {
       // Set field to optional if field is union, this is so we can get a
       // pointer to the field.
-      if (tstruct->is_union())
-        (*m_iter)->set_req(t_field::T_OPTIONAL);
       if (sorted_keys_pos != (*m_iter)->get_key()) {
         int first_unused = (std::max)(1, sorted_keys_pos++);
         while (sorted_keys_pos != (*m_iter)->get_key()) {
@@ -1322,6 +1461,22 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
         }
       }
 
+      if (m_iter == group_end) {
+        max_field_name_len = 0;
+        max_field_type_len = 0;
+        int group_key = (*m_iter)->get_key();
+        for (group_end = m_iter; group_end != sorted_members.end(); ++group_end) {
+          if ((*group_end)->get_key() != group_key) {
+            break;
+          }
+          max_field_name_len = (std::max)(max_field_name_len, publicize((*group_end)->get_name()).size());
+          max_field_type_len = (std::max)(
+              max_field_type_len,
+              type_to_go_type_with_opt((*group_end)->get_type(), is_pointer_field(*group_end)).size());
+          ++group_key;
+        }
+      }
+
       t_type* fieldType = (*m_iter)->get_type();
       string goType = type_to_go_type_with_opt(fieldType, is_pointer_field(*m_iter));
 
@@ -1354,7 +1509,9 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
       gotag.resize(gotag.size()-1);
 
       generate_deprecation_comment(out, (*m_iter)->annotations_);
-      indent(out) << publicize((*m_iter)->get_name()) << " " << goType << " `thrift:\""
+      string field_name = publicize((*m_iter)->get_name());
+      indent(out) << field_name << string(max_field_name_len - field_name.size() + 1, ' ')
+                  << goType << string(max_field_type_len - goType.size() + 1, ' ') << "`thrift:\""
                   << escape_string((*m_iter)->get_name()) << "," << sorted_keys_pos;
       if ((*m_iter)->get_req() == t_field::T_REQUIRED) {
         out << ",required";
@@ -1364,10 +1521,16 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
       sorted_keys_pos++;
     }
   } else {
+    size_t max_field_name_len = 0;
+    for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
+      max_field_name_len = (std::max)(max_field_name_len, publicize((*m_iter)->get_name()).size());
+    }
+
     for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
       generate_deprecation_comment(out, (*m_iter)->annotations_);
       // This fills in default values, as opposed to nulls
-      out << indent() << publicize((*m_iter)->get_name()) << " "
+      string field_name = publicize((*m_iter)->get_name());
+      out << indent() << field_name << string(max_field_name_len - field_name.size() + 1, ' ')
           << type_to_go_type((*m_iter)->get_type()) << '\n';
     }
   }
@@ -1389,6 +1552,7 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
     t_type* fieldType = (*m_iter)->get_type();
     string goType = type_to_go_type_with_opt(fieldType, false);
     string def_var_name = tstruct_name + "_" + publicized_name + "_DEFAULT";
+    bool emitted_default = false;
     if ((*m_iter)->get_req() == t_field::T_OPTIONAL || is_pointer_field(*m_iter)) {
       generate_deprecation_comment(out, (*m_iter)->annotations_);
       out << indent() << "var " << def_var_name << " " << goType;
@@ -1396,8 +1560,11 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
         out << " = " << render_const_value(fieldType, def_value, (*m_iter)->get_name());
       }
       out << '\n';
+      emitted_default = true;
+    }
+    if (emitted_default) {
+      out << '\n';
     }
-    out << '\n';
 
     // num_setable is used for deciding if Count* methods will be generated for union fields.
     // This applies to all nullable fields including slices (used for set, list and binary) and maps, not just pointers.
@@ -1422,7 +1589,6 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
       indent_down();
       out << indent() << "}" << '\n' << '\n';
     } else {
-      out << '\n';
       generate_deprecation_comment(out, (*m_iter)->annotations_);
       out << indent() << "func (p *" << tstruct_name << ") Get" << publicized_name << "() "
           << goType << " {" << '\n';
@@ -1454,28 +1620,32 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
   out << indent() << "return fmt.Sprintf(\"" << escape_string(tstruct_name) << "(%+v)\", *p)"
       << '\n';
   indent_down();
-  out << indent() << "}" << '\n' << '\n';
+  out << indent() << "}" << '\n';
 
   if (is_exception) {
+    out << '\n';
     out << indent() << "func (p *" << tstruct_name << ") Error() string {" << '\n';
     indent_up();
     out << indent() << "return p.String()" << '\n';
     indent_down();
-    out << indent() << "}" << '\n' << '\n';
+    out << indent() << "}" << '\n';
 
+    out << '\n';
     out << indent() << "func (" << tstruct_name << ") TExceptionType() thrift.TExceptionType {" << '\n';
     indent_up();
     out << indent() << "return thrift.TExceptionTypeCompiled" << '\n';
     indent_down();
-    out << indent() << "}" << '\n' << '\n';
+    out << indent() << "}" << '\n';
 
+    out << '\n';
     out << indent() << "var _ thrift.TException = (*" << tstruct_name << ")(nil)"
-        << '\n' << '\n';
+        << '\n';
   }
 
   if (!read_write_private_) {
     // Generate the implementation of slog.LogValuer,
     // see: https://issues.apache.org/jira/browse/THRIFT-5745
+    out << '\n';
     out << indent() << "func (p *" << tstruct_name << ") LogValue() slog.Value {" << '\n';
     indent_up();
     out << indent() << "if p == nil {" << '\n';
@@ -1485,16 +1655,17 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
     out << indent() << "}" << '\n';
     out << indent() << "v := thrift.SlogTStructWrapper{" << '\n';
     indent_up();
-    out << indent() << "Type: \"*" << package_name_ << "." << tstruct_name << "\"," << '\n';
+    out << indent() << "Type:  \"*" << package_name_ << "." << tstruct_name << "\"," << '\n';
     out << indent() << "Value: p," << '\n';
     indent_down();
     out << indent() << "}" << '\n';
     out << indent() << "return slog.AnyValue(v)" << '\n';
     indent_down();
-    out << indent() << "}" << '\n' << '\n';
+    out << indent() << "}" << '\n';
 
+    out << '\n';
     out << indent() << "var _ slog.LogValuer = (*" << tstruct_name << ")(nil)"
-        << '\n' << '\n';
+        << '\n';
   }
 }
 
@@ -1565,7 +1736,7 @@ void t_go_generator::generate_countsetfields_helper(ostream& out,
 
     const string field_name(publicize(escape_string((*f_iter)->get_name())));
 
-    out << indent() << "if (p.IsSet" << field_name << "()) {" << '\n';
+    out << indent() << "if p.IsSet" << field_name << "() {" << '\n';
     indent_up();
     out << indent() << "count++" << '\n';
     indent_down();
@@ -1596,17 +1767,15 @@ void t_go_generator::generate_go_struct_reader(ostream& out,
   out << indent() << "return thrift.PrependError(fmt.Sprintf(\"%T read error: \", p), err)"
       << '\n';
   indent_down();
-  out << indent() << "}" << '\n' << '\n';
+  out << indent() << "}" << '\n';
 
   // Required variables does not have IsSet functions, so we need tmp vars to check them.
   for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
     if ((*f_iter)->get_req() == t_field::T_REQUIRED) {
       const string field_name(publicize(escape_string((*f_iter)->get_name())));
-      indent(out) << "var isset" << field_name << " bool = false;" << '\n';
+      indent(out) << "var isset" << field_name << " bool = false" << '\n';
     }
   }
-  out << '\n';
-
   // Loop over reading in fields
   indent(out) << "for {" << '\n';
   indent_up();
@@ -1724,7 +1893,7 @@ void t_go_generator::generate_go_struct_reader(ostream& out,
   for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
     if ((*f_iter)->get_req() == t_field::T_REQUIRED) {
       const string field_name(publicize(escape_string((*f_iter)->get_name())));
-      out << indent() << "if !isset" << field_name << "{" << '\n';
+      out << indent() << "if !isset" << field_name << " {" << '\n';
       indent_up();
       out << indent() << "return thrift.NewTProtocolExceptionWithType(thrift.INVALID_DATA, "
                       << "fmt.Errorf(\"Required field " << field_name << " is not set\"))" << '\n';
@@ -1808,7 +1977,11 @@ void t_go_generator::generate_go_struct_writer(ostream& out,
     }
 
     out << indent() << "if err := p." << field_method_prefix << field_method_suffix
-        << "(ctx, oprot); err != nil { return err }" << '\n';
+        << "(ctx, oprot); err != nil {" << '\n';
+    indent_up();
+    out << indent() << "return err" << '\n';
+    indent_down();
+    out << indent() << "}" << '\n';
   }
 
   indent_down();
@@ -1916,8 +2089,8 @@ void t_go_generator::generate_go_struct_equals(ostream& out,
     // Compare field contents
     if (is_pointer_field(*f_iter)
         && (ttype->is_base_type() || ttype->is_enum() || ttype->is_container())) {
-      string tgtv = "(*" + tgt + ")";
-      string srcv = "(*" + src + ")";
+      string tgtv = "*" + tgt;
+      string srcv = "*" + src;
       out << indent() << "if " << tgt << " != " << src << " {" << '\n';
       indent_up();
       out << indent() << "if " << tgt << " == nil || " << src << " == nil {" << '\n';
@@ -1954,7 +2127,6 @@ void t_go_generator::generate_service(t_service* tservice) {
   if(!skip_remote_) {
     generate_service_remote(tservice);
   }
-  f_types_ << '\n';
 }
 
 /**
@@ -1963,11 +2135,18 @@ void t_go_generator::generate_service(t_service* tservice) {
  * @param tservice The service to generate a header definition for
  */
 void t_go_generator::generate_service_helpers(t_service* tservice) {
+  begin_types_declaration();
   vector<t_function*> functions = tservice->get_functions();
   vector<t_function*>::iterator f_iter;
-  f_types_ << "// HELPER FUNCTIONS AND STRUCTURES" << '\n' << '\n';
+  f_types_ << "// HELPER FUNCTIONS AND STRUCTURES" << '\n';
+  if (!functions.empty()) {
+    f_types_ << '\n';
+  }
 
   for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
+    if (f_iter != functions.begin()) {
+      f_types_ << '\n';
+    }
     t_struct* ts = (*f_iter)->get_arglist();
     generate_go_struct_definition(f_types_, ts, false, false, true);
     generate_go_function_helpers(*f_iter);
@@ -1999,6 +2178,7 @@ void t_go_generator::generate_go_function_helpers(t_function* tfunction) {
       result.append(f);
     }
 
+    f_types_ << '\n';
     generate_go_struct_definition(f_types_, &result, false, true);
   }
 }
@@ -2009,6 +2189,7 @@ void t_go_generator::generate_go_function_helpers(t_function* tfunction) {
  * @param tservice The service to generate a header definition for
  */
 void t_go_generator::generate_service_interface(t_service* tservice) {
+  begin_types_declaration();
   string extends = "";
   string extends_if = "";
   string serviceName(publicize(tservice->get_name()));
@@ -2030,12 +2211,17 @@ void t_go_generator::generate_service_interface(t_service* tservice) {
 
   generate_deprecation_comment(f_types_, tservice->annotations_);
   f_types_ << indent() << "type " << interfaceName << " interface {" << extends_if;
+  if (extends_if.empty()) {
+    f_types_ << '\n';
+  }
   indent_up();
   generate_go_docstring(f_types_, tservice);
   vector<t_function*> functions = tservice->get_functions();
 
   if (!functions.empty()) {
-    f_types_ << '\n';
+    if (tservice->has_doc()) {
+      f_types_ << '\n';
+    }
     vector<t_function*>::iterator f_iter;
 
     for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
@@ -2046,7 +2232,7 @@ void t_go_generator::generate_service_interface(t_service* tservice) {
   }
 
   indent_down();
-  f_types_ << indent() << "}" << '\n' << '\n';
+  f_types_ << indent() << "}" << '\n';
 }
 
 /**
@@ -2055,11 +2241,14 @@ void t_go_generator::generate_service_interface(t_service* tservice) {
  * @param tservice The service to generate a server for.
  */
 void t_go_generator::generate_service_client(t_service* tservice) {
+  begin_types_declaration();
   string extends = "";
   string extends_field = "";
   string extends_client = "";
   string extends_client_new = "";
   string serviceName(publicize(tservice->get_name()));
+  vector<t_function*> functions = tservice->get_functions();
+  vector<t_function*>::const_iterator f_iter;
 
   if (tservice->get_extends() != nullptr) {
     extends = type_name(tservice->get_extends());
@@ -2086,7 +2275,7 @@ void t_go_generator::generate_service_client(t_service* tservice) {
   if (!extends_client.empty()) {
     f_types_ << indent() << "*" << extends_client << '\n';
   } else {
-    f_types_ << indent() << "c thrift.TClient" << '\n';
+    f_types_ << indent() << "c    thrift.TClient" << '\n';
     f_types_ << indent() << "meta thrift.ResponseMeta" << '\n';
   }
 
@@ -2102,7 +2291,7 @@ void t_go_generator::generate_service_client(t_service* tservice) {
   f_types_ << indent() << "return &" << serviceName << "Client";
 
   if (!extends.empty()) {
-    f_types_ << "{" << extends_field << ": " << extends_client_new << "Factory(t, f)}";
+    f_types_ << "{" << extends_field << ": " << extends_client_new << "Factory(t, f)}" << '\n';
   } else {
     indent_up();
     f_types_ << "{" << '\n';
@@ -2153,7 +2342,10 @@ void t_go_generator::generate_service_client(t_service* tservice) {
   f_types_ << indent() << "}" << '\n';
 
   indent_down();
-  f_types_ << indent() << "}" << '\n' << '\n';
+  f_types_ << indent() << "}" << '\n';
+  if (extends.empty() || !functions.empty()) {
+    f_types_ << '\n';
+  }
 
   if (extends.empty()) {
     f_types_ << indent() << "func (p *" << serviceName << "Client) Client_() thrift.TClient {" << '\n';
@@ -2172,12 +2364,13 @@ void t_go_generator::generate_service_client(t_service* tservice) {
     indent_up();
     f_types_ << indent() << "p.meta = meta" << '\n';
     indent_down();
-    f_types_ << indent() << "}" << '\n' << '\n';
+    f_types_ << indent() << "}" << '\n';
+    if (!functions.empty()) {
+      f_types_ << '\n';
+    }
   }
 
   // Generate client method implementations
-  vector<t_function*> functions = tservice->get_functions();
-  vector<t_function*>::const_iterator f_iter;
 
   for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
     t_struct* arg_struct = (*f_iter)->get_arglist();
@@ -2228,7 +2421,7 @@ void t_go_generator::generate_service_client(t_service* tservice) {
           const std::string pubname = publicize((*x_iter)->get_name());
           const std::string field = resultName + "." + pubname;
 
-          f_types_ << indent() << "case " << field << "!= nil:" << '\n';
+          f_types_ << indent() << "case " << field << " != nil:" << '\n';
           indent_up();
 
           if (!(*f_iter)->get_returntype()->is_void()) {
@@ -2281,7 +2474,10 @@ void t_go_generator::generate_service_client(t_service* tservice) {
     }
 
     indent_down();
-    f_types_ << "}" << '\n' << '\n';
+    f_types_ << "}" << '\n';
+    if (f_iter + 1 != functions.end()) {
+      f_types_ << '\n';
+    }
   }
 }
 
@@ -2340,19 +2536,34 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
   system_packages.push_back("os");
   system_packages.push_back("strconv");
   system_packages.push_back("strings");
+
+  std::vector<string> thrift_package;
   // For the thrift import, always do rename import to make sure it's called thrift.
-  system_packages.push_back("thrift \"" + gen_thrift_import_ + "\"");
+  thrift_package.push_back("thrift \"" + gen_thrift_import_ + "\"");
+
+  string system_imports = render_system_packages(system_packages);
+  string thrift_import = render_system_packages(thrift_package);
 
   f_remote << go_autogen_comment();
   f_remote << indent() << "package main" << '\n' << '\n';
   f_remote << indent() << "import (" << '\n';
-  f_remote << render_system_packages(system_packages);
-  f_remote << indent() << render_included_programs(unused_protection);
-  f_remote << render_program_import(program_, unused_protection);
+  f_remote << system_imports;
+  f_remote << '\n' << thrift_import;
+  std::map<std::string, const t_program*> local_programs;
+  for (auto include : program_->get_includes()) {
+    local_programs.emplace(get_real_go_module(include), include);
+  }
+  local_programs.emplace(get_real_go_module(program_), program_);
+  if (!local_programs.empty()) {
+    f_remote << '\n';
+    for (auto const& local_program : local_programs) {
+      f_remote << render_program_import(local_program.second, unused_protection);
+    }
+  }
   f_remote << indent() << ")" << '\n';
-  f_remote << indent() << '\n';
+  f_remote << '\n';
   f_remote << indent() << unused_protection; // filled in render_included_programs()
-  f_remote << indent() << '\n';
+  f_remote << '\n';
   f_remote << indent() << "func Usage() {" << '\n';
   indent_up();
   f_remote << indent() << "fmt.Fprintln(os.Stderr, \"Usage of \", os.Args[0], \" "
@@ -2388,17 +2599,17 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
   f_remote << indent() << "os.Exit(0)" << '\n';
   indent_down();
   f_remote << indent() << "}" << '\n';
-  f_remote << indent() << '\n';
+  f_remote << '\n';
 
   f_remote << indent() << "type httpHeaders map[string]string" << '\n';
-  f_remote << indent() << '\n';
+  f_remote << '\n';
   f_remote << indent() << "func (h httpHeaders) String() string {" << '\n';
   indent_up();
   f_remote << indent() << "var m map[string]string = h" << '\n';
   f_remote << indent() << "return fmt.Sprintf(\"%s\", m)" << '\n';
   indent_down();
   f_remote << indent() << "}" << '\n';
-  f_remote << indent() << '\n';
+  f_remote << '\n';
   f_remote << indent() << "func (h httpHeaders) Set(value string) error {" << '\n';
   indent_up();
   f_remote << indent() << "parts := strings.Split(value, \": \")" << '\n';
@@ -2411,7 +2622,7 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
   f_remote << indent() << "return nil" << '\n';
   indent_down();
   f_remote << indent() << "}" << '\n';
-  f_remote << indent() << '\n';
+  f_remote << '\n';
 
   f_remote << indent() << "func main() {" << '\n';
   indent_up();
@@ -2439,7 +2650,7 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
   f_remote << indent() << "flag.BoolVar(&useHttp, \"http\", false, \"Use http\")" << '\n';
   f_remote << indent() << "flag.Var(headers, \"H\", \"Headers to set on the http(s) request (e.g. -H \\\"Key: Value\\\")\")" << '\n';
   f_remote << indent() << "flag.Parse()" << '\n';
-  f_remote << indent() << '\n';
+  f_remote << '\n';
   f_remote << indent() << "if len(urlString) > 0 {" << '\n';
   indent_up();
   f_remote << indent() << "var err error" << '\n';
@@ -2466,7 +2677,7 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
   f_remote << indent() << "}" << '\n';
   indent_down();
   f_remote << indent() << "}" << '\n';
-  f_remote << indent() << '\n';
+  f_remote << '\n';
   f_remote << indent() << "cmd := flag.Arg(0)" << '\n';
   f_remote << indent() << "var err error" << '\n';
   f_remote << indent() << "var cfg *thrift.TConfiguration = nil" << '\n';
@@ -2557,7 +2768,7 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
   f_remote << indent() << "os.Exit(1)" << '\n';
   indent_down();
   f_remote << indent() << "}" << '\n';
-  f_remote << indent() << '\n';
+  f_remote << '\n';
   f_remote << indent() << "switch cmd {" << '\n';
 
   for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
@@ -2569,7 +2780,7 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
     string argumentsName(publicize(funcName + "_args", true, func_to_service[funcName]));
     f_remote << indent() << "case \"" << escape_string(funcName) << "\":" << '\n';
     indent_up();
-    f_remote << indent() << "if flag.NArg() - 1 != " << num_args << " {" << '\n';
+    f_remote << indent() << "if flag.NArg()-1 != " << num_args << " {" << '\n';
     indent_up();
     f_remote << indent() << "fmt.Fprintln(os.Stderr, \"" << escape_string(pubName) << " requires "
              << num_args << " args\")" << '\n';
@@ -2858,6 +3069,7 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
  * @param tservice The service to generate a server for.
  */
 void t_go_generator::generate_service_server(t_service* tservice) {
+  begin_types_declaration();
   // Generate the dispatch methods
   vector<t_function*> functions = tservice->get_functions();
   vector<t_function*>::iterator f_iter;
@@ -2890,7 +3102,7 @@ void t_go_generator::generate_service_server(t_service* tservice) {
     f_types_ << indent() << "type " << serviceName << "Processor struct {" << '\n';
     indent_up();
     f_types_ << indent() << "processorMap map[string]thrift.TProcessorFunction" << '\n';
-    f_types_ << indent() << "handler " << serviceName << '\n';
+    f_types_ << indent() << "handler      " << serviceName << '\n';
     indent_down();
     f_types_ << indent() << "}" << '\n' << '\n';
     f_types_ << indent() << "func (p *" << serviceName
@@ -2920,14 +3132,14 @@ void t_go_generator::generate_service_server(t_service* tservice) {
     indent_up();
     f_types_
         << indent() << self << " := &" << serviceName
-        << "Processor{handler:handler, processorMap:make(map[string]thrift.TProcessorFunction)}"
+        << "Processor{handler: handler, processorMap: make(map[string]thrift.TProcessorFunction)}"
         << '\n';
 
     for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
       string escapedFuncName(escape_string((*f_iter)->get_name()));
       f_types_ << indent() << self << ".processorMap[\"" << escapedFuncName << "\"] = &"
                  << pServiceName << "Processor" << publicize((*f_iter)->get_name())
-                 << "{handler:handler}" << '\n';
+                 << "{handler: handler}" << '\n';
     }
 
     string x(tmp("x"));
@@ -2939,7 +3151,11 @@ void t_go_generator::generate_service_server(t_service* tservice) {
                   "thrift.TException) {" << '\n';
     indent_up();
     f_types_ << indent() << "name, _, seqId, err2 := iprot.ReadMessageBegin(ctx)" << '\n';
-    f_types_ << indent() << "if err2 != nil { return false, thrift.WrapTException(err2) }" << '\n';
+    f_types_ << indent() << "if err2 != nil {" << '\n';
+    indent_up();
+    f_types_ << indent() << "return false, thrift.WrapTException(err2)" << '\n';
+    indent_down();
+    f_types_ << indent() << "}" << '\n';
     f_types_ << indent() << "if processor, ok := p.GetProcessorFunction(name); ok {" << '\n';
     indent_up();
     f_types_ << indent() << "return processor.Process(ctx, seqId, iprot, oprot)" << '\n';
@@ -2949,14 +3165,14 @@ void t_go_generator::generate_service_server(t_service* tservice) {
     f_types_ << indent() << "iprot.ReadMessageEnd(ctx)" << '\n';
     f_types_ << indent() << "" << x
                << " := thrift.NewTApplicationException(thrift.UNKNOWN_METHOD, \"Unknown function "
-                  "\" + name)" << '\n';
+                  "\"+name)" << '\n';
     f_types_ << indent() << "oprot.WriteMessageBegin(ctx, name, thrift.EXCEPTION, seqId)" << '\n';
     f_types_ << indent() << "" << x << ".Write(ctx, oprot)" << '\n';
     f_types_ << indent() << "oprot.WriteMessageEnd(ctx)" << '\n';
     f_types_ << indent() << "oprot.Flush(ctx)" << '\n';
     f_types_ << indent() << "return false, " << x << '\n';
     indent_down();
-    f_types_ << indent() << "}" << '\n' << '\n';
+    f_types_ << indent() << "}" << '\n';
   } else {
     f_types_ << indent() << "type " << serviceName << "Processor struct {" << '\n';
     indent_up();
@@ -2973,20 +3189,20 @@ void t_go_generator::generate_service_server(t_service* tservice) {
       string escapedFuncName(escape_string((*f_iter)->get_name()));
       f_types_ << indent() << self << ".AddToProcessorMap(\"" << escapedFuncName
                  << "\", &" << pServiceName << "Processor" << publicize((*f_iter)->get_name())
-                 << "{handler:handler})" << '\n';
+                 << "{handler: handler})" << '\n';
     }
 
     f_types_ << indent() << "return " << self << '\n';
     indent_down();
-    f_types_ << indent() << "}" << '\n' << '\n';
+    f_types_ << indent() << "}" << '\n';
   }
 
   // Generate the process subfunctions
   for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
+    f_types_ << '\n';
     generate_process_function(tservice, *f_iter);
   }
 
-  f_types_ << '\n';
 }
 
 /**
@@ -3173,7 +3389,7 @@ void t_go_generator::generate_process_function(t_service* tservice, t_function*
     string exc(tmp("_exc"));
     f_types_ << indent() << exc << " := thrift.NewTApplicationException(thrift.INTERNAL_ERROR, "
                               "\"Internal error processing " << escape_string(tfunction->get_name())
-               << ": \" + err2.Error())" << '\n';
+               << ": \"+err2.Error())" << '\n';
 
     f_types_ << indent() << "if err2 := oprot.WriteMessageBegin(ctx, \"" << escape_string(tfunction->get_name())
                << "\", thrift.EXCEPTION, seqId); err2 != nil {" << '\n';
@@ -3289,7 +3505,7 @@ void t_go_generator::generate_process_function(t_service* tservice, t_function*
     f_types_ << indent() << "return true, err" << '\n';
   }
   indent_down();
-  f_types_ << indent() << "}" << '\n' << '\n';
+  f_types_ << indent() << "}" << '\n';
 }
 
 /**
@@ -3730,9 +3946,9 @@ void t_go_generator::generate_serialize_container(ostream& out,
     indent(out) << "}" << '\n';
   } else if (ttype->is_set()) {
     t_set* tset = (t_set*)ttype;
-    out << indent() << "for i := 0; i<len(" << prefix << "); i++ {" << '\n';
+    out << indent() << "for i := 0; i < len(" << prefix << "); i++ {" << '\n';
     indent_up();
-    out << indent() << "for j := i+1; j<len(" << prefix << "); j++ {" << '\n';
+    out << indent() << "for j := i + 1; j < len(" << prefix << "); j++ {" << '\n';
     indent_up();
     string wrapped_prefix = prefix;
     if (pointer_field) {
@@ -3874,7 +4090,11 @@ void t_go_generator::generate_go_equals(ostream& out, t_type* ori_type, string t
       out << tgt << " != " << src;
     }
 
-    out << " { return false }" << '\n';
+    out << " {" << '\n';
+    indent_up();
+    out << indent() << "return false" << '\n';
+    indent_down();
+    out << indent() << "}" << '\n';
   } else {
     throw "compiler error: Invalid type in generate_go_equals '" + ttype->get_name() + "' for '"
         + tgt + "'";
@@ -3890,7 +4110,11 @@ void t_go_generator::generate_go_equals_struct(ostream& out,
                                                string src) {
   (void)ttype;
   out << indent() << "if !" << tgt << "." << equals_method_name_ << "(" << src
-      << ") { return false }" << '\n';
+      << ") {" << '\n';
+  indent_up();
+  out << indent() << "return false" << '\n';
+  indent_down();
+  out << indent() << "}" << '\n';
 }
 
 /**
@@ -3900,13 +4124,17 @@ void t_go_generator::generate_go_equals_container(ostream& out,
                                                   t_type* ttype,
                                                   string tgt,
                                                   string src) {
-  out << indent() << "if len(" << tgt << ") != len(" << src << ") { return false }" << '\n';
+  out << indent() << "if len(" << tgt << ") != len(" << src << ") {" << '\n';
+  indent_up();
+  out << indent() << "return false" << '\n';
+  indent_down();
+  out << indent() << "}" << '\n';
   if (ttype->is_map()) {
     t_map* tmap = (t_map*)ttype;
     out << indent() << "for k, _tgt := range " << tgt << " {" << '\n';
     indent_up();
     string element_source = tmp("_src");
-    out << indent() << element_source << " := " << src << "[k]" << '\n';
+    out << indent() << element_source << " := " << indexable_go_expr(src) << "[k]" << '\n';
     generate_go_equals(out, tmap->get_val_type(), "_tgt", element_source);
     indent_down();
     indent(out) << "}" << '\n';
@@ -3922,7 +4150,7 @@ void t_go_generator::generate_go_equals_container(ostream& out,
     out << indent() << "for i, _tgt := range " << tgt << " {" << '\n';
     indent_up();
     string element_source = tmp("_src");
-    out << indent() << element_source << " := " << src << "[i]" << '\n';
+    out << indent() << element_source << " := " << indexable_go_expr(src) << "[i]" << '\n';
     generate_go_equals(out, elem, "_tgt", element_source);
     indent_down();
     indent(out) << "}" << '\n';
@@ -3973,7 +4201,7 @@ void t_go_generator::generate_go_docstring(ostream& out,
 
     for (p_iter = fields.begin(); p_iter != fields.end(); ++p_iter) {
       t_field* p = *p_iter;
-      ss << " - " << publicize(p->get_name());
+      ss << "  - " << publicize(p->get_name());
 
       if (p->has_doc()) {
         ss << ": " << p->get_doc();
@@ -3981,11 +4209,10 @@ void t_go_generator::generate_go_docstring(ostream& out,
         ss << '\n';
       }
     }
-    ss << '\n';
   }
 
   if (has_doc) {
-    generate_docstring_comment(out, "", "// ", ss.str(), "");
+    generate_go_docstring_comment(out, ss.str());
   }
 }
 
@@ -3994,7 +4221,33 @@ void t_go_generator::generate_go_docstring(ostream& out,
  */
 void t_go_generator::generate_go_docstring(ostream& out, t_doc* tdoc) {
   if (tdoc->has_doc()) {
-    generate_docstring_comment(out, "", "//", tdoc->get_doc(), "");
+    generate_go_docstring_comment(out, tdoc->get_doc());
+  }
+}
+
+void t_go_generator::generate_go_docstring_comment(ostream& out, const string& contents) {
+  if (contents.find_first_not_of(" \t\r\n") == string::npos) {
+    return;
+  }
+  stringstream docs(contents, std::ios_base::in);
+  while (!(docs.eof() || docs.fail())) {
+    char line[1024];
+    docs.getline(line, 1024);
+
+    if (strlen(line) > 0) {
+      string doc_line(line);
+      if (doc_line.find("  -") != 0) {
+        doc_line.erase(0, doc_line.find_first_not_of(" \t"));
+      }
+      size_t quote_pos = 0;
+      while ((quote_pos = doc_line.find("''", quote_pos)) != string::npos) {
+        doc_line.replace(quote_pos, 2, "\"\"");
+        quote_pos += 2;
+      }
+      indent(out) << "// " << doc_line << '\n';
+    } else if (!docs.eof()) {
+      indent(out) << "//" << '\n';
+    }
   }
 }
 
@@ -4126,6 +4379,9 @@ string t_go_generator::module_name(t_type* ttype) {
         program_->get_namespace("go").empty() ||
         program->get_namespace("go") != program_->get_namespace("go")) {
       string module(get_real_go_module(program));
+      if (module == get_real_go_module(program_)) {
+        return "";
+      }
       return package_identifiers_[module];
     }
   }
diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.h b/compiler/cpp/src/thrift/generate/t_go_generator.h
index 72825bc9caa..5ffdbb73cfe 100644
--- a/compiler/cpp/src/thrift/generate/t_go_generator.h
+++ b/compiler/cpp/src/thrift/generate/t_go_generator.h
@@ -243,6 +243,7 @@ class t_go_generator : public t_generator {
 
   std::string go_autogen_comment();
   std::string go_package();
+  void generate_go_docstring_comment(std::ostream& out, const std::string& contents);
   std::string go_imports_begin(bool consts);
   std::string go_imports_end();
   std::string render_includes(bool consts);
@@ -313,6 +314,8 @@ class t_go_generator : public t_generator {
   std::string package_dir_;
   std::unordered_map<std::string, std::string> package_identifiers_;
   std::set<std::string> package_identifiers_set_;
+  int last_const_block_;
+  bool types_file_has_declaration_;
   std::string read_method_name_;
   std::string write_method_name_;
   std::string equals_method_name_;
@@ -328,7 +331,9 @@ class t_go_generator : public t_generator {
   std::string privatize(const std::string& value) const;
   std::string new_prefix(const std::string& value) const;
   static std::string variable_name_to_go_name(const std::string& value);
+  static std::string package_name_to_go_name(const std::string& value);
   static bool omit_initialization(t_field* tfield);
+  void begin_types_declaration();
 };
 
 #endif