Go review references
There is a lot of wisdom about Go scattered throughout its wiki, blog, GitHub issues, presentations by the Go team, and standard library documentation. The goal of this page is to collect that wisdom so it can be easily accessed and shared during code review.
The Google Go style guide is deliberately excluded from this list, as its goal is to be relevant for Go development inside Google, not Go development in general.
Problem | Reference |
---|---|
Bad error type name. | Errors#naming |
Bad error variable name. | Errors#naming |
Bad package name. | CodeReviewComments#package-names |
Bad receiver name. | CodeReviewComments#receiver-names |
Bad subtest name. | TestComments#choose-human-readable-subtest-names |
C imported without build tags. | proverbs#c |
Cancelled instead of canceled. | go/golang#11626 |
Cleverness. | proverbs#clear, simplicity-is-complicated#12 |
Code not formatted by goimports. | CodeReviewComments#gofmt; VS Code |
Comment is not a sentence. | CodeReviewComments#comment-sentences |
Constructor has functional options. | lesiw.dev/go/opts |
Constructor is unnecessary. | proverbs#zero |
Data race. | articles/race_detector |
Dot import. | CodeReviewComments#import-dot |
Error not decorated. | blog/go1.13-errors, proverbs#handle |
Error string begins with a capital letter. | CodeReviewComments#error-strings |
Filters instead of for . | robpike/filter#6, proverbs#clear |
Follows “golang-standards.” | golang-standards/project-layout#117, blog/organizing-go-code |
Function can fail, but does not return error/ok. | CodeReviewComments#in-band-errors |
Function forces goroutine. | CodeReviewComments#synchronous-functions |
Function returns repeated unnamed type. | CodeReviewComments#named-result-parameters |
go:linkname. | golang/go#67401 |
Happy path not left-aligned. | CodeReviewComments#indent-error-flow; related blog |
Interface declared but not used. | CodeReviewComments#interfaces |
Interface is overly specific. | proverbs#interface |
interface{} used instead of any. | modernize, golang/go#33232, golang/go#49884 |
Large import, small use case. | proverbs#copy |
Machine-readable comment with leading space. | go/golang#37974 |
map[T]bool instead of map[T]struct{} for a set. | blog/range-functions |
math/rand used for cryptographic purposes. | CodeReviewComments#crypto-rand |
Missing defer (e.g. Close). | effective_go#defer |
Missing or bad doc comment. | doc/comment |
Missing or bad package comment. | CodeReviewComments#package-comments |
Mutex used for orchestration. | proverbs#sync |
Name not mixedCaps. | effective_go#mixed-caps |
Name w/o capitalized initials. | CodeReviewComments#initialisms |
Non-nil empty slice. | CodeReviewComments#declaring-empty-slices |
Optimization without benchmarks. | testing#Benchmarks |
panic() used as error handling. | effective_go#errors; performance implications |
Parsing error strings. | blog/go1.13-errors#errors-and-package-apis |
pkg_test. | CodeReviewComments#import-dot |
Receiver should (not) be pointer. | CodeReviewComments#receiver-type |
Reducible slice copy. | slices#Clone |
Reducible slice cut. | SliceTricks#cut |
Reducible slice delete. | slices#Delete |
Reducible slice filter. | slices#DeleteFunc |
Reducible slice insert (n elements). | SliceTricks#insertvector |
Reducible slice insert (one element). | slices#Insert |
Redundant type or function name. | CodeReviewComments#package-names |
Shared memory. | effective_go#sharing, proverbs#mem |
Side-effectual import not in main package. | CodeReviewComments#import-blank |
Struct type used only once. | tricks.slide#9 |
syscall imported without build tags. | proverbs#syscall |
Test compares exact error strings. | TestComments#test-error-semantics |
Test compares fields instead of structs. | TestComments#compare-full-structures |
Test creates temp file without t.TempDir. | testing#T.TempDir |
Test doesn’t specify what failed. | CodeReviewComments#useful-test-failures |
Test got/want is hard to parse. | TestComments#print-diffs |
Test helper not marked as t.Helper. | TestComments#mark-test-helpers |
Test message does not identify function. | TestComments#identify-the-function |
Test message does not identify input. | TestComments#identify-the-input |
Test message does not include got or want. | TestComments#got-before-want |
Test muddled for the sake of parallelism. | golang/go#24335, proverbs#clear |
Test performs cleanup in a defer. | testing#T.Cleanup |
Test relies on implementation details. | TestComments#compare-stable-results |
Test sets env var without t.Setenv. | testing#T.Setenv |
Test stops (calls t.Fatal) prematurely. | TestComments#keep-going |
Test uses assert libraries. | TestComments#assert-libraries |
Test uses reflect.DeepEqual. | TestComments#equality-comparison-and-diffs |
Tests with repeated code. | TableDrivenTests |
Unclear when goroutine will exit. | CodeReviewComments#goroutine-lifetimes; related blog |
Unhandled/swallowed error. | CodeReviewComments#handle-errors |
Unnecessarily public package. | go1.4#internalpackages |
Unnecessary pointer. | CodeReviewComments#pass-values |
Unnecessary use of any. | proverbs#any |
Unnecessary use of cgo. | proverbs#cgo, cgo is not Go |
Unnecessary use of reflection. | proverbs#reflect; try generics or code gen w/ go/types |
Unnecessary use of unsafe. | proverbs#unsafe |
Variable name too long. | CodeReviewComments#variable-names |
Workers instead of semaphores. | golang.org/x/sync/semaphore |