AdGuard Go Team Development Guidelines

Following this document is obligatory for all new Go code. Some of the rules aren't enforced as thoroughly or remain broken in old code, but this is still the place to find out about what we want our code to look like and how to improve it.

The rules are mostly sorted in the alphabetical order.

Code

§ Avoid break and continue with labels. Most of the time the code can be rewritten without them, and most of the time the resulting new code is also clearer.
§ Always recover from panics in new goroutines. Preferably in the very first statement. If all you want there is a log message, use log.OnPanic.
§ Avoid fallthrough. It makes it harder to rearrange cases, to reason about the code, and also to switch the code to a handler approach, if that becomes necessary later.
§ Avoid goto.
§ Avoid init and use explicit initialization functions instead.
§ Avoid lazy initialization. Constructors should validate their arguments and return meaningful errors.
§ Avoid new, especially with structs, unless a temporary value is needed, for example when checking the type of an error using errors.As.
§ Avoid packages reflect and unsafe unless absolutely necessary. Always provide a comment explaining why you are using it.

§ Be aware of and strive to shorten resource scopes.

For example, if you have a long function that makes an HTTP request and defers the close of the body at the beginning and then does more stuff, it's better to factor out the HTTP request and the response parsing into a separate method. Same goes for mutexes.

That is, do not do this:

// Bad!  Resource r is only closed at the end of the very long function.
func Foo() (err error) {
    r, err := open()
    check(err)
    defer r.close()

    v, err := decode(r)
    check(err)

    // Lots of slow stuff with v. r is only closed once Foo exits.
}

Do this instead:

// Good, r is closed after loadV returns.
func Foo() (err error) {
    v, err := loadV()
    check(err)

    // Lots of slow stuff with v.
}

func loadV() (v *V, err error) {
    r, err := open()
    check(err)
    defer r.close()

    return decode(r)
}

§ Be aware of structure alignment as well as the number of pointer bytes. Exceptions could be made if a suboptimal alignment produces significantly better readability.

// Bad!  Lots of padding between the uint64s and bools. Pointers are at the
// end of the structure.
type bad struct {
    a uint64
    b bool
    c uint64
    d bool
    e uint64
    f bool
    g uint64
    h bool

    y *otherType
    z *otherType
}

// Good. The padding is minimized, and the pointers are closer to the top.
type good struct {
    y *otherType
    z *otherType

    g uint64
    e uint64
    c uint64
    a uint64

    h bool
    f bool
    d bool
    b bool
}

Use the fieldalignment analyzer when unsure.

§ Check against empty strings like this:
```
if s == "" {
    // …
}
```
Except when the check is done to then use the first character:
```
if len(s) > 0 {
    c := s[0]
}
```

§ Context values should be considered immutable. Do not use modifications of context values as a means of communicating something up the stack. That is, do not do this:

// Bad!  Function outer expects inner to mutate logEntry.
func outer(ctx context.Context, /* … */) {
    logEntry := &LogEntry{}
    ctx = contextWithLogEntry(ctx, logEntry)

    inner(ctx, /* … */)

    if logEntry.Success {
        // …
    }
}

// Bad!  Function inner mutates logEntry just so that outer could receive
// that change.
func inner(ctx context.Context, /* … */) {
    logEntry := logEntryFromContext(ctx)
    logEntry.Success = true

    // …
}

§ Don't rely only on file names for build tags to work. Always add build tags as well.
§ Don't use fmt.Sprintf where a more structured approach to string conversion could be used. For example, netutil.JoinHostPort, net.JoinHostPort or url.(*URL).String.
§ Don't use naked returns.
§ Eschew external dependencies, including transitive, unless absolutely necessary.
§ In templates, put spaces between the delimiters ({{ and }} by default) and the content, because when the minus sign is used to remove whitespace, it's an error to omit these spaces.
§ No name shadowing, including of predeclared identifiers, since it can often lead to subtle bugs, especially with errors. This rule does not apply to struct fields, since they are always used together with the name of the struct value, so there isn't any confusion.
§ Prefer build constraints to runtime.GOOS.
§ Prefer constants to variables where possible. Avoid global variables unless necessary. Use constant errors instead of errors.New.
§ Prefer defining Foo.String and ParseFoo in terms of Foo.MarshalText and Foo.UnmarshalText correspondingly and not the other way around.
§ Prefer to use pointers to structs in function arguments, including method receivers, unless there is a reason to do the opposite. Among valid reasons are:
- the struct is small, which typically means less than a few machine words;
- the struct is read-only, like netip.Addr or time.Time;
- the struct is immediately used by an external API that requires a non-pointer struct;
- the method implements a FooMarshaler kind of interface, and so a non-pointer receiver is required (see staticcheck issue 911).
§ Prefer to return structs and accept interfaces.
§ Prefer to use named functions for goroutines.
§ Prefer to use package golang.org/x/sys and not package syscall, as the latter is frozen.
§ Program code lines should not be longer than one hundred (100) columns. For comments, see the text guidelines.

Don't forget to also set the tab width in your editor's settings.
§ Use linters. make go-lint, if the project has one. A minimum of go vet, errcheck, and staticcheck if the project does not.

§ When returning an error from a function that also returns a non-nilable type, for example a time.Time, a time.Duration, or a netip.Addr, spell the empty value explicitly to show that the value is invalid.

So, do this:

func fooTime() (t time.Time, err error) {
    err = bar()
    if err != nil {
        return time.Time{}, err
    }

    // …
}

and not this:

func fooTime() (t time.Time, err error) {
    err = bar()
    if err != nil {
        // Bad!  This could be read as t being valid, which it is not.
        return t, err
    }

    // …
}

§ Write logs and error messages in lowercase only to make it easier to grep logs and error messages without using the -i flag.
§ Write static interface type checks like this, including the standard comment:
```
// type check
var _ ifaceType = (*implType)(nil)
```
Put them right before the declaration of the first method implementing the interface in question.

Commenting

Error Handling

§ Add context to errors but avoid duplication. For example, os.Open always adds the path to its error, so this is redundant:

// Bad!  Will duplicate the file name.
f, err := os.Open(fileName)
if err != nil {
    return fmt.Errorf("opening %q: %w", fileName, err)
}

If a function returns enough context, or a deferred helper is used, document that. Prefer to use a standard comment across a project. For example:

err = f()
if err != nil {
    // Don't wrap the error, because it's informative enough as is.
    return err
}

§ Avoid having multiple errors in a function. In situations when it's not feasible, use meaningful names. For example, closeErr for errors from Close() or testErr for subtest errors.

§ Avoid using the word error inside error messages.

// BAD!
err = foo()
if err != nil {
    return fmt.Errorf("error while calling foo: %w", err)
}

Just provide the action instead:

// Good.
err = foo()
if err != nil {
    return fmt.Errorf("performing foo: %w", err)
}

§ Parsing functions should include the invalid input into the error message, unless the input is too big.
§ Use only lowercase unless you have to reference an identifier in code or the project has its own conventions regarding uppercase letters.
§ Use panic only to indicate critical assertion failures. Do not use panics for normal error handling.
§ Use utilities from the github.com/AdguardTeam/golibs/testutil package when necessary.

Formatting

§ Consider putting empty lines between documented struct fields and interface methods to improve readability:

// FooConfig is the configuration for a single foo.
type FooConfig struct {
    // ID is the unique ID of foo.
    ID FooID

    // Name is the human-readable name of this foo.
    Name string

    // Timeout is the timeout used for all frobulation operations
    // performed by this foo.
    Timeout time.Duration
}

§ Decorate break, continue, return, and other terminating statements with empty lines unless it's the only statement in that block.
§ Don't group type declarations together. Unlike with blocks of consts, where a iota may be used or where all constants belong to a certain type, there is no reason to group types.

§ Don't mix horizontal and vertical placement of groups of arguments and return values in calls and definitions of functions and methods. That is, either this:

func f(a, b, c T) {
    // …
}

err := f(a, b, c)

Or, when the arguments are too long, this:

func functionWithALongName(
    firstArgumentWithALongName typeWithALongName,
    secondArgumentWithALongName otherTypeWithALongName,
    thirdArgumentWithALongName thirdTypeWithALongName,
) {
    // …
}

err := functionWithALongName(
    firstArgumentWithALongName,
    secondArgumentWithALongName,
    thirdArgumentWithALongName,
)

Or, with a call with a struct literal:

err := functionWithALongName(arg, structType{
    field1: val1,
    field2: val2,
})

But never this:

// Bad!
func functionWithALongName(firstArgumentWithALongName typeWithALongName,
    secondArgumentWithALongName otherTypeWithALongName,
    thirdArgumentWithALongName thirdTypeWithALongName) {
    // …
}

// Bad!
err := functionWithALongName(firstArgumentWithALongName,
    secondArgumentWithALongName,
    thirdArgumentWithALongName,
)

§ Don't write non-test code with more than four (4) levels of indentation. Just like Linus said, plus an additional level for an occasional error check or struct initialization.

The exception proving the rule is the table-driven test code, where an additional level of indentation is allowed.
§ Group require.* blocks together with the preceding related statements, but separate from the following assert.* and unrelated requirements.
```
val, ok := valMap[key]
require.True(t, ok)
require.NotNil(t, val)

assert.Equal(t, expected, val)
```
§ Put deferred calls of destructors, for example f.Close(), into the same paragraph as constructors. This is an exception to the paragraph rule.
```
f, err := os.Open(fileName)
if err != nil {
    return err
}
defer func() { processError(f.Close()) }()
```

§ Split numbers with more than four (4) digits into triplets using underscores:

const (
    max8  = 255
    max16 = 65_535
    max32 = 4_294_967_295
)

§ Start a new paragraph after the final closing curly brace of a block. So this:

if a == b {
    // …
}

for i := 0; i < N; i++ {
    // …
}

switch x {
    // …
}

and not this:

// Bad!
if a == b {
    // …
}
for i := 0; i < N; i++ {
    // …
}
switch x {
    // …
}

The exceptions are the final block inside a bigger block and the destructor defers.

§ Use gofumpt --extra -s.

§ When a function's definition becomes too long, first make the function's arguments vertically placed and only then do the same with the return values. That is, do this:

func functionWithALongName(
    argumentWithALongName typeWithALongName,
) (returnValueWithALongName typeWithALongName, err error) {
    // …
}

and not this:

// Bad!
func functionWithALongName(argumentWithALongName typeWithALongName) (
    returnValueWithALongName typeWithALongName,
    err error,
) {
    // …
}

§ Write switches with large lists of values in one case like this:

switch n {
case
    longValueName1,
    longValueName2,
    longValueName3,
    longValueName4,
    longValueName5:
    return true
default:
    return false
}

§ Write slices of struct like this:

ts := []T{{
    Field: Value0,
    // …
}, {
    Field: Value1,
    // …
}, {
    Field: Value2,
    // …
}}

Naming

§ Example files should be names example_test.go if there is one such file for the whole package and foo_example_test.go if there are several.
§ Don't use underscores in file and package names, unless they're build tags or for tests. This is to prevent accidental build errors with weird tags.
§ For brands or words with more than 1 capital letter, lowercase all letters: githubToken, not gitHubToken.

§ Methods that convert types for external data, such as configuration structures and response structures, into internal types should be called toInternal:

// toInternal converts a user object from the configuration file into
// a *User.
func (u *confUser) toInternal() (user *User) {
    // …
}

§ Name benchmarks and tests using the same convention as examples. For example:

func TestFunction(t *testing.T) { /* … */ }
func TestFunction_suffix(t *testing.T) { /* … */ }
func TestType_Method(t *testing.T) { /* … */ }
func TestType_Method_suffix(t *testing.T) { /* … */ }

§ Name context.Context helper functions that return values from the context FooFromContext and the ones that return a new contest with new values, ContextWithFoo or WithFoo. Just like in the standard library, the parent context should be called parent.

// ContextWithFoo returns a copy of the parent context with the value of `f`
// attached to it.
func ContextWithFoo(parent context.Context, f Foo) (ctx context.Context) {
    // …
}

// FooFromContext returns the Foo attached to the context.
func FooFromContext(parent context.Context) (f Foo, ok bool) {
    // …
}

§ Name parameters in interface definitions:

type Frobulator interface {
    Frobulate(f Foo, b Bar) (r Result, err error)
}

§ Unused arguments in anonymous functions must be called _:

v.onSuccess = func(_ int, msg string) {
    // …
}

§ Use named returns to improve readability of function signatures:

func split(data []*datum) (less, greater []*datum) {
    // …
}

§ Use names like ErrFoo for package-level error values and FooError for error types.

§ When naming a file which defines an entity, use singular nouns, unless the entity is some form of a container for other entities:

// File: client.go

package foo

type Client struct {
    // …
}

// File: clients.go

package foo

type Clients []*Client

// …

type ClientsWithCache struct {
    // …
}

Testing

§ If you write a fake implementation of an interface for a test, prefer to write it using callbacks:

// FakeReader …
type FakeReader struct {
    OnRead func(b []byte) (n int, err error)
}

// Read implements the io.Reader interface for *FakeReader.
func (r *FakeReader) Read(b []byte) (n int, err error) {
    return r.OnRead(b)
}

If the method must not be called in this particular test, place a single panic("not implemented") as its body:

r := &FakeReader{
    OnRead: func(b []byte) (n int, err error) { panic("not implemented") },
}

§ Prefer to put all tests into a separate test package. Tests in the same package that check unexported APIs should be put into a separate file named foo_internal_test.go.
§ Strive to make the test suite finish quickly. If you have long-running integration test or fuzzes, document them, put them into a separate Makefile target, and include them into the CI pipeline, but not the main make test target.
§ Use assert.NoError and require.NoError instead of assert.Nil and require.Nil on errors.
§ Use formatted helpers, like assert.Nilf or require.Nilf, instead of simple helpers when a formatted message is required.
§ Use functions like require.Foo instead of assert.Foo when the test cannot continue if the condition is false.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Go.md

Go.md

AdGuard Go Team Development Guidelines

Contents

Code

Commenting

Error Handling

Formatting

Naming

Testing

Recommended Reading

Files

Go.md

Latest commit

History

Go.md

File metadata and controls

AdGuard Go Team Development Guidelines

Contents

Code

Commenting

Error Handling

Formatting

Naming

Testing

Recommended Reading