Go Code Style Guidelines

Guidelines

Copy Slices and Maps at Boundaries

Slices and maps contain pointers to the underlying data so be wary of scenarios when they need to be copied.

Receiving Slices and Maps

Keep in mind that users can modify a map or slice you received as an argument if you store a reference to it.

Bad Good

func (d *Driver) SetTrips(trips []Trip) {
  d.trips = trips
}

trips := ...
d1.SetTrips(trips)

// Did you mean to modify d1.trips?
trips[0] = ...

func (d *Driver) SetTrips(trips []Trip) {
  d.trips = make([]Trip, len(trips))
  copy(d.trips, trips)
}

trips := ...
d1.SetTrips(trips)

// We can now modify trips[0] without affecting d1.trips.
trips[0] = ...

Returning Slices and Maps

Similarly, be wary of user modifications to maps or slices exposing internal state.

Bad Good

type Stats struct {
  mu sync.Mutex
  counters map[string]int
}

// Snapshot returns the current stats.
func (s *Stats) Snapshot() map[string]int {
  s.mu.Lock()
  defer s.mu.Unlock()

  return s.counters
}

// snapshot is no longer protected by the mutex, so any
// access to the snapshot is subject to data races.
snapshot := stats.Snapshot()

type Stats struct {
  mu sync.Mutex
  counters map[string]int
}

func (s *Stats) Snapshot() map[string]int {
  s.mu.Lock()
  defer s.mu.Unlock()

  result := make(map[string]int, len(s.counters))
  for k, v := range s.counters {
    result[k] = v
  }
  return result
}

// Snapshot is now a copy.
snapshot := stats.Snapshot()

Start Enums at One

The standard way of introducing enumerations in Go is to declare a custom type and a const group with iota. Since variables have a 0 default value, you should usually start your enums on a non-zero value.

Bad Good

type Operation int

const (
  Add Operation = iota
  Subtract
  Multiply
)

// Add=0, Subtract=1, Multiply=2

type Operation int

const (
  Add Operation = iota + 1
  Subtract
  Multiply
)

// Add=1, Subtract=2, Multiply=3

There are cases where using the zero value makes sense, for example when the zero value case is the desirable default behavior.

type LogOutput int

const (
  LogToStdout LogOutput = iota
  LogToFile
  LogToRemote
)

// LogToStdout=0, LogToFile=1, LogToRemote=2

Error Types

There are various options for declaring errors:

errors.New for errors with simple static strings
fmt.Errorf for formatted and wrapped error strings
Custom types that implement the error interface

When returning errors, consider the following to determine the best choice:

Is this a simple error that needs no extra information? If so, errors.New should suffice.
Do the clients need to detect and handle this error? If so, you should use a custom type, and implement the Error() method.
Are you propagating an error returned by a downstream function? If so, check the section on error wrapping.
Otherwise, fmt.Errorf is okay.

If the client needs to detect a specific error case, a sentinel error should be created using errors.New.

Bad Good

// package foo

func Open() error {
  return errors.New("could not open")
}

// package bar

func use() {
  err := foo.Open()
  if err != nil {
    if err.Error() == "could not open" {
      // handle
    } else {
      panic("unknown error")
    }
  }
}

// package foo

var ErrCouldNotOpen = errors.New("could not open")

func Open() error {
  return ErrCouldNotOpen
}

// package bar

err := foo.Open()
if err != nil {
  if errors.Is(err, foo.ErrCouldNotOpen) {
    // handle
  } else {
    panic("unknown error")
  }
}

If you have an error that clients may need to detect, and you would like to add more information to it (e.g., it is not a static string), then you should use a custom type.

Bad Good

func open(file string) error {
  return fmt.Errorf("file %q not found", file)
}

func use() {
  err := open("testfile.txt")
  if err != nil {
    if strings.Contains(err.Error(), "not found") {
      // handle
    } else {
      panic("unknown error")
    }
  }
}

type errNotFound struct {
  file string
}

func (e errNotFound) Error() string {
  return fmt.Sprintf("file %q not found", e.file)
}

func open(file string) error {
  return errNotFound{file: file}
}

func use() {
  err := open("testfile.txt")
  if err != nil {
    var errNF *errNotFound
    if errors.As(err, &errNF) {
      // handle the error. errNF contains the error's value.
    } else {
      panic("unknown error")
    }
  }
}

Error Wrapping

There are three main options for propagating errors if a call fails:

Return the original error if there is no additional context to add and you want to maintain the original error type.
Add context using fmt.Errorf with the %w flag, so that the error message provides more context.

It is recommended to add context where possible so that instead of a vague error such as "connection refused", you get more useful errors such as "call service foo: connection refused".

Handle Type Assertion Failures

The single return value form of a type assertion will panic on an incorrect type. Therefore, always use the "comma ok" idiom.

Bad	Good
`t := i.(string)`	`t, ok := i.(string) if !ok { // handle the error gracefully }`

Avoid Mutable Globals

Avoid mutating global variables, instead opting for dependency injection. This applies to function pointers as well as other kinds of values.

Bad Good

// sign.go

var _timeNow = time.Now

func sign(msg string) string {
  now := _timeNow()
  return signWithTime(msg, now)
}

// sign.go

type signer struct {
  now func() time.Time
}

func newSigner() *signer {
  return &signer{
    now: time.Now,
  }
}

func (s *signer) Sign(msg string) string {
  now := s.now()
  return signWithTime(msg, now)
}

// sign_test.go

func TestSign(t *testing.T) {
  oldTimeNow := _timeNow
  _timeNow = func() time.Time {
    return someFixedTime
  }
  defer func() { _timeNow = oldTimeNow }()

  assert.Equal(t, want, sign(give))
}

// sign_test.go

func TestSigner(t *testing.T) {
  s := newSigner()
  s.now = func() time.Time {
    return someFixedTime
  }

  assert.Equal(t, want, s.Sign(give))
}

Avoid `init()`

Avoid init() where possible. When init() is unavoidable or desirable, code should attempt to:

Be completely deterministic, regardless of program environment or invocation.
Avoid depending on the ordering or side effects of other init() functions. While init() ordering is well-known, code can change, and thus relationships between init() functions can make code brittle and error-prone.
Avoid accessing or manipulating global or environment state, such as machine information, environment variables, working directory, program arguments/inputs, etc.
Avoid I/O, including both filesystem, network, and system calls.

Code that cannot satisfy these requirements likely belongs as a helper to be called as part of main() (or elsewhere in a program's lifecycle), or be written as part of main() itself. In particular, libraries that are intended to be used by other programs should take special care to be completely deterministic and not perform "init magic".

Bad Good

type Foo struct {
    // ...
}

var _defaultFoo Foo

func init() {
    _defaultFoo = Foo{
        // ...
    }
}

var _defaultFoo = Foo{
    // ...
}

// or, better, for testability:

var _defaultFoo = defaultFoo()

func defaultFoo() Foo {
    return Foo{
        // ...
    }
}

type Config struct {
    // ...
}

var _config Config

func init() {
    // Bad: based on current directory
    cwd, _ := os.Getwd()

    // Bad: I/O
    raw, _ := ioutil.ReadFile(
        path.Join(cwd, "config", "config.yaml"),
    )

    yaml.Unmarshal(raw, &_config)
}

type Config struct {
    // ...
}

func loadConfig() Config {
    cwd, err := os.Getwd()
    // handle err

    raw, err := ioutil.ReadFile(
        path.Join(cwd, "config", "config.yaml"),
    )
    // handle err

    var config Config
    yaml.Unmarshal(raw, &config)

    return config
}

Considering the above, some situations in which init() may be preferable or necessary might include:

Complex expressions that cannot be represented as single assignments.
Pluggable hooks, such as database/sql dialects, encoding type registries, etc.

Exit in Main

Go programs use os.Exit or log.Fatal* to exit immediately. (Panicking is not a good way to exit programs, please don't panic.)

Call one of os.Exit or log.Fatal* only in main(). All other functions should return errors to signal failure.

Bad Good

func main() {
  body := readFile(path)
  fmt.Println(body)
}

func readFile(path string) string {
  file, err := os.Open(path)
  if err != nil {
    log.Fatal(err)
  }

  payload, err := ioutil.ReadAll(file)
  if err != nil {
    log.Fatal(err)
  }

  return string(payload)
}

func main() {
  body, err := readFile(path)
  if err != nil {
    log.Fatal(err)
  }
  fmt.Println(body)
}

func readFile(path string) (string, error) {
  file, err := os.Open(path)
  if err != nil {
    return "", err
  }

  payload, err := ioutil.ReadAll(file)
  if err != nil {
    return "", err
  }

  return string(payload), nil
}

Rationale: Programs with multiple functions that exit present a few issues:

Non-obvious control flow: Any function can exit the program, so it becomes difficult to reason about the control flow.
Difficult to test: A function that exits the program will also exit the test calling it. This makes the function difficult to test and introduces risk of skipping other tests that have not yet been run by go test.
Skipped cleanup: When a function exits the program, it skips function calls enqueued with defer statements. This adds risk of skipping important cleanup tasks.

Exit Once

If possible, prefer to call os.Exit or log.Fatal at most once in your main(). If there are multiple error scenarios that halt program execution, put that logic under a separate function and return errors from it.

This has the effect of shortening your main() function and putting all key business logic into a separate, testable function.

Bad Good

package main

func main() {
  args := os.Args[1:]
  if len(args) != 1 {
    log.Fatal("missing file")
  }
  name := args[0]

  file, err := os.Open(name)
  if err != nil {
    log.Fatal(err)
  }
  defer file.Close()

  // If we call log.Fatal after this line,
  // file.Close will not be called.

  payload, err := ioutil.ReadAll(file)
  if err != nil {
    log.Fatal(err)
  }

  // ...
}

package main

func main() {
  err := run()
  if err != nil {
    log.Fatal(err)
  }
}

func run() error {
  args := os.Args[1:]
  if len(args) != 1 {
    return errors.New("missing file")
  }
  name := args[0]

  file, err := os.Open(name)
  if err != nil {
    return err
  }
  defer file.Close()

  payload, err := ioutil.ReadAll(file)
  if err != nil {
    return err
  }

  // ...
}

Performance

Performance-specific guidelines apply only to the hot path(s) of the application.

Prefer strconv over fmt

When converting primitives to/from strings, strconv is faster than fmt.

Bad Good

for i := 0; i < b.N; i++ {
  s := fmt.Sprint(rand.Int())
}

for i := 0; i < b.N; i++ {
  s := strconv.Itoa(rand.Int())
}

BenchmarkFmtSprint-4    143 ns/op    2 allocs/op

BenchmarkStrconv-4    64.2 ns/op    1 allocs/op

Avoid string-to-byte conversion

Do not create byte slices from a fixed string repeatedly. Instead, perform the conversion once and capture the result.

Bad Good

for i := 0; i < b.N; i++ {
  w.Write([]byte("Hello world"))
}

data := []byte("Hello world")
for i := 0; i < b.N; i++ {
  w.Write(data)
}

BenchmarkBad-4   50000000   22.2 ns/op

BenchmarkGood-4  500000000   3.25 ns/op

Prefer Specifying Container Capacity

Specify container capacity where possible in order to allocate memory for the container up front. This minimizes subsequent allocations (by copying and resizing of the container) as elements are added.

Specifying Map Capacity Hints

Where possible, provide capacity hints when initializing maps with make().

make(map[T1]T2, hint)

Providing a capacity hint to make() tries to right-size the map at initialization time, which reduces the need for growing the map and allocations as elements are added to the map.

Note that, unlike slices, map capacity hints do not guarantee complete, preemptive allocation, but are used to approximate the number of hashmap buckets required. Consequently, allocations may still occur when adding elements to the map, even up to the specified capacity.

Bad Good

m := make(map[string]os.FileInfo)

files, _ := ioutil.ReadDir("./files")
for _, f := range files {
    m[f.Name()] = f
}

files, _ := ioutil.ReadDir("./files")

m := make(map[string]os.FileInfo, len(files))
for _, f := range files {
    m[f.Name()] = f
}

`m` is created without a size hint; there may be more allocations at assignment time. `m` is created with a size hint; there may be fewer allocations at assignment time.

Specifying Slice Capacity

Where possible, provide capacity hints when initializing slices with make(), particularly when appending.

make([]T, length, capacity)

Unlike maps, slice capacity is not a hint: the compiler will allocate enough memory for the capacity of the slice as provided to make(), which means that subsequent append() operations will incur zero allocations (until the length of the slice matches the capacity, after which any appends will require a resize to hold additional elements).

Bad Good

for n := 0; n < b.N; n++ {
  data := make([]int, 0)
  for k := 0; k < size; k++{
    data = append(data, k)
  }
}

for n := 0; n < b.N; n++ {
  data := make([]int, 0, size)
  for k := 0; k < size; k++{
    data = append(data, k)
  }
}

BenchmarkBad-4    100000000    2.48s

BenchmarkGood-4   100000000    0.21s

Style

Import Grouping

In order to make imports orderly and clear, imported packages should be grouped in the following order from top to bottom:

Standard library
External dependencies
External dependencies from our org
Internal dependencies

Within each import group, imports should be sorted alphabetically.

Bad Good

import (
    "errors"
    "fmt"
    "github.com/rs/zerolog"
    "github.com/onflow/flow-go/ledger"
    "github.com/onflow/flow-go/ledger/complete/mtrie/trie"
    "github.com/onflow/flow-go/model/flow"
    "github.com/optakt/flow-dps/models/dps"
    "sync"
    "time"
)

import (
    "errors"
    "fmt"
    "sync"
    "time"

    "github.com/rs/zerolog"

    "github.com/onflow/flow-go/ledger"
    "github.com/onflow/flow-go/ledger/complete/mtrie/trie"
    "github.com/onflow/flow-go/model/flow"

    "github.com/optakt/flow-dps/models/dps"
)

Package Names

When naming packages, choose a name that is:

All lower-case. No capitals or underscores.
Does not need to be renamed using named imports at most call sites.
Short and succinct. Remember that the name is identified in full at every call site.
Not plural. For example, net/url, not net/urls.
Not "common", "util", "shared", or "lib". These are bad, uninformative names.

Function Names

We follow the Go community's convention of using MixedCaps for function names. An exception is made for test functions, which may contain underscores for the purpose of grouping related test cases, e.g., TestMyFunction_WhatIsBeingTested.

Function Grouping and Ordering

Functions should be sorted in rough call order.
Functions in a file should be grouped by receiver.

Therefore, exported functions should appear first in a file, after struct, const, var definitions.

A newXYZ()/NewXYZ() may appear after the type is defined, but before the rest of the methods on the receiver.

Since functions are grouped by receiver, plain utility functions should appear towards the end of the file.

Bad Good

func (s *something) Cost() {
  return calcCost(s.weights)
}

type something struct{ ... }

func calcCost(n []int) int {...}

func (s *something) Stop() {...}

func newSomething() *something {
    return &something{}
}

type something struct{ ... }

func newSomething() *something {
    return &something{}
}

func (s *something) Cost() {
  return calcCost(s.weights)
}

func (s *something) Stop() {...}

func calcCost(n []int) int {...}

Reduce Nesting

Code should reduce nesting where possible by handling error cases/special conditions first and returning early or continuing the loop. Reduce the amount of code that is nested multiple levels.

Bad Good

for _, v := range data {
  if v.F1 == 1 {
    v = process(v)
    err := v.Call()
    if err == nil {
      v.Send()
    } else {
      return err
    }
  } else {
    log.Printf("Invalid v: %v", v)
  }
}

for _, v := range data {
  if v.F1 != 1 {
    log.Printf("Invalid v: %v", v)
    continue
  }

  v = process(v)
  err := v.Call()
  if err != nil {
    return err
  }
  v.Send()
}

Unnecessary Else

If a variable is set in both branches of an if, it can be replaced with a single if.

Bad	Good
`var a int if b { a = 100 } else { a = 10 }`	`a := 10 if b { a = 100 }`

Top-level Variable Declarations

At the top level, use the standard var keyword. Do not specify the type, unless it is not the same type as the expression.

Bad	Good
`var _s string = F() func F() string { return "A" }`	`var _s = F() // Since F already states that it returns a string, we don't need to specify // the type again. func F() string { return "A" }`

Specify the type if the type of the expression does not match the desired type exactly.

type myError struct{}

func (myError) Error() string { return "error" }

func F() myError { return myError{} }

var _e error = F()
// F returns an object of type myError, but we want error.

Local Variable Declarations

Short variable declarations (:=) should be used if a variable is being set to some value explicitly.

Bad	Good
`var s = "foo"`	`s := "foo"`

However, there are cases where the default value is clearer when the varkeyword is used. Declaring Empty Slices, for example.

Bad Good

func f(list []int) {
  filtered := []int{}
  for _, v := range list {
    if v > 10 {
      filtered = append(filtered, v)
    }
  }
}

func f(list []int) {
  var filtered []int
  for _, v := range list {
    if v > 10 {
      filtered = append(filtered, v)
    }
  }
}

Reduce Scope of Variables

Where possible, reduce scope of variables. Do not reduce the scope if it conflicts with Reduce Nesting.

Bad Good

data, err := ioutil.ReadFile(name)
if err == nil {
  err = cfg.Decode(data)
  if err != nil {
    return err
  }

  fmt.Println(cfg)
  return nil
} else {
  return err
}

data, err := ioutil.ReadFile(name)
if err != nil {
   return err
}

err = cfg.Decode(data)
if err != nil {
  return err
}

fmt.Println(cfg)
return nil

Avoid Naked Parameters

Naked parameters in function calls can hurt readability. Add C-style comments(/* ... */) for parameter names when their meaning is not obvious.

Bad	Good
`// func printInfo(name string, isLocal, done bool) printInfo("foo", true, true)`	`// func printInfo(name string, isLocal, done bool) printInfo("foo", true /* isLocal /, true / done */)`

Better yet, replace naked bool types with custom types for more readable and type-safe code. This allows more than just two states (true/false) for that parameter in the future.

type Region int

const (
  UnknownRegion Region = iota
  Local
)

type Status int

const (
  StatusReady Status = iota + 1
  StatusDone
  // Maybe we will have a StatusInProgress in the future.
)

func printInfo(name string, region Region, status Status)

Initializing Structs

Use Field Names to Initialize Structs

You should almost always specify field names when initializing structs. This is enforced by go vet.

Bad	Good
`k := User{"John", "Doe", true}`	`k := User{ FirstName: "John", LastName: "Doe", Admin: true, }`

Exception: Field names may be omitted in test tables when there are 3 or fewer fields.

tests := []struct{
  op Operation
  want string
}{
  {Add, "add"},
  {Subtract, "subtract"},
}

Omit Zero Value Fields in Structs

When initializing structs with field names, omit fields that have zero values unless they provide meaningful context. Otherwise, let Go set these to zero values automatically.

Bad	Good
`user := User{ FirstName: "John", LastName: "Doe", MiddleName: "", Admin: false, }`	`user := User{ FirstName: "John", LastName: "Doe", }`

This helps reduce noise for readers by omitting values that are default in that context. Only meaningful values are specified.

Include zero values where field names provide meaningful context. For example, test cases in test tables can benefit from names of fields even when they are zero-valued.

tests := []struct{
  give string
  want int
}{
  {give: "0", want: 0},
  // ...
}

Use `var` for Zero Value Structs

When all the fields of a struct are omitted in a declaration, use the var form to declare the struct.

Bad	Good
`user := User{}`	`var user User`

This differentiates zero valued structs from those with non-zero fields similar to the distinction created for map initialization, and matches how we prefer to declare empty slices.

Initializing Struct References

Use &T{} instead of new(T) when initializing struct references so that it is consistent with the struct initialization.

Bad	Good
`sval := T{Name: "foo"} // inconsistent sptr := new(T) sptr.Name = "bar"`	`sval := T{Name: "foo"} sptr := &T{Name: "bar"}`

Initializing Maps

Prefer make(..) for empty maps, and maps populated programmatically. This makes map initialization visually distinct from declaration, and it makes it easy to add size hints later if available.

Bad Good

var (
  // m1 is safe to read and write;
  // m2 will panic on writes.
  m1 = map[T1]T2{}
  m2 map[T1]T2
)

var (
  // m1 is safe to read and write;
  // m2 will panic on writes.
  m1 = make(map[T1]T2)
  m2 map[T1]T2
)

Declaration and initialization are visually similar. Declaration and initialization are visually distinct.

Where possible, provide capacity hints when initializing maps with make(). See Specifying Map Capacity Hints for more information.

On the other hand, if the map holds a fixed list of elements, use map literals to initialize the map.

Bad	Good
`m := make(map[T1]T2, 3) m[k1] = v1 m[k2] = v2 m[k3] = v3`	`m := map[T1]T2{ k1: v1, k2: v2, k3: v3, }`

The basic rule of thumb is to use map literals when adding a fixed set of elements at initialization time, otherwise use make (and specify a size hint if available).

Patterns

Functional Options

Functional options is a pattern in which you declare an opaque Option type that records information in some internal struct. You accept a variadic number of these options and act upon the full information recorded by the options on the internal struct.

Use this pattern for optional arguments in constructors and other public APIs that you foresee needing to expand, especially if you already have three or more arguments on those functions.

Bad Good

// package db

func Open(
  addr string,
  cache bool,
  logger *zap.Logger
) (*Connection, error) {
  // ...
}

// package db

type Option interface {
  // ...
}

func WithCache(c bool) Option {
  // ...
}

func WithLogger(log *zap.Logger) Option {
  // ...
}

// Open creates a connection.
func Open(
  addr string,
  opts ...Option,
) (*Connection, error) {
  // ...
}

The cache and logger parameters must always be provided, even if the user wants to use the default.

db.Open(addr, db.DefaultCache, zap.NewNop())
db.Open(addr, db.DefaultCache, log)
db.Open(addr, false /* cache */, zap.NewNop())
db.Open(addr, false /* cache */, log)

Options are provided only if needed.

db.Open(addr)
db.Open(addr, db.WithLogger(log))
db.Open(addr, db.WithCache(false))
db.Open(
  addr,
  db.WithCache(false),
  db.WithLogger(log),
)

Our suggested way of implementing this pattern is with an Option interface that holds an unexported method, recording options on an unexported options struct.

type options struct {
  cache  bool
  logger *zap.Logger
}

type Option interface {
  apply(*options)
}

type cacheOption bool

func (c cacheOption) apply(opts *options) {
  opts.cache = bool(c)
}

func WithCache(c bool) Option {
  return cacheOption(c)
}

type loggerOption struct {
  Log *zap.Logger
}

func (l loggerOption) apply(opts *options) {
  opts.logger = l.Log
}

func WithLogger(log *zap.Logger) Option {
  return loggerOption{Log: log}
}

// Open creates a connection.
func Open(
  addr string,
  opts ...Option,
) (*Connection, error) {
  options := options{
    cache:  defaultCache,
    logger: zap.NewNop(),
  }

  for _, o := range opts {
    o.apply(&options)
  }

  // ...
}

Note that there's a method of implementing this pattern with closures, but we believe that the pattern above provides more flexibility for authors and is easier to debug and test for users. In particular, it allows options to be compared against each other in tests and mocks, versus closures where this is impossible. Further, it lets options implement other interfaces, including fmt.Stringer which allows for user-readable string representations of the options.

Go Code Style Guidelines

Guidelines

Copy Slices and Maps at Boundaries

Receiving Slices and Maps

Returning Slices and Maps

Start Enums at One

Error Types

Error Wrapping

Handle Type Assertion Failures

Avoid Mutable Globals

Avoid init()

Exit in Main

Exit Once

Performance

Prefer strconv over fmt

Avoid string-to-byte conversion

Prefer Specifying Container Capacity

Specifying Map Capacity Hints

Specifying Slice Capacity

Style

Import Grouping

Package Names

Function Names

Function Grouping and Ordering

Reduce Nesting

Unnecessary Else

Top-level Variable Declarations

Local Variable Declarations

Reduce Scope of Variables

Avoid Naked Parameters

Initializing Structs

Use Field Names to Initialize Structs

Omit Zero Value Fields in Structs

Use var for Zero Value Structs

Initializing Struct References

Initializing Maps

Patterns

Functional Options

Avoid `init()`

Use `var` for Zero Value Structs