Go (Golang) coding guidelines
- 1 Introduction
- 2 Guidelines
- 3 Performance
- 4 Style
- 4.1 Code comments
- 4.2 Line Length
- 4.3 Wrapping Function Signatures
- 4.4 Group Similar Declarations
- 4.5 Import Group Ordering
- 4.6 Package Names
- 4.7 Function Names
- 4.8 Import Aliasing
- 4.9 Function Grouping and Ordering
- 4.10 Reduce Nesting
- 4.11 Unnecessary Else
- 4.12 Top-level Variable Declarations
- 4.13 Use Field Names to initialize Structs
- 4.14 Local Variable Declarations
- 4.15 nil is a valid slice
- 4.16 Reduce Scope of Variables
- 4.17 Avoid Naked Parameters
- 4.18 Try to avoid bool parameters
- 4.19 fmt Verbs
- 4.20 Use Raw String Literals to Avoid Escaping
- 4.21 Initializing Struct References
- 4.22 Format Strings outside Printf
- 4.23 Naming Printf-style Functions
- 4.24 Go implementation of SQL
- 5 Patterns
- 5.1 Data-driven tests
- 5.1.1 Scope of Tested Behavior
- 5.2 Test Tables
- 5.3 Functional Options
- 5.1 Data-driven tests
This coding guide is inspired from the Uber Go style guide at https://github.com/uber-go/guide/blob/master/style.md, with modifications specific to Cockroach Labs.
Introduction
These guidelines are the conventions that govern our code. 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 Cockroach Labs. These rules exist to keep the code base manageable while still allowing engineers to use Go language features productively.
This documents idiomatic conventions in Go code that we follow at CRL. A lot of these are general guidelines for Go, while others extend upon external resources:
All code should be error-free when run through golint
and go vet
. We recommend setting up your editor to:
Run
crlfmt
on saveRun make lintshort to check for errors
You can find information in editor support for Go tools here: golang/go/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:
A pointer to some type-specific information. You can think of this as "type."
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.
Receivers and Interfaces
Methods with value receivers can be called on pointers as well as values.
For example,
type S struct {
data string
}
func (s S) Read() string {
return s.data
}
func (s *S) Write(str string) {
s.data = str
}
sVals := map[int]S{1: {"A"}}
// With a value you can call read, but not Write.
sVals[1].Read()
// This will not compile:
// sVals[0].Write("test")
sPtrs := map[int]*S{1: {"A"}}
// You can call both Read and Write using a pointer
sPtrs[1].Read()
sPtrs[1].Write("test")
Similarly, an interface can be satisfied by a pointer, even if the method has a value receiver.
type F interface {
f()
}
type S1 struct{}
func (s S1) f() {}
type S2 struct{}
func (s *S2) f() {}
s1Val := S1{}
s1Ptr := &S1{}
s2Val := S2{}
s2Ptr := &S2{}
var i F
i = s1Val
i = s1Ptr
i = s2Ptr
// The following doesn't compile, since s2Val is a value, and there is no value receiver for f.
// i = s2Val
Effective Go has a good write up on Pointers vs. Values.
Zero-value Mutexes are Valid
The zero-value of sync.Mutex
and sync.RWMutex
is valid, so you almost never need a pointer to a mutex.
Bad | Good |
---|---|
mu := new(sync.Mutex)
mu.Lock() |
If you use a struct by pointer, then the mutex can be a non-pointer field or, preferably, embedded directly into the struct.
Embed for private types or types that need to implement the Mutex interface. | For exported types, use a private lock. |
Copy Slices and Maps when necessary
Slices and maps contain pointers to the underlying data so be conscious of ownership when passing them as parameters or returning them as values.
On the other hand, do not unnecessarily copy slices - heap allocations are expensive and we want to avoid them when possible.
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.
If your API captures a slice by reference, document it in the function header.
Bad | Good |
---|---|
Returning Slices and Maps
Similarly, be wary of user modifications to maps or slices exposing internal state.
Your exported API should refrain from returning internal state that otherwise has accessors, e.g. when it is hidden behind a mutex.
For "internal" functions there is more flexibility, although the function name should reflect this (see e.g. log.closeFileLocked
in pkg/util/log
).
Bad | Good |
---|---|
Defer to Clean Up
Use defer to clean up resources such as files and locks.
Bad | Good |
---|---|
Defer has an extremely small overhead and should be avoided only if you can prove that your function execution time is in the order of nanoseconds. The readability win of using defers is worth the minuscule cost of using them. This is especially true for larger methods that have more than simple memory accesses, where the other computations are more significant than the defer
Sometimes, you may wish to Lock a mutex, do some work, and the Unlock it before the end of the function has been reached. If the work being done in this critical section has the potential to panic
, this could end up leaving the mutex locked if the executing code is wrapped in a panic handler, leading to deadlocks.
To protect against this, execute the code in a separate function so that defer
can still be used. This way, even if a panic occurs in the critical section, the mutex will be released.
Bad | Good |
---|---|
Channel Size is One or None
Channels should usually have a size of one or be unbuffered. By default, channels are unbuffered and have a size of zero. Any other size must be subject to a high level of scrutiny. Consider how the size is determined, what prevents the channel from filling up under load and blocking writers, and what happens when this occurs.
Bad | Good |
---|---|
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 |
---|---|
There are cases where using the zero value makes sense, for example when the zero value case is the desirable default behavior.
Error handling
The following sub-section are an excerpt from Error concepts and handling in CockroachDB . Read the full page for more background and context.
Performance
Performance-specific guidelines apply only to the hot path.
Be mindful that strconv is faster than fmt
When converting primitives to/from strings, strconv
is faster than fmt
.
Bad | Good |
---|---|
Avoid string-to-byte conversion
Do not create byte slices from a fixed string repeatedly: it causes a heap allocation and a copy. Instead, perform the conversion once and capture the result.
Bad | Good |
---|---|
Style
Code comments
See Code commenting guidelines .
Line Length
Format your code assuming it will be read in a window 100 columns wide. Wrap code at 100 characters and comments at 80 unless doing so makes the code less legible. These values assume tab width is 2 characters.
Wrapping Function Signatures
When wrapping function signatures that do not fit on one line, put the name, arguments, and return types on separate lines, with the closing )
at the same indentation as func
(this helps visually separate the indented arguments from the indented function body). Example:
If the arguments list is too long to fit on a single line, switch to one argument per line:
If the return types need to be wrapped, use the same rules:
Exception when omitting repeated types for consecutive arguments: short and related arguments (e.g. `start, end int64`) should either go on the same line or the type should be repeated on each line -- no argument should appear by itself on a line with no type (confusing and brittle when edited).
Group Similar Declarations
Go supports grouping similar declarations.
Bad | Good |
---|---|
This also applies to constants, variables, and type declarations.
Bad | Good |
---|---|
Only group related declarations. Do not group declarations that are unrelated.
Bad | Good |
---|---|
Groups are not limited in where they can be used. For example, you can use them inside of functions.
Bad | Good |
---|---|
Import Group Ordering
There should be two import groups:
Standard library
Everything else
This is the grouping applied by goimports by default.
Bad | Good |
---|---|
Package Names
When creating and naming packages, refer to the separate page Organization of code into packages .
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
.
Import Aliasing
Import aliasing must be used if the package name does not match the last element of the import path.
In all other scenarios, import aliases should be avoided unless there is a direct conflict between imports.
Bad | Good |
---|---|
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 |
---|---|
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 |
---|---|
Unnecessary Else
Avoid "else" clauses if they're not needed.
If a variable is set in both branches of an if, it can be replaced with a single if:
Bad | Good |
---|---|
A return statement inside of an if block means that you won't need an "else" clause.
Bad | Good |
---|---|
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 |
---|---|
Specify the type if the type of the expression does not match the desired type exactly.
Use Field Names to initialize Structs
You should almost always specify field names when initializing structs. This is now enforced by go vet
.
Bad | Good |
---|---|
Exception: Field names may be omitted in test tables when there are 3 or fewer fields.
Local Variable Declarations
Short variable declarations (:=
) should be used if a variable is being set to some value explicitly.
Bad | Good |
---|---|
However, there are cases where the default value is clearer when the var
keyword is use. Declaring Empty Slices, for example.
Bad | Good |
---|---|
nil is a valid slice
nil
is a valid slice of length 0. This means that,
You should not return a slice of length zero explicitly. Return nil
instead.
Bad | Good |
---|---|
To check if a slice is empty, always use len(s) == 0
. Do not check for nil
.
Bad | Good |
---|---|
The zero value (a slice declared with var
) is usable immediately without make()
.
Bad | Good |
---|---|
Reduce Scope of Variables
Where possible, reduce scope of variables. Do not reduce the scope if it conflicts with Reduce Nesting.
Bad | Good |
---|---|
If you need a result of a function call outside of the if, then you should not try to reduce the scope.
Bad | Good |
---|---|
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 |
---|---|
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.
Note: For `bool` constants, like for all literals, the comment should indicate the name of the parameter and does not depend on the argument value. *Do not* put a bang in the comment when commenting the `false` constant. Also, do not adapt the comment to negate the name of the parameter. For example:
Try to avoid bool
parameters
bool
arguments to functions are often dubious things, as they hint to code that essentially reads like:
This is not always the case. However, in cases where that is a fair assessment of the situation, consider whether the doSomething
function should exist at all.
In cases where the `bool` in question, along with other arguments, acts as a "knob" to the function consider replacing it with some type of "configuration" struct (for examples, see [Dave Cheney's treatment of the topic](https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis)). In situations where there's a single bool
param or the situation is less clear-cut, consider replacing the `bool` in question with an enum. For example:
is better than:
fmt Verbs
Prefer the most specific verb for your use. In other words, prefer to avoid %v
when possible. However, %v
is to be used when formatting bindings which might be nil and which do not already handle nil formatting. Notably, nil errors formatted as %s
will render as %!s(<nil>)
while nil errors formatted as %v
will render as <nil>
. Therefore, prefer %v
when formatting errors which are not known to be non-nil.
Use Raw String Literals to Avoid Escaping
Go supports raw string literals, which can span multiple lines and include quotes. Use these to avoid hand-escaped strings which are much harder to read.
Bad | Good |
---|---|
Initializing Struct References
Use &T{}
instead of new(T)
when initializing struct references so that it is consistent with the struct initialization.
Bad | Good |
---|---|
Format Strings outside Printf
If you declare format strings for Printf
-style functions outside a string literal, make them const
values.
This helps go vet
perform static analysis of the format string.
Bad | Good |
---|---|
Naming Printf-style Functions
When you declare a Printf
-style function, make sure that go vet
can detect it and check the format string.
This means that you should use pre-defined Printf
-style function names if possible. go vet
will check these by default. See Printf family for more information.
If using the pre-defined names is not an option, end the name you choose with f: Wrapf
, not Wrap
. go vet
can be asked to check specific Printf
-style names but they must end with f.
See also go vet: Printf family check.
Go implementation of SQL
When defining the result column labels of statements or virtual tables, apply the following general principles:
the casing of column labels must be consistent.
the same concept in different statements should be named using the same word.
For example, `variable` and `value` corresponds between SHOW ALL CLUSTER SETTINGS and SHOW SESSION ALL.the labels must be usable by a surrounding query without mandating quotes.
For example, `start_key` instead of `"Start Key"`. Also, avoid SQL keywords which would also need to be quoted: `table_name` instead of `"table"`, `index_name` instead of `"index"`, etc.
Never name a column `"user"`! The risk of mistake with the syntax special form `user` is too high. Use `user_name` instead.use underscores to separate words, for consistency with `information_schema`.
For example, `start_key` instead of `startkey`avoid abbreviations unless most uses of the word in spoken English use the abbreviation already.
For example, `id` for "identifier" is OK, but `loc` for "location" is not.for columns akin to "primary keys" (that identify the object being described by the remaining columns), words whose meaning is highly overloaded must be disambiguated in the label.
For example, `zone_id`, `job_id`, `table_id` instead of just `id` everywhere. `table_name`, `column_name`, etc instead of `name` everywhere.for database objects that can be referred to by two or more types of handles (e.g. by-ID or by-name), the column label must disambiguate which type of handle is being reported. This makes it possible to later report the other handles in additional columns.
For example, `table_name` instead of `table`, so that `table_id` can be added later.labels that reproduce data already present in an `information_schema` table or a prior `SHOW` statement should use the same labels as the `information_schema` table or `SHOW` statement, provided it matches the principles above already.
Patterns
Data-driven tests
We provide an alternative to table-driven tests (see next section) in CockroachDB via the datadriven test package. And example of the general format of these can be found on the RunTest
method comment in the datadriven package. One of the most used implementations of data-driven tests in the Cockroach codebase is “logictests” which execute SQL queries and statements and compare the query results to the content in the file.
Scope of Tested Behavior
The goal of every test is to observe some behavior of some functionality and assert it matches the expected behavior of that functionality. In particular this is committed and then re-run on every PR so that if any PR later causes unintended changes to that behavior – e.g. a modification in the functionality itself or a library or a refactor – the committed test will detect that change and fail, alerting those making the modifications to a potential regression.
However one thing a good test generally wants to avoid, but which can happen very easily when writing data driven tests, is observing and asserting behavior beyond the actual functionality being tested by including overly broad observations and assertions in the test. Doing so makes the test more brittle, adds toil to maintaining it and may ultimately defeat the actual purpose of the test completely – detecting regressions and stopping them from being merged unintentionally.
For contrived example, a test verifying the behavior of basic addition in a library might check `1 + 1`
equals the expected output 2
. But if that test did this by, say, including that result in a string that for some reason also included that current operating system version, e.g. “MacOS 15.2”, and comparing that, then this test would be quite brittle: the test becomes sensitive to unrelated factors outside the behavior it wanted to test such as OS upgrades. At best, constantly updating this test due to environmental changes outside the library would add unnecessary toil and friction but worse yet, engineers would likely become habituated to just blindly updating this test to make it pass every time it fails – as its failures and the need to update it become “expected”. However once this happens, any genuine regressions risk being missed, when the test is instead just blindly updated to reflect whatever new output is observed, as this has become the norm.
The above is of course an absurd and very clearly contrived example, but more subtle cases like this arise often, particularly when working logictests and --rewrite
where a query or trace thereof might include a table ID or range ID – making it now depend on factors like the total number of tables – or might enumerate all tables to verify an introspection utility.
A well-written logic test should choose its SELECT statement and its WHERE clause to ensure that the output includes and only includes and asserts output that is directly relevant to the tested behavior and fully specified by the test’s constructed conditions.
For example, if testing a builtin table that returned the names of all columns in each table in the cluster, one would want to select only the rows for tables the test itself created, or perhaps include specific, intentionally selected system
or internal tables to further verify the functionality correctly handles system and internal tables. Such a test however would not want to just select every system and internal table; doing so would create output that is determined by the total set of all system tables and the columns of each, not just the deliberately constructed and chosen test conditions, making the test overly broad in its assertion and brittle much like our contrived example above.
Test Tables
Use table-driven tests with subtests to avoid duplicating code when the core test logic is repetitive.
Bad | Good |
---|---|
Test tables make it easier to add context to error messages, reduce duplicate logic, and add new test cases.
We follow the convention that the slice of structs is referred to as tests
and each test case tt
. Further, we encourage explicating the input and output values for each test case with give
and want
prefixes.
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 |
---|---|
See also about functional options:
Copyright (C) Cockroach Labs.
Attention: This documentation is provided on an "as is" basis, without warranties or conditions of any kind, either express or implied, including, without limitation, any warranties or conditions of title, non-infringement, merchantability, or fitness for a particular purpose.