# Uber Go Style Guide - [Introduction](#introduction) - [Guidelines](#guidelines) - [Pointers to Interfaces](#pointers-to-interfaces) - [Verify Interface Compliance](#verify-interface-compliance) - [Receivers and Interfaces](#receivers-and-interfaces) - [Zero-value Mutexes are Valid](#zero-value-mutexes-are-valid) - [Copy Slices and Maps at Boundaries](#copy-slices-and-maps-at-boundaries) - [Defer to Clean Up](#defer-to-clean-up) - [Channel Size is One or None](#channel-size-is-one-or-none) - [Start Enums at One](#start-enums-at-one) - [Use `"time"` to handle time](#use-time-to-handle-time) - [Errors](#errors) - [Error Types](#error-types) - [Error Wrapping](#error-wrapping) - [Error Naming](#error-naming) - [Handle Errors Once](#handle-errors-once) - [Handle Type Assertion Failures](#handle-type-assertion-failures) - [Don't Panic](#dont-panic) - [Use go.uber.org/atomic](#use-gouberorgatomic) - [Avoid Mutable Globals](#avoid-mutable-globals) - [Avoid Embedding Types in Public Structs](#avoid-embedding-types-in-public-structs) - [Avoid Using Built-In Names](#avoid-using-built-in-names) - [Avoid `init()`](#avoid-init) - [Exit in Main](#exit-in-main) - [Exit Once](#exit-once) - [Use field tags in marshaled structs](#use-field-tags-in-marshaled-structs) - [Don't fire-and-forget goroutines](#dont-fire-and-forget-goroutines) - [Wait for goroutines to exit](#wait-for-goroutines-to-exit) - [No goroutines in `init()`](#no-goroutines-in-init) - [Performance](#performance) - [Prefer strconv over fmt](#prefer-strconv-over-fmt) - [Avoid repeated string-to-byte conversions](#avoid-repeated-string-to-byte-conversions) - [Prefer Specifying Container Capacity](#prefer-specifying-container-capacity) - [Style](#style) - [Avoid overly long lines](#avoid-overly-long-lines) - [Be Consistent](#be-consistent) - [Group Similar Declarations](#group-similar-declarations) - [Import Group Ordering](#import-group-ordering) - [Package Names](#package-names) - [Function Names](#function-names) - [Import Aliasing](#import-aliasing) - [Function Grouping and Ordering](#function-grouping-and-ordering) - [Reduce Nesting](#reduce-nesting) - [Unnecessary Else](#unnecessary-else) - [Top-level Variable Declarations](#top-level-variable-declarations) - [Prefix Unexported Globals with _](#prefix-unexported-globals-with-_) - [Embedding in Structs](#embedding-in-structs) - [Local Variable Declarations](#local-variable-declarations) - [nil is a valid slice](#nil-is-a-valid-slice) - [Reduce Scope of Variables](#reduce-scope-of-variables) - [Avoid Naked Parameters](#avoid-naked-parameters) - [Use Raw String Literals to Avoid Escaping](#use-raw-string-literals-to-avoid-escaping) - [Initializing Structs](#initializing-structs) - [Use Field Names to Initialize Structs](#use-field-names-to-initialize-structs) - [Omit Zero Value Fields in Structs](#omit-zero-value-fields-in-structs) - [Use `var` for Zero Value Structs](#use-var-for-zero-value-structs) - [Initializing Struct References](#initializing-struct-references) - [Initializing Maps](#initializing-maps) - [Format Strings outside Printf](#format-strings-outside-printf) - [Naming Printf-style Functions](#naming-printf-style-functions) - [Patterns](#patterns) - [Test Tables](#test-tables) - [Functional Options](#functional-options) - [Linting](#linting) ## Introduction Styles are the conventions that govern our code. The term style is a bit of a misnomer, since these conventions cover far more than just source file formatting—gofmt handles that for us. The goal of this guide is to manage this complexity by describing in detail the Dos and Don'ts of writing Go code at Uber. These rules exist to keep the code base manageable while still allowing engineers to use Go language features productively. This guide was originally created by [Prashant Varanasi](https://github.com/prashantv) and [Simon Newton](https://github.com/nomis52) as a way to bring some colleagues up to speed with using Go. Over the years it has been amended based on feedback from others. This documents idiomatic conventions in Go code that we follow at Uber. A lot of these are general guidelines for Go, while others extend upon external resources: 1. [Effective Go](https://go.dev/doc/effective_go) 2. [Go Common Mistakes](https://go.dev/wiki/CommonMistakes) 3. [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments) We aim for the code samples to be accurate for the two most recent minor versions of Go [releases](https://go.dev/doc/devel/release). All code should be error-free when run through `golint` and `go vet`. We recommend setting up your editor to: - Run `goimports` on save - Run `golint` and `go vet` to check for errors You can find information in editor support for Go tools here: https://go.dev/wiki/IDEsAndTextEditorPlugins ## Guidelines ### Pointers to Interfaces You almost never need a pointer to an interface. You should be passing interfaces as values—the underlying data can still be a pointer. An interface is two fields: 1. A pointer to some type-specific information. You can think of this as "type." 2. Data pointer. If the data stored is a pointer, it’s stored directly. If the data stored is a value, then a pointer to the value is stored. If you want interface methods to modify the underlying data, you must use a pointer. ### Verify Interface Compliance Verify interface compliance at compile time where appropriate. This includes: - Exported types that are required to implement specific interfaces as part of their API contract - Exported or unexported types that are part of a collection of types implementing the same interface - Other cases where violating an interface would break users
Bad | Good |
---|---|
```go type Handler struct { // ... } func (h *Handler) ServeHTTP( w http.ResponseWriter, r *http.Request, ) { ... } ``` | ```go type Handler struct { // ... } var _ http.Handler = (*Handler)(nil) func (h *Handler) ServeHTTP( w http.ResponseWriter, r *http.Request, ) { // ... } ``` |
Bad | Good |
---|---|
```go mu := new(sync.Mutex) mu.Lock() ``` | ```go var mu sync.Mutex mu.Lock() ``` |
Bad | Good |
---|---|
```go type SMap struct { sync.Mutex data map[string]string } func NewSMap() *SMap { return &SMap{ data: make(map[string]string), } } func (m *SMap) Get(k string) string { m.Lock() defer m.Unlock() return m.data[k] } ``` | ```go type SMap struct { mu sync.Mutex data map[string]string } func NewSMap() *SMap { return &SMap{ data: make(map[string]string), } } func (m *SMap) Get(k string) string { m.mu.Lock() defer m.mu.Unlock() return m.data[k] } ``` |
The `Mutex` field, and the `Lock` and `Unlock` methods are unintentionally part of the exported API of `SMap`. | The mutex and its methods are implementation details of `SMap` hidden from its callers. |
Bad | Good |
---|---|
```go func (d *Driver) SetTrips(trips []Trip) { d.trips = trips } trips := ... d1.SetTrips(trips) // Did you mean to modify d1.trips? trips[0] = ... ``` | ```go 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] = ... ``` |
Bad | Good |
---|---|
```go 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() ``` | ```go 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() ``` |
Bad | Good |
---|---|
```go p.Lock() if p.count < 10 { p.Unlock() return p.count } p.count++ newCount := p.count p.Unlock() return newCount // easy to miss unlocks due to multiple returns ``` | ```go p.Lock() defer p.Unlock() if p.count < 10 { return p.count } p.count++ return p.count // more readable ``` |
Bad | Good |
---|---|
```go // Ought to be enough for anybody! c := make(chan int, 64) ``` | ```go // Size of one c := make(chan int, 1) // or // Unbuffered channel, size of zero c := make(chan int) ``` |
Bad | Good |
---|---|
```go type Operation int const ( Add Operation = iota Subtract Multiply ) // Add=0, Subtract=1, Multiply=2 ``` | ```go type Operation int const ( Add Operation = iota + 1 Subtract Multiply ) // Add=1, Subtract=2, Multiply=3 ``` |
Bad | Good |
---|---|
```go func isActive(now, start, stop int) bool { return start <= now && now < stop } ``` | ```go func isActive(now, start, stop time.Time) bool { return (start.Before(now) || start.Equal(now)) && now.Before(stop) } ``` |
Bad | Good |
---|---|
```go func poll(delay int) { for { // ... time.Sleep(time.Duration(delay) * time.Millisecond) } } poll(10) // was it seconds or milliseconds? ``` | ```go func poll(delay time.Duration) { for { // ... time.Sleep(delay) } } poll(10*time.Second) ``` |
Bad | Good |
---|---|
```go // {"interval": 2} type Config struct { Interval int `json:"interval"` } ``` | ```go // {"intervalMillis": 2000} type Config struct { IntervalMillis int `json:"intervalMillis"` } ``` |
No error matching | Error matching |
---|---|
```go // package foo func Open() error { return errors.New("could not open") } // package bar if err := foo.Open(); err != nil { // Can't handle the error. panic("unknown error") } ``` | ```go // package foo var ErrCouldNotOpen = errors.New("could not open") func Open() error { return ErrCouldNotOpen } // package bar if err := foo.Open(); err != nil { if errors.Is(err, foo.ErrCouldNotOpen) { // handle the error } else { panic("unknown error") } } ``` |
No error matching | Error matching |
---|---|
```go // package foo func Open(file string) error { return fmt.Errorf("file %q not found", file) } // package bar if err := foo.Open("testfile.txt"); err != nil { // Can't handle the error. panic("unknown error") } ``` | ```go // package foo type NotFoundError struct { File string } func (e *NotFoundError) Error() string { return fmt.Sprintf("file %q not found", e.File) } func Open(file string) error { return &NotFoundError{File: file} } // package bar if err := foo.Open("testfile.txt"); err != nil { var notFound *NotFoundError if errors.As(err, ¬Found) { // handle the error } else { panic("unknown error") } } ``` |
Bad | Good |
---|---|
```go s, err := store.New() if err != nil { return fmt.Errorf( "failed to create new store: %w", err) } ``` | ```go s, err := store.New() if err != nil { return fmt.Errorf( "new store: %w", err) } ``` |
```plain failed to x: failed to y: failed to create new store: the error ``` | ```plain x: y: new store: the error ``` |
Description | Code |
---|---|
**Bad**: Log the error and return it Callers further up the stack will likely take a similar action with the error. Doing so causing a lot of noise in the application logs for little value. | ```go u, err := getUser(id) if err != nil { // BAD: See description log.Printf("Could not get user %q: %v", id, err) return err } ``` |
**Good**: Wrap the error and return it Callers further up the stack will handle the error. Use of `%w` ensures they can match the error with `errors.Is` or `errors.As` if relevant. | ```go u, err := getUser(id) if err != nil { return fmt.Errorf("get user %q: %w", id, err) } ``` |
**Good**: Log the error and degrade gracefully If the operation isn't strictly necessary, we can provide a degraded but unbroken experience by recovering from it. | ```go if err := emitMetrics(); err != nil { // Failure to write metrics should not // break the application. log.Printf("Could not emit metrics: %v", err) } ``` |
**Good**: Match the error and degrade gracefully If the callee defines a specific error in its contract, and the failure is recoverable, match on that error case and degrade gracefully. For all other cases, wrap the error and return it. Callers further up the stack will handle other errors. | ```go tz, err := getUserTimeZone(id) if err != nil { if errors.Is(err, ErrUserNotFound) { // User doesn't exist. Use UTC. tz = time.UTC } else { return fmt.Errorf("get user %q: %w", id, err) } } ``` |
Bad | Good |
---|---|
```go t := i.(string) ``` | ```go t, ok := i.(string) if !ok { // handle the error gracefully } ``` |
Bad | Good |
---|---|
```go func run(args []string) { if len(args) == 0 { panic("an argument is required") } // ... } func main() { run(os.Args[1:]) } ``` | ```go func run(args []string) error { if len(args) == 0 { return errors.New("an argument is required") } // ... return nil } func main() { if err := run(os.Args[1:]); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) } } ``` |
Bad | Good |
---|---|
```go // func TestFoo(t *testing.T) f, err := os.CreateTemp("", "test") if err != nil { panic("failed to set up test") } ``` | ```go // func TestFoo(t *testing.T) f, err := os.CreateTemp("", "test") if err != nil { t.Fatal("failed to set up test") } ``` |
Bad | Good |
---|---|
```go type foo struct { running int32 // atomic } func (f* foo) start() { if atomic.SwapInt32(&f.running, 1) == 1 { // already running… return } // start the Foo } func (f *foo) isRunning() bool { return f.running == 1 // race! } ``` | ```go type foo struct { running atomic.Bool } func (f *foo) start() { if f.running.Swap(true) { // already running… return } // start the Foo } func (f *foo) isRunning() bool { return f.running.Load() } ``` |
Bad | Good |
---|---|
```go // sign.go var _timeNow = time.Now func sign(msg string) string { now := _timeNow() return signWithTime(msg, now) } ``` | ```go // 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) } ``` |
```go // 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)) } ``` | ```go // sign_test.go func TestSigner(t *testing.T) { s := newSigner() s.now = func() time.Time { return someFixedTime } assert.Equal(t, want, s.Sign(give)) } ``` |
Bad | Good |
---|---|
```go // ConcreteList is a list of entities. type ConcreteList struct { *AbstractList } ``` | ```go // ConcreteList is a list of entities. type ConcreteList struct { list *AbstractList } // Add adds an entity to the list. func (l *ConcreteList) Add(e Entity) { l.list.Add(e) } // Remove removes an entity from the list. func (l *ConcreteList) Remove(e Entity) { l.list.Remove(e) } ``` |
Bad | Good |
---|---|
```go // AbstractList is a generalized implementation // for various kinds of lists of entities. type AbstractList interface { Add(Entity) Remove(Entity) } // ConcreteList is a list of entities. type ConcreteList struct { AbstractList } ``` | ```go // ConcreteList is a list of entities. type ConcreteList struct { list AbstractList } // Add adds an entity to the list. func (l *ConcreteList) Add(e Entity) { l.list.Add(e) } // Remove removes an entity from the list. func (l *ConcreteList) Remove(e Entity) { l.list.Remove(e) } ``` |
Bad | Good |
---|---|
```go var error string // `error` shadows the builtin // or func handleErrorMessage(error string) { // `error` shadows the builtin } ``` | ```go var errorMessage string // `error` refers to the builtin // or func handleErrorMessage(msg string) { // `error` refers to the builtin } ``` |
```go type Foo struct { // While these fields technically don't // constitute shadowing, grepping for // `error` or `string` strings is now // ambiguous. error error string string } func (f Foo) Error() error { // `error` and `f.error` are // visually similar return f.error } func (f Foo) String() string { // `string` and `f.string` are // visually similar return f.string } ``` | ```go type Foo struct { // `error` and `string` strings are // now unambiguous. err error str string } func (f Foo) Error() error { return f.err } func (f Foo) String() string { return f.str } ``` |
Bad | Good |
---|---|
```go type Foo struct { // ... } var _defaultFoo Foo func init() { _defaultFoo = Foo{ // ... } } ``` | ```go var _defaultFoo = Foo{ // ... } // or, better, for testability: var _defaultFoo = defaultFoo() func defaultFoo() Foo { return Foo{ // ... } } ``` |
```go type Config struct { // ... } var _config Config func init() { // Bad: based on current directory cwd, _ := os.Getwd() // Bad: I/O raw, _ := os.ReadFile( path.Join(cwd, "config", "config.yaml"), ) yaml.Unmarshal(raw, &_config) } ``` | ```go type Config struct { // ... } func loadConfig() Config { cwd, err := os.Getwd() // handle err raw, err := os.ReadFile( path.Join(cwd, "config", "config.yaml"), ) // handle err var config Config yaml.Unmarshal(raw, &config) return config } ``` |
Bad | Good |
---|---|
```go func main() { body := readFile(path) fmt.Println(body) } func readFile(path string) string { f, err := os.Open(path) if err != nil { log.Fatal(err) } b, err := io.ReadAll(f) if err != nil { log.Fatal(err) } return string(b) } ``` | ```go func main() { body, err := readFile(path) if err != nil { log.Fatal(err) } fmt.Println(body) } func readFile(path string) (string, error) { f, err := os.Open(path) if err != nil { return "", err } b, err := io.ReadAll(f) if err != nil { return "", err } return string(b), nil } ``` |
Bad | Good |
---|---|
```go package main func main() { args := os.Args[1:] if len(args) != 1 { log.Fatal("missing file") } name := args[0] f, err := os.Open(name) if err != nil { log.Fatal(err) } defer f.Close() // If we call log.Fatal after this line, // f.Close will not be called. b, err := io.ReadAll(f) if err != nil { log.Fatal(err) } // ... } ``` | ```go package main func main() { if err := run(); err != nil { log.Fatal(err) } } func run() error { args := os.Args[1:] if len(args) != 1 { return errors.New("missing file") } name := args[0] f, err := os.Open(name) if err != nil { return err } defer f.Close() b, err := io.ReadAll(f) if err != nil { return err } // ... } ``` |
Bad | Good |
---|---|
```go type Stock struct { Price int Name string } bytes, err := json.Marshal(Stock{ Price: 137, Name: "UBER", }) ``` | ```go type Stock struct { Price int `json:"price"` Name string `json:"name"` // Safe to rename Name to Symbol. } bytes, err := json.Marshal(Stock{ Price: 137, Name: "UBER", }) ``` |
Bad | Good |
---|---|
```go go func() { for { flush() time.Sleep(delay) } }() ``` | ```go var ( stop = make(chan struct{}) // tells the goroutine to stop done = make(chan struct{}) // tells us that the goroutine exited ) go func() { defer close(done) ticker := time.NewTicker(delay) defer ticker.Stop() for { select { case <-ticker.C: flush() case <-stop: return } } }() // Elsewhere... close(stop) // signal the goroutine to stop <-done // and wait for it to exit ``` |
There's no way to stop this goroutine. This will run until the application exits. | This goroutine can be stopped with `close(stop)`, and we can wait for it to exit with `<-done`. |
Bad | Good |
---|---|
```go func init() { go doWork() } func doWork() { for { // ... } } ``` | ```go type Worker struct{ /* ... */ } func NewWorker(...) *Worker { w := &Worker{ stop: make(chan struct{}), done: make(chan struct{}), // ... } go w.doWork() } func (w *Worker) doWork() { defer close(w.done) for { // ... case <-w.stop: return } } // Shutdown tells the worker to stop // and waits until it has finished. func (w *Worker) Shutdown() { close(w.stop) <-w.done } ``` |
Spawns a background goroutine unconditionally when the user exports this package. The user has no control over the goroutine or a means of stopping it. | Spawns the worker only if the user requests it. Provides a means of shutting down the worker so that the user can free up resources used by the worker. Note that you should use `WaitGroup`s if the worker manages multiple goroutines. See [Wait for goroutines to exit](#wait-for-goroutines-to-exit). |
Bad | Good |
---|---|
```go for i := 0; i < b.N; i++ { s := fmt.Sprint(rand.Int()) } ``` | ```go for i := 0; i < b.N; i++ { s := strconv.Itoa(rand.Int()) } ``` |
```plain BenchmarkFmtSprint-4 143 ns/op 2 allocs/op ``` | ```plain BenchmarkStrconv-4 64.2 ns/op 1 allocs/op ``` |
Bad | Good |
---|---|
```go for i := 0; i < b.N; i++ { w.Write([]byte("Hello world")) } ``` | ```go data := []byte("Hello world") for i := 0; i < b.N; i++ { w.Write(data) } ``` |
```plain BenchmarkBad-4 50000000 22.2 ns/op ``` | ```plain BenchmarkGood-4 500000000 3.25 ns/op ``` |
Bad | Good |
---|---|
```go m := make(map[string]os.FileInfo) files, _ := os.ReadDir("./files") for _, f := range files { m[f.Name()] = f } ``` | ```go files, _ := os.ReadDir("./files") m := make(map[string]os.DirEntry, 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. |
Bad | Good |
---|---|
```go for n := 0; n < b.N; n++ { data := make([]int, 0) for k := 0; k < size; k++{ data = append(data, k) } } ``` | ```go for n := 0; n < b.N; n++ { data := make([]int, 0, size) for k := 0; k < size; k++{ data = append(data, k) } } ``` |
```plain BenchmarkBad-4 100000000 2.48s ``` | ```plain BenchmarkGood-4 100000000 0.21s ``` |
Bad | Good |
---|---|
```go import "a" import "b" ``` | ```go import ( "a" "b" ) ``` |
Bad | Good |
---|---|
```go const a = 1 const b = 2 var a = 1 var b = 2 type Area float64 type Volume float64 ``` | ```go const ( a = 1 b = 2 ) var ( a = 1 b = 2 ) type ( Area float64 Volume float64 ) ``` |
Bad | Good |
---|---|
```go type Operation int const ( Add Operation = iota + 1 Subtract Multiply EnvVar = "MY_ENV" ) ``` | ```go type Operation int const ( Add Operation = iota + 1 Subtract Multiply ) const EnvVar = "MY_ENV" ``` |
Bad | Good |
---|---|
```go func f() string { red := color.New(0xff0000) green := color.New(0x00ff00) blue := color.New(0x0000ff) // ... } ``` | ```go func f() string { var ( red = color.New(0xff0000) green = color.New(0x00ff00) blue = color.New(0x0000ff) ) // ... } ``` |
Bad | Good |
---|---|
```go func (c *client) request() { caller := c.name format := "json" timeout := 5*time.Second var err error // ... } ``` | ```go func (c *client) request() { var ( caller = c.name format = "json" timeout = 5*time.Second err error ) // ... } ``` |
Bad | Good |
---|---|
```go import ( "fmt" "os" "go.uber.org/atomic" "golang.org/x/sync/errgroup" ) ``` | ```go import ( "fmt" "os" "go.uber.org/atomic" "golang.org/x/sync/errgroup" ) ``` |
Bad | Good |
---|---|
```go import ( "fmt" "os" nettrace "golang.net/x/trace" ) ``` | ```go import ( "fmt" "os" "runtime/trace" nettrace "golang.net/x/trace" ) ``` |
Bad | Good |
---|---|
```go 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{} } ``` | ```go 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 {...} ``` |
Bad | Good |
---|---|
```go for _, v := range data { if v.F1 == 1 { v = process(v) if err := v.Call(); err == nil { v.Send() } else { return err } } else { log.Printf("Invalid v: %v", v) } } ``` | ```go for _, v := range data { if v.F1 != 1 { log.Printf("Invalid v: %v", v) continue } v = process(v) if err := v.Call(); err != nil { return err } v.Send() } ``` |
Bad | Good |
---|---|
```go var a int if b { a = 100 } else { a = 10 } ``` | ```go a := 10 if b { a = 100 } ``` |
Bad | Good |
---|---|
```go var _s string = F() func F() string { return "A" } ``` | ```go 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" } ``` |
Bad | Good |
---|---|
```go // foo.go const ( defaultPort = 8080 defaultUser = "user" ) // bar.go func Bar() { defaultPort := 9090 ... fmt.Println("Default port", defaultPort) // We will not see a compile error if the first line of // Bar() is deleted. } ``` | ```go // foo.go const ( _defaultPort = 8080 _defaultUser = "user" ) ``` |
Bad | Good |
---|---|
```go type Client struct { version int http.Client } ``` | ```go type Client struct { http.Client version int } ``` |
Bad | Good |
---|---|
```go type A struct { // Bad: A.Lock() and A.Unlock() are // now available, provide no // functional benefit, and allow // users to control details about // the internals of A. sync.Mutex } ``` | ```go type countingWriteCloser struct { // Good: Write() is provided at this // outer layer for a specific // purpose, and delegates work // to the inner type's Write(). io.WriteCloser count int } func (w *countingWriteCloser) Write(bs []byte) (int, error) { w.count += len(bs) return w.WriteCloser.Write(bs) } ``` |
```go type Book struct { // Bad: pointer changes zero value usefulness io.ReadWriter // other fields } // later var b Book b.Read(...) // panic: nil pointer b.String() // panic: nil pointer b.Write(...) // panic: nil pointer ``` | ```go type Book struct { // Good: has useful zero value bytes.Buffer // other fields } // later var b Book b.Read(...) // ok b.String() // ok b.Write(...) // ok ``` |
```go type Client struct { sync.Mutex sync.WaitGroup bytes.Buffer url.URL } ``` | ```go type Client struct { mtx sync.Mutex wg sync.WaitGroup buf bytes.Buffer url url.URL } ``` |
Bad | Good |
---|---|
```go var s = "foo" ``` | ```go s := "foo" ``` |
Bad | Good |
---|---|
```go func f(list []int) { filtered := []int{} for _, v := range list { if v > 10 { filtered = append(filtered, v) } } } ``` | ```go func f(list []int) { var filtered []int for _, v := range list { if v > 10 { filtered = append(filtered, v) } } } ``` |
Bad | Good |
---|---|
```go if x == "" { return []int{} } ``` | ```go if x == "" { return nil } ``` |
Bad | Good |
---|---|
```go func isEmpty(s []string) bool { return s == nil } ``` | ```go func isEmpty(s []string) bool { return len(s) == 0 } ``` |
Bad | Good |
---|---|
```go nums := []int{} // or, nums := make([]int) if add1 { nums = append(nums, 1) } if add2 { nums = append(nums, 2) } ``` | ```go var nums []int if add1 { nums = append(nums, 1) } if add2 { nums = append(nums, 2) } ``` |
Bad | Good |
---|---|
```go err := os.WriteFile(name, data, 0644) if err != nil { return err } ``` | ```go if err := os.WriteFile(name, data, 0644); err != nil { return err } ``` |
Bad | Good |
---|---|
```go if data, err := os.ReadFile(name); err == nil { err = cfg.Decode(data) if err != nil { return err } fmt.Println(cfg) return nil } else { return err } ``` | ```go data, err := os.ReadFile(name) if err != nil { return err } if err := cfg.Decode(data); err != nil { return err } fmt.Println(cfg) return nil ``` |
Bad | Good |
---|---|
```go const ( _defaultPort = 8080 _defaultUser = "user" ) func Bar() { fmt.Println("Default port", _defaultPort) } ``` | ```go func Bar() { const ( defaultPort = 8080 defaultUser = "user" ) fmt.Println("Default port", defaultPort) } ``` |
Bad | Good |
---|---|
```go // func printInfo(name string, isLocal, done bool) printInfo("foo", true, true) ``` | ```go // func printInfo(name string, isLocal, done bool) printInfo("foo", true /* isLocal */, true /* done */) ``` |
Bad | Good |
---|---|
```go wantError := "unknown name:\"test\"" ``` | ```go wantError := `unknown error:"test"` ``` |
Bad | Good |
---|---|
```go k := User{"John", "Doe", true} ``` | ```go k := User{ FirstName: "John", LastName: "Doe", Admin: true, } ``` |
Bad | Good |
---|---|
```go user := User{ FirstName: "John", LastName: "Doe", MiddleName: "", Admin: false, } ``` | ```go user := User{ FirstName: "John", LastName: "Doe", } ``` |
Bad | Good |
---|---|
```go user := User{} ``` | ```go var user User ``` |
Bad | Good |
---|---|
```go sval := T{Name: "foo"} // inconsistent sptr := new(T) sptr.Name = "bar" ``` | ```go sval := T{Name: "foo"} sptr := &T{Name: "bar"} ``` |
Bad | Good |
---|---|
```go var ( // m1 is safe to read and write; // m2 will panic on writes. m1 = map[T1]T2{} m2 map[T1]T2 ) ``` | ```go 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. |
Bad | Good |
---|---|
```go m := make(map[T1]T2, 3) m[k1] = v1 m[k2] = v2 m[k3] = v3 ``` | ```go m := map[T1]T2{ k1: v1, k2: v2, k3: v3, } ``` |
Bad | Good |
---|---|
```go msg := "unexpected values %v, %v\n" fmt.Printf(msg, 1, 2) ``` | ```go const msg = "unexpected values %v, %v\n" fmt.Printf(msg, 1, 2) ``` |
Bad | Good |
---|---|
```go // func TestSplitHostPort(t *testing.T) host, port, err := net.SplitHostPort("192.0.2.0:8000") require.NoError(t, err) assert.Equal(t, "192.0.2.0", host) assert.Equal(t, "8000", port) host, port, err = net.SplitHostPort("192.0.2.0:http") require.NoError(t, err) assert.Equal(t, "192.0.2.0", host) assert.Equal(t, "http", port) host, port, err = net.SplitHostPort(":8000") require.NoError(t, err) assert.Equal(t, "", host) assert.Equal(t, "8000", port) host, port, err = net.SplitHostPort("1:8") require.NoError(t, err) assert.Equal(t, "1", host) assert.Equal(t, "8", port) ``` | ```go // func TestSplitHostPort(t *testing.T) tests := []struct{ give string wantHost string wantPort string }{ { give: "192.0.2.0:8000", wantHost: "192.0.2.0", wantPort: "8000", }, { give: "192.0.2.0:http", wantHost: "192.0.2.0", wantPort: "http", }, { give: ":8000", wantHost: "", wantPort: "8000", }, { give: "1:8", wantHost: "1", wantPort: "8", }, } for _, tt := range tests { t.Run(tt.give, func(t *testing.T) { host, port, err := net.SplitHostPort(tt.give) require.NoError(t, err) assert.Equal(t, tt.wantHost, host) assert.Equal(t, tt.wantPort, port) }) } ``` |
Bad | Good |
---|---|
```go func TestComplicatedTable(t *testing.T) { tests := []struct { give string want string wantErr error shouldCallX bool shouldCallY bool giveXResponse string giveXErr error giveYResponse string giveYErr error }{ // ... } for _, tt := range tests { t.Run(tt.give, func(t *testing.T) { // setup mocks ctrl := gomock.NewController(t) xMock := xmock.NewMockX(ctrl) if tt.shouldCallX { xMock.EXPECT().Call().Return( tt.giveXResponse, tt.giveXErr, ) } yMock := ymock.NewMockY(ctrl) if tt.shouldCallY { yMock.EXPECT().Call().Return( tt.giveYResponse, tt.giveYErr, ) } got, err := DoComplexThing(tt.give, xMock, yMock) // verify results if tt.wantErr != nil { require.EqualError(t, err, tt.wantErr) return } require.NoError(t, err) assert.Equal(t, want, got) }) } } ``` | ```go func TestShouldCallX(t *testing.T) { // setup mocks ctrl := gomock.NewController(t) xMock := xmock.NewMockX(ctrl) xMock.EXPECT().Call().Return("XResponse", nil) yMock := ymock.NewMockY(ctrl) got, err := DoComplexThing("inputX", xMock, yMock) require.NoError(t, err) assert.Equal(t, "want", got) } func TestShouldCallYAndFail(t *testing.T) { // setup mocks ctrl := gomock.NewController(t) xMock := xmock.NewMockX(ctrl) yMock := ymock.NewMockY(ctrl) yMock.EXPECT().Call().Return("YResponse", nil) _, err := DoComplexThing("inputY", xMock, yMock) assert.EqualError(t, err, "Y failed") } ``` |
Bad | Good |
---|---|
```go // package db func Open( addr string, cache bool, logger *zap.Logger ) (*Connection, error) { // ... } ``` | ```go // 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. ```go 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. ```go db.Open(addr) db.Open(addr, db.WithLogger(log)) db.Open(addr, db.WithCache(false)) db.Open( addr, db.WithCache(false), db.WithLogger(log), ) ``` |