From c098353acb6d8ee2dcf995e2e2ae67cf35afe30d Mon Sep 17 00:00:00 2001 From: "Matt T. Proud" Date: Wed, 18 Mar 2026 15:56:05 +0100 Subject: [PATCH] go: Export the latest version of the internal guide. (#965) The update revises guidance around line length, import renaming, context usage, sentinel placement, and interface usage. --- go/best-practices.md | 393 +++++++++++++++++++++++++++++++++++++++++++ go/decisions.md | 149 ++++++---------- go/guide.md | 2 +- 3 files changed, 444 insertions(+), 100 deletions(-) diff --git a/go/best-practices.md b/go/best-practices.md index 5206148..bb70a09 100644 --- a/go/best-practices.md +++ b/go/best-practices.md @@ -1096,6 +1096,75 @@ fmt.Println(err3) // err3-1 err2-1 err1 err2-2 err3-2 Therefore, in order for error text to mirror error chain structure, prefer placing the `%w` verb at the end with the form `[...]: %w`. + + +#### Sentinel error placement + +An exception to this rule is when wrapping sentinel errors. A sentinel error is +an error that serves as a primary categorization of a failure. This helps +observers quickly understand the nature of a failure (such as "not found" or +"invalid argument") without having to parse the entire error message. +Identifying that error type as early as possible in the error string is +beneficial. + +Examples of sentinel errors include os errors (e.g., [`os.ErrInvalid`]) and +package-level errors. + +In these cases, placing the `%w` verb at the beginning of the error string can +improve readability by immediately identifying the category of the error. + +```go +// Good: +package parser + +var ErrParse = fmt.Errorf("parse error") + +// This is another package error that could be returned. +var ErrParseInvalidHeader = fmt.Errorf("%w: invalid header", ErrParse) + +func parseHeader() error { + err := checkHeader() + return fmt.Errorf("%w: invalid character in header: %v", ErrParseInvalidHeader, err) +} + +err := fmt.Errorf("%w: couldn't find fortune database: %v", ErrInternal, err) +``` + +Placing the status at the beginning ensures that the most relevant categorical +information is most prominent. + +```go +// Bad: +package parser + +var ErrParse = fmt.Errorf("parse error") + +// This is another package error that could be returned. +var ErrParseInvalidHeader = fmt.Errorf("%w: invalid header", ErrParse) + +func parseHeader() error { + err := checkHeader() + return fmt.Errorf("invalid character in header: %v: %w", err, ErrParseInvalidHeader) +} + +var ErrInternal = status.Error(codes.Internal, "internal") +err2 := fmt.Errorf("couldn't find fortune database: %v: %w", err, ErrInternal) +``` + +When you place it at the end, it makes it harder to identify the error category +when reading the error text, as it's buried in the specific error details. + +[`os.ErrInvalid`]: https://pkg.go.dev/os#ErrInvalid + +See also: + +* [Go Tip #48: Error Sentinel Values] +* [Go Tip #106: Error Naming Conventions] + +[commentary]: decisions#commentary +[Go Tip #48: Error Sentinel Values]: https://google.github.io/styleguide/go/index.html#gotip +[Go Tip #106: Error Naming Conventions]: https://google.github.io/styleguide/go/index.html#gotip + ### Logging errors @@ -3597,3 +3666,327 @@ See also: * [Go Tip #36: Enclosing Package-Level State](https://google.github.io/styleguide/go/index.html#gotip) * [Go Tip #80: Dependency Injection Principles](https://google.github.io/styleguide/go/index.html#gotip) + + + +## Interfaces + +Interfaces in Go are powerful but can be overused or misunderstood. Because Go +interfaces are satisfied implicitly, they are a structural tool rather than a +declarative one. The following guidance provides the best practices for how to +design and return interfaces in Go without over-engineering your codebase. + +Refer to [Decisions' section on interfaces](decisions#interfaces) for a summary. + + + +### Avoid unnecessary interfaces + +The most common mistake is creating an interface before a +[real need](guide#simplicity) exists. + +1. **Don’t confuse the concept with the keyword:** Just because you are + designing a "service" or a "repository" or similar pattern doesn't mean you + need a named interface type (e.g., `type Service interface`). Focus on the + behavior and its concrete implementation first. + +2. **Reuse existing interfaces:** If an interface already exists, especially in + generated code, like a RPC client or server, use it ([testing RPC]). Do not + wrap a generated RPC code in a new, manual interface just for the sake of + abstraction or testing. [Use real transports](#use-real-transports) instead. + +3. **Don't define back doors only for tests:** Do not export a [test double] + implementation of an interface from an API that consumes it. Instead, prefer + to design the API so that it can be tested using the [public API] of the + real implementation. + + Every exported type increases the cognitive load for the reader. When you + export a test double alongside the real implementation, you force the reader + to understand three entities (the interface, the real implementation, and + the test double) instead of one. + + Export an interface for a test double when you have a + [material need](guide#least-mechanism) to support substitution. + +When it does make sense to create an interface: + +1. **Multiple implementations:** When there are two or more concrete types that + must be handled by the same logic (e.g., something that operates with both + [json.Encoder](https://pkg.go.dev/encoding/json#Encoder) and + [gob.GobEncoder](https://pkg.go.dev/encoding/gob#GobEncoder)), the API + consumer could define an interface. + +2. **Decoupling packages:** To break circular dependencies between two packages + (see an [example](#avoiding-circular-dependencies)), an API producer could + define an interface. + + **Caution:** Carefully observe guidance on [Package Size](#package-size). + Introducing interfaces to break dependency cycles is often a signal of + improperly structured packages. + +3. **Hiding complexity:** When a concrete type has a massive API surface, but a + specific function only needs one or two methods, an API consumer may define + an interface. + + + +### Interface ownership and visibility + +1. **Do not export interface types unnecessarily:** If an interface is only + used internally within a package to satisfy a specific logic flow, keep the + interface unexported. Exporting an interface commits you to maintaining that + API for external callers. + +2. **The consumer defines the interface:** In Go, interfaces generally belong + in the package that uses them, not the package that implements them. The + consumer should define only the methods they actually use + [GoTip #78: Minimal Viable Interfaces], adhering to the idea that + [the bigger the interface, the weaker the abstraction](https://go-proverbs.github.io/). + + There are common scenarios where it often makes sense for the producer (the + package providing the logic) to export the interface: + + * **The interface is the product:** When a package’s primary purpose is to + provide a common protocol that many different implementations must + follow, the producer defines the interface. For example, + [io.Writer](https://pkg.go.dev/io#Writer), + [hash.Hash](https://pkg.go.dev/hash#Hash). The concept of "protocol" + includes aspects like [documentation](#documentation) about critical + behaviors (e.g., expected use case, edge cases, concurrency) that need + to be centrally and canonically explicated. Another prominent example of + this is generated interfaces from protobuf. It doesn't abstract a + specific behavior, it defines a boundary. Its purpose is to ensure that + your server implementation exactly matches the schema defined in the + `.proto` file. Here, the interface serves as a rigid legal contract + between the service and its clients. + + For large systems, if the interface lives inside a huge implementation + package, every client is forced to import the entire world just to + reference the interface. You may define the interface in a standalone, + implementation-free package, avoiding unnecessary symbols and potential + circular dependencies. This is also the same philosophy used by + generated code from protobuf. + + * **Prevent interface bloat:** In large codebases, maintenance becomes + difficult if numerous packages utilize the same `AuthService` while each + defining an identical `type Authorizer interface`. While Go often favors + [a little copying over a little dependency](https://go-proverbs.github.io/), + keep in mind that maintaining perfectly mirrored interfaces (see point + above) across many packages can create an unnecessary burden. + + * **Resolve circular dependency:** see + [an example](#avoiding-circular-dependencies) below. + + + +### Designing effective interfaces + +1. **Keep interfaces small:** The larger the interface, + [the harder it is to implement and to write code that takes advantage of it](https://go-proverbs.github.io/). + Small interfaces are easier to compose into larger ones if needed. + +2. **Documentation:** Treat every interface as the "user manual" for your + abstraction. The depth of your documentation should be proportional to the + interface's cognitive load, not just the count of its methods. Whether an + interface has ten methods or a single `Write` of + [io.Writer](https://pkg.go.dev/io#Writer), if a programmer is expected to + interact with that type, the API must be documented thoroughly. + + * **Single-method interfaces:** documentation on the type itself is + usually sufficient (e.g., io.Writer). Explain its contract, edge cases, + and expected errors. + * **Multi-method interfaces:** each individual method requires its own + documentation. + * **Unexported interfaces:** consider documenting them anyway. They are + often the glue that holds complex internal logic together, and because + they are invisible to external users, they can easily become mystery + code for future maintainers (including your future self). + +3. **Accept interfaces, return concrete types:** Returning a concrete type + allows the caller to use the full functionality of the value without being + locked into a specific interface abstraction + [GoTip #49: Accept Interfaces, Return Concrete Types]. + +There are several common scenarios where returning an interface is the idiomatic +choice: + +1. **Encapsulation:** While interfaces cannot strictly hide exported methods + (as they remain accessible via type assertions), returning an interface is a + powerful tool for limiting the default API surface and guiding the caller's + behavior.. The most common example is the `error` interface; you + [almost never return a concrete error type](decisions#errors) like + `*MyCustomError`. + + Consider a `ThrottledReader` that implements `io.Reader` but also has a + `Refill` method for internal bucket management. Returning the concrete + `*ThrottledReader` invites the caller to manage the bucket manually, which + could lead to race conditions or broken rate-limiting logic. By returning an + interface, you tell the caller that your only job is to consume this reader. + If you try to cast this back to a `ThrottledReader` to `Refill` the internal + bucket, you are breaking the contract. + + ```go + // Good: + type ThrottledReader struct { + source io.Reader + limit int // bytes per second + balance int // current allowance of bytes + lastRefill time.Time + } + + // Read implements the io.Reader interface with rate-limiting logic. + func (t *ThrottledReader) Read(p []byte) (int, error) { ... } + + // Refill manually adds tokens to the bucket. + // INTERNAL USE ONLY: Calling this from outside breaks the rate limit logic. + func (t *ThrottledReader) Refill(amount int) { + t.balance = min(t.balance + amount, t.limit) + } + + // New returns the io.Reader with rate-limiting. + func New(r io.Reader, bytesPerSec int) io.Reader { + return &ThrottledReader{ + source: r, + limit: bytesPerSec, + balance: bytesPerSec, // start with a full bucket + lastRefill: time.Now(), + } + } + ``` + + This raises a natural question: if `Refill` is dangerous, why export it at + all? In complex systems, you often need internal orchestration. For example, + an `AggregateReader` manages multiple `ThrottledReader` values to ensure + total bandwidth across all streams stays under a global limit. This + coordinator needs to call Refill to distribute tokens, but the non-power + user processing the data should never see that capability. + + **Caution:** Before returning an interface to hide implementation, ask: + "Would a user calling these extra methods actually break the system's + integrity or meaningfully limit maintainability?" If the extra details allow + the user to bypass safety checks, or if exposing the concrete type makes it + impossible to change the underlying provider later without a breaking + change, you may return an interface. Do not rotely encapsulate without + reason. + +2. **Certain patterns:** If a function is designed to return one of several + different concrete types based on decisions made at runtime, it must return + an interface. This is commonly true with command, chaining, factory, and + [strategy](https://en.wikipedia.org/wiki/Strategy_pattern) patterns. + Consider this code that selects which encoder to use based the requested + format: + + ```go + // Good: + func NewWriter(format string) io.Writer { + switch format { + case "json": + return &jsonWriter{} + case "xml": + return &xmlWriter{} + default: + return &textWriter{} + } + } + ``` + + The following example of a chaining API demonstrates how returning an + interface enables polymorphic behavior. By allowing callers to use either + `client.Do(req)` or `client.WithAuth("token").Do(req)`, you can swap + implementations without breaking the calling code. + + ```go + // Good: + type Client interface { + WithAuth(token string) Client + Do(req *Request) error + } + ``` + + These patterns are guidelines, not rules. Avoid forcing an interface if a + single, robust concrete type can handle the abstraction internally. For + example, the standard [database/sql](https://pkg.go.dev/database/sql#DB) + library exports a single concrete `DB` type instead of forcing an interface + to handle types like `MySQLDB` and `OracleDB`. + +3. **Avoiding circular + dependencies:** If returning a concrete type would require importing + a package that already imports your current package, you must return an + interface to break the circular dependency. + + For example: + + ```go + // Bad: + package app + + import "myproject/plugin" + + type Config struct { + APIKey string + } + + func Start() { + p := plugin.New() + } + ``` + + ```go + // Bad: + package plugin + + import "myproject/app" // ERROR: Import cycle! + + func New() *app.Config { + return &app.Config{APIKey: "secret"} + } + ``` + + In this case, `plugin`'s `New` cannot return `*app.Config` because it would + create a circular import. To break this, we use the fact that interfaces are + satisfied implicitly. We move the "contract" to a neutral place or have the + producer return an interface that the consumer already understands. + + If `plugin`'s `New` returns an interface instead of the concrete + `*app.Config` struct, it no longer needs to import package `app`. + + ```go + package plugin + + type Configurer interface { + APIKey() string + } + + type localConfig struct { + key string + } + + func (c localConfig) APIKey() string { return c.key } + + // New returns the interface Configurer instead of the concrete app.Config + func New() Configurer { + return &localConfig{key: "secret"} + } + ``` + + ```go + package app + + import "myproject/plugin" + + func Start() { + conf := plugin.New() // 'conf' is now a Configurer interface + fmt.Println(conf.APIKey()) + } + ``` + + **Caution:** Carefully observe guidance on [Package Size](#package-size). + Introducing interfaces to break dependency cycles is often a signal of + improperly structured packages. Consolidated packages are often preferred + over too many too small packages that fail to stand on their own. + +[GoTip #78: Minimal Viable Interfaces]: https://google.github.io/styleguide/go/index.html#gotip +[GoTip #49: Accept Interfaces, Return Concrete Types]: https://google.github.io/styleguide/go/index.html#gotip +[testing RPC]: https://codelabs.developers.google.com/grpc/getting-started-grpc-go#3 +[test double]: https://abseil.io/resources/swe-book/html/ch13.html +[public API]: https://abseil.io/resources/swe-book/html/ch12.html#test_via_public_apis diff --git a/go/decisions.md b/go/decisions.md index 2a423f4..831d83b 100644 --- a/go/decisions.md +++ b/go/decisions.md @@ -144,6 +144,7 @@ See also: [Go blog post about package names](https://go.dev/blog/package-names). * Short (usually one or two letters in length) * Abbreviations for the type itself * Applied consistently to every receiver for that type +* Not an underscore; omit the name if it is unused Long Name | Better Name --------------------------- | ------------------------- @@ -529,22 +530,16 @@ decoration should generally be avoided. ### Comment line length -Ensure that commentary is readable from source even on narrow screens. +There is no fixed [line length] for comments in Go. -When a comment gets too long, it is recommended to wrap it into multiple -single-line comments. When possible, aim for comments that will read well on an -80-column wide terminal, however this is not a hard cut-off; there is no fixed -line length limit for comments in Go. The standard library, for example, often -chooses to break a comment based on punctuation, which sometimes leaves the -individual lines closer to the 60-70 character mark. +[line length]: guide#line-length -There is plenty of existing code in which comments exceed 80 characters in -length. This guidance should not be used as a justification to change such code -as part of a readability review (see [consistency](guide#consistency)), though -teams are encouraged to opportunistically update comments to follow this -guideline as a part of other refactors. The primary goal of this guideline is to -ensure that all Go readability mentors make the same recommendation when and if -recommendations are made. +Long comment lines should be wrapped to ensure that source is readable in tools +which do not perform automatic wrapping of comment lines. If you are uncertain +where to wrap, 80 or 100 columns are common choices. However, this is not a hard +cut-off; there are situations where breaking a long literal text is harmful. +There is no requirement for the specific column width at which wrapping occurs. +Aim to be [consistent](guide#consistency) within a file. See this [post from The Go Blog on documentation] for more on commentary. @@ -564,17 +559,12 @@ See this [post from The Go Blog on documentation] for more on commentary. // if it helps rather than hinders. ``` -Avoid comments that will wrap repeatedly on small screens, which is a poor -reader experience. +Avoid comments that fit large amounts of text onto a single line, which is a +poor reader experience. ```text # Bad: -// This is a comment paragraph. The length of individual lines doesn't matter in -Godoc; -// but the choice of wrapping causes jagged lines on narrow screens or in code -review, -// which can be annoying, especially when in a comment block that will wrap -repeatedly. +// This is a comment paragraph. While some code editors and viewers will wrap the paragraph for the reader, others will display a very long line that will overflow most windows and require users to scroll horizontally. In addition, even on a screen capable of displaying the entire line, it is easier to read a narrower paragraph than very wide one. // // Don't worry too much about the long URL: // https://supercalifragilisticexpialidocious.example.com:8080/Animalia/Chordata/Mammalia/Rodentia/Geomyoidea/Geomyidae/ @@ -2227,88 +2217,47 @@ See also: -Go interfaces generally belong in the package that *consumes* values of the -interface type, not a package that *implements* the interface type. The -implementing package should return concrete (usually pointer or struct) types. -That way, new methods can be added to implementations without requiring -extensive refactoring. See [GoTip #49: Accept Interfaces, Return Concrete Types] -for more details. +Avoid creating interfaces until a [real need](guide#simplicity) exists. Focus on +the required behavior rather than just abstract named patterns like "service" or +"repository" and the like. -Do not export a [test double][double types] implementation of an interface from -an API that consumes it. Instead, design the API so that it can be tested using -the [public API] of the [real implementation]. See -[GoTip #42: Authoring a Stub for Testing] for more details. Even when it is not -feasible to use the real implementation, it may not be necessary to introduce an -interface fully covering all methods in the real type; the consumer can create -an interface containing only the methods it needs, as demonstrated in -[GoTip #78: Minimal Viable Interfaces]. +* Do not wrap RPC clients in new manual interfaces just for the sake of + abstraction or testing. + [Use real transports](best-practices#use-real-transports) instead + ([testing RPC]). -To test packages that use Stubby RPC clients, use a real client connection. If a -real server cannot be run in the test, Google's internal practice is to obtain a -real client connection to a local [test double] using the internal rpctest -package (coming soon!). +* Do not define back doors or export [test double] implementations of an + interface solely for testing. Prefer testing via the [public API] of the + real implementation instead. -Do not define interfaces before they are used (see -[TotT: Code Health: Eliminate YAGNI Smells][tott-438] ). Without a realistic -example of usage, it is too difficult to see whether an interface is even -necessary, let alone what methods it should contain. +Design interfaces to be small for easier implementation and composition +([GoTip #78: Minimal Viable Interfaces]). Document interfaces appropriately +including their contract, edge cases, and expected errors. Keep interface types +unexported if they are only used internally within a package. -Do not use interface-typed parameters if the users of the package do not need to -pass different types for them. +The consumer of the interface should define it (not the package implementing the +interface), ensuring it includes only the methods they actually use. The +producer package may export the interface if the interface is the product (a +common protocol) to prevent interface redefinition bloat. -Do not export interfaces that the users of the package do not need. +There is an adage: Functions should take interfaces as arguments but return +concrete types ([GoTip #49: Accept Interfaces, Return Concrete Types]). +Returning concrete types allows the caller to have access to every public method +and field of that specific implementation, not just the subset of methods +defined in a pre-chosen interface. The caller can still pass that concrete +result into any other function that expects an interface. Sometimes returning an +interface is acceptable for encapsulation (e.g., `error` interface), and certain +constructs like command, chaining, factory, and +[strategy](https://en.wikipedia.org/wiki/Strategy_pattern) patterns. -**TODO:** Write a more in-depth doc on interfaces and link to it here. +Deeper discussion on interfaces exists in the +[Best Practices' section on interfaces](best-practices#interfaces). -[GoTip #42: Authoring a Stub for Testing]: https://google.github.io/styleguide/go/index.html#gotip -[GoTip #49: Accept Interfaces, Return Concrete Types]: https://google.github.io/styleguide/go/index.html#gotip [GoTip #78: Minimal Viable Interfaces]: https://google.github.io/styleguide/go/index.html#gotip -[real implementation]: best-practices#use-real-transports +[GoTip #49: Accept Interfaces, Return Concrete Types]: https://google.github.io/styleguide/go/index.html#gotip +[testing RPC]: https://codelabs.developers.google.com/grpc/getting-started-grpc-go#3 +[test double]: https://abseil.io/resources/swe-book/html/ch13.html [public API]: https://abseil.io/resources/swe-book/html/ch12.html#test_via_public_apis -[double types]: https://abseil.io/resources/swe-book/html/ch13.html#techniques_for_using_test_doubles -[test double]: https://abseil.io/resources/swe-book/html/ch13.html#basic_concepts -[tott-438]: https://testing.googleblog.com/2017/08/code-health-eliminate-yagni-smells.html - -```go -// Good: -package consumer // consumer.go - -type Thinger interface { Thing() bool } - -func Foo(t Thinger) string { ... } -``` - -```go -// Good: -package consumer // consumer_test.go - -type fakeThinger struct{ ... } -func (t fakeThinger) Thing() bool { ... } -... -if Foo(fakeThinger{...}) == "x" { ... } -``` - -```go -// Bad: -package producer - -type Thinger interface { Thing() bool } - -type defaultThinger struct{ ... } -func (t defaultThinger) Thing() bool { ... } - -func NewThinger() Thinger { return defaultThinger{ ... } } -``` - -```go -// Good: -package producer - -type Thinger struct{ ... } -func (t Thinger) Thing() bool { ... } - -func NewThinger() Thinger { return Thinger{ ... } } -``` @@ -2770,13 +2719,15 @@ Exceptions are: the generated server type, which implements `grpc.ServerStream`. See [gRPC Generated Code documentation](https://grpc.io/docs/languages/go/generated-code/). -* In entrypoint functions (see below for examples of such functions), use - [`context.Background()`] or, for tests, - [`tb.Context()`](https://pkg.go.dev/testing#TB.Context). +* In test functions (e.g. `TestXXX`, `BenchmarkXXX`, `FuzzXXX`), where the + context comes from + [`(testing.TB).Context()`](https://pkg.go.dev/testing#TB.Context). + +* In other entrypoint functions (see below for examples of such functions), + use [`context.Background()`]. * In binary targets: `main` * In general purpose code and libraries: `init` - * In tests: `TestXXX`, `BenchmarkXXX`, `FuzzXXX` > **Note**: It is very rare for code in the middle of a callchain to require > creating a base context of its own using [`context.Background()`]. Always diff --git a/go/guide.md b/go/guide.md index 961469f..46557bf 100644 --- a/go/guide.md +++ b/go/guide.md @@ -257,7 +257,7 @@ if err := doSomething(); err == nil { // if NO error } ``` -[Table-driven testing]: https://github.com/golang/go/wiki/TableDrivenTests +[Table-driven testing]: https://go.dev/wiki/TableDrivenTests [error handling]: https://go.dev/blog/errors-are-values ["boosting"]: best-practices#signal-boost