Rule | Description | Example | KPI |
---|---|---|---|
S1000-Use plain channel send or receive instead of single-case select | Select statements with a single case can be replaced with a simple send or receive. | //non-compliant code select { case x := <-ch: fmt.Println(x) } //compliant code x := <-ch fmt.Println(x) | Functionality |
S1003-Replace call to strings.Index with strings.Contains | Replace call to strings.Index with strings.Contains | //non-compliant code if strings.Index(x, y) != -1 {} //compliant code if strings.Contains(x, y) {} | Functionality, Robustness |
S1004-Replace call to bytes.Compare with bytes.Equal | Replace call to bytes.Compare with bytes.Equal | //non-compliant code if bytes.Compare(x, y) == 0 {} //compliant code if bytes.Equal(x, y) {} | Functionality, Robustness |
S1007-Simplify regular expression by using raw string literal | Raw string literals use ` instead of " and do not support any escape sequences. This means that the backslash (\) can be used freely, without the need of escaping. Since regular expressions have their own escape sequences, raw strings can improve their readability. | //non-compliant code regexp.Compile("\\A(\\w+) profile: total \\d+\\n\\z") //compliant code regexp.Compile(`\A(\w+) profile: total \d+\n\z`) | Maintainability, Functionality |
S1008-Simplify returning boolean expression | Simplify returning boolean expression | //non-compliant code if return true } return false //compliant code if return true } return false | Maintainability |
S1009-Omit redundant nil check on slices | The len function is defined for all slices, even nil ones, which have a length of zero. It is not necessary to check if a slice is not nil before checking that its length is not zero. | //non-compliant code if x != nil && len(x) != 0 {} //compliant code if len(x) != 0 {} | Functionality, Conceptual Integrity |
S1010-Omit default slice index | When slicing, the second index defaults to the length of the value, making s[n:len(s)] and s[n:] equivalent. | Functionality | |
S1011-Use a single append to concatenate two slices | Use a single append to concatenate two slices | //non-compliant code for _, e := range y { x = append(x, e) } //compliant code x = append(x, y...) | Functonality, Efficiency |
S1016-Use a type conversion instead of manually copying struct fields | Two struct types with identical fields can be converted between each other. In older versions of Go, the fields had to have identical struct tags. Since Go 1.8, however, struct tags are ignored during conversions. It is thus not necessary to manually copy every field individually. | //non-compliant code var x T1 y := T2{ Field1: x.Field1, Field2: x.Field2, } //compliant code var x T1 y := T2(x) | Functonality, Efficiency |
S1017-Replace manual trimming with strings.TrimPrefix | Instead of using strings.HasPrefix and manual slicing, use the strings.TrimPrefix function. If the string doesn't start with the prefix, the original string will be returned. Using strings.TrimPrefix reduces complexity, and avoids common bugs, such as off-by-one mistakes. | //non-compliant code if strings.HasPrefix(str, prefix) { str = str[len(prefix):] } //compliant code str = strings.TrimPrefix(str, prefix) | Functionality |
S1018-Use copy for sliding elements | copy() permits using the same source and destination slice, even with overlapping ranges. This makes it ideal for sliding elements in a slice. | //non-compliant code for i := 0; i < n; i++ { bs[i] = bs[offset+i] } //compliant code copy(bs[:n], bs[offset:]) | Functionality |
S1019-Simplify make call by omitting redundant arguments | The make function has default values for the length and capacity arguments. For channels and maps, the length defaults to zero. Additionally, for slices the capacity defaults to the length. | Functionality | |
S1024-Replace x.Sub(time.Now()) with time.Until(x) | The time.Until helper has the same effect as using x.Sub(time.Now()) but is easier to read. | //non-compliant code x.Sub(time.Now()) //compliant code time.Until(x) | Functionality |
S1025-Don't use fmt.Sprintf("%s", x) unnecessarily | In many instances, there are easier and more efficient ways of getting a value's string representation. Whenever a value's underlying type is a string already, or the type has a String method, they should be used directly. | //non-compliant code fmt.Sprintf("%s", x) fmt.Sprintf("%s", y) fmt.Sprintf("%s", z) //compliant code x string(y) z.String() | Functionality |
S1028-Simplify error construction with fmt.Errorf | Simplify error construction with fmt.Errorf | //non-compliant code errors.New(fmt.Sprintf(...)) //compliant code fmt.Errorf(...) | Functionality |
S1030-Use bytes.Buffer.String or bytes.Buffer.Bytes | bytes.Buffer has both a String and a Bytes method. It is almost never necessary to use string(buf.Bytes()) or []byte(buf.String()) – simply use the other method. The only exception to this are map lookups. Due to a compiler optimization, m[string(buf.Bytes())] is more efficient than m[buf.String()]. | Functionality | |
S1035-Redundant call to net/http.CanonicalHeaderKey in method call on net/http.Header | The methods on net/http.Header, namely Add, Del, Get and Set, already canonicalize the given header name. | Functionality | |
S1037-Elaborate way of sleeping | Using a select statement with a single case receiving from the result of time.After is a very elaborate way of sleeping that can much simpler be expressed with a simple call to time.Sleep. | Functionality | |
S1040-Type assertion to current type | The type assertion 'x.(SomeInterface)', when 'x' already has type 'SomeInterface', can only fail if 'x' is nil. Usually, this is left-over code from when 'x' had a different type and you can safely delete the type assertion. If you want to check that 'x' is not nil, consider being explicit and using an actual 'if x == nil' comparison instead of relying on the type assertion panicking. | Functionality | |
SA1004-Suspiciously small untyped constant in time.Sleep | The time.Sleep function takes a time.Duration as its only argument. Durations are expressed in nanoseconds. Thus, calling time.Sleep(1) will sleep for 1 nanosecond. This is a common source of bugs, as sleep functions in other languages often accept seconds or milliseconds. The time package provides constants such as time.Second to express large durations. These can be combined with arithmetic to express arbitrary durations, for example '5 * time.Second' for 5 seconds. If you truly meant to sleep for a tiny amount of time, use 'n * time.Nanosecond' to signal to Staticcheck that you did mean to sleep for some amount of nanoseconds. | Robustness | |
SA1006-Printf with dynamic first argument and no further arguments | Printf with dynamic first argument and no further arguments Using fmt.Printf with a dynamic first argument can lead to unexpected output. The first argument is a format string, where certain character combinations have special meaning. | Robustness | |
SA1008-Non-canonical key in http.Header map | Keys in http.Header maps are canonical, meaning they follow a specific combination of uppercase and lowercase letters. Methods such as http.Header.Add and http.Header.Del convert inputs into this canonical form before manipulating the map. When manipulating http.Header maps directly, as opposed to using the provided methods, care should be taken to stick to canonical form in order to avoid inconsistencies. | //non-compliant code h := http.Header{} h["etag"] = []string{"1234"} h.Add("etag", "5678") fmt.Println(h) //compliant code h := http.CanonicalHeaderKey{} h["etag"] = []string{"1234"} h.Add("etag", "5678") fmt.Println(h) | Functionality |
SA1012-A nil context.Context is being passed to a function, consider using context.TODO instead | A nil context.Context is being passed to a function, consider using context.TODO instead | Maintainability | |
SA1015-Using time.Tick in a way that will leak. Consider using time.NewTicker, and only use time.Tick in tests, commands and endless functions | Using time.Tick in a way that will leak. Consider using time.NewTicker, and only use time.Tick in tests, commands and endless functions | Robustness | |
SA1016-Trapping a signal that cannot be trapped | Not all signals can be intercepted by a process. Speficially, on UNIX-like systems, the syscall.SIGKILL and syscall.SIGSTOP signals are never passed to the process, but instead handled directly by the kernel. It is therefore pointless to try and handle these signals. | Functionality | |
SA1017-Channels used with os/signal.Notify should be buffered | The os/signal package uses non-blocking channel sends when delivering signals. If the receiving end of the channel isn't ready and the channel is either unbuffered or full, the signal will be dropped. To avoid missing signals, the channel should be buffered and of the appropriate size. For a channel used for notification of just one signal value, a buffer of size 1 is sufficient. | Maintainability, Efficiency | |
SA1019-Using a deprecated function, variable, constant or field | Using a deprecated function, variable, constant or field | Maintainability | |
SA1024-A string cutset contains duplicate characters | The strings.TrimLeft and strings.TrimRight functions take cutsets, not prefixes. A cutset is treated as a set of characters to remove from a string. For example, strings.TrimLeft("42133word", "1234")) will result in the string "word" – any characters that are 1, 2, 3 or 4 are cut from the left of the string. In order to remove one string from another, use strings.TrimPrefix instead. | Functionality | |
SA1025-It is not possible to use (*time.Timer).Reset's return value correctly | It is not possible to use (*time.Timer).Reset's return value correctly | Fuctionality, Robustness | |
SA1028-sort.Slice can only be used on slices | The first argument of sort.Slice must be a slice. | Functionality | |
SA2001-Empty critical section, did you mean to defer the unlock? | Empty critical sections of the kind mu.Lock() mu.Unlock() are very often a typo, and the following was intended instead: mu.Lock() defer mu.Unlock() Do note that sometimes empty critical sections can be useful, as a form of signaling to wait on another goroutine. Many times, there are simpler ways of achieving the same effect. When that isn't the case, the code should be amply commented to avoid confusion. Combining such comments with a //lint:ignore directive can be used to suppress this rare false positive. | Functionality | |
SA3001-Assigning to b.N in benchmarks distorts the results | The testing package dynamically sets b.N to improve the reliability of benchmarks and uses it in computations to determine the duration of a single operation. Benchmark code must not alter b.N as this would falsify results. | Robustness | |
SA4004-The loop exits unconditionally after one iteration | The loop exits unconditionally after one iteration | Functionality | |
SA4006-A value assigned to a variable is never read before being overwritten. Forgotten error check or dead code? | A value assigned to a variable is never read before being overwritten. Forgotten error check or dead code? | Conceptual Integrity | |
SA4008-The variable in the loop condition never changes, are you incrementing the wrong variable? | The variable in the loop condition never changes, are you incrementing the wrong variable? | Conceptual Integrity | |
SA4009-A function argument is overwritten before its first use | A function argument is overwritten before its first use | Conceptual Integrity | |
SA4010-The result of append will never be observed anywhere | The result of append will never be observed anywhere | Conceptual Integrity | |
SA4011-Break statement with no effect. Did you mean to break out of an outer loop? | Break statement with no effect. Did you mean to break out of an outer loop? | Conceptual Integrity | |
SA5000-Assignment to nil map | Assignment to nil map | if len(slice) < 0 { fmt.Println("unreachable code") } | Functionality, Conceptual Integrity |
SA5001-Defering Close before checking for a possible error | Defering Close before checking for a possible error | Conceptual Integrity | |
SA5004-for { select { ... with an empty default branch spins | for { select { ... with an empty default branch spins | Conceptual Integrity | |
SA5012-Passing odd-sized slice to function expecting even size | Some functions that take slices as parameters expect the slices to have an even number of elements. Often, these functions treat elements in a slice as pairs. For example, strings.NewReplacer takes pairs of old and new strings, and calling it with an odd number of elements would be an error. | Robustness | |
SA6000-Using regexp.Match or related in a loop, should use regexp.Compile | Using regexp.Match or related in a loop, should use regexp.Compile | COnceptual INtegrity | |
SA6001-Missing an optimization opportunity when indexing maps by byte slices | Map keys must be comparable, which precludes the use of byte slices. This usually leads to using string keys and converting byte slices to strings. Normally, a conversion of a byte slice to a string needs to copy the data and causes allocations. The compiler, however, recognizes m[string(b)] and uses the data of b directly, without copying it, because it knows that the data can't change during the map lookup. | //non-comliant code k := string(b) println(m[k]) println(m[k]) //compliant code println(m[string(b)]) println(m[string(b)]) | Functionality, Conceptual Integrity |
SA6003-Converting a string to a slice of runes before ranging over it | You may want to loop over the runes in a string. Instead of converting the string to a slice of runes and looping over that, you can loop over the string itself. | //non-comliant code for _, r := range s {} //compliant code for _, r := range []rune(s) {} | Functionality |
SA9002-Using a non-octal os.FileMode that looks like it was meant to be in octal. | Using a non-octal os.FileMode that looks like it was meant to be in octal. | Maintainability | |
SA9005-Trying to marshal a struct with no public fields nor custom marshaling | The encoding/json and encoding/xml packages only operate on exported fields in structs, not unexported ones. It is usually an error to try to (un)marshal structs that only consist of unexported fields. This check will not flag calls involving types that define custom marshaling behavior, e.g. via MarshalJSON methods. It will also not flag empty structs. | Conceptual Integrity, Functionality | |
SA9006-Dubious bit shifting of a fixed size integer value | Bit shifting a value past its size will always clear the value. For instance: v := int8(42) v >>= 8 will always result in 0. This check flags bit shifiting operations on fixed size integer values only. That is, int, uint and uintptr are never flagged to avoid potential false positives in somewhat exotic but valid bit twiddling tricks: // Clear any value above 32 bits if integers are more than 32 bits. func f(i int) int { v := i >> 32 v = v << 32 return i-v } | Robustness | |
ST1008-A function's error value should be its last return value | A function's error value should be its last return value. | Functionality | |
ST1016-Use consistent method receiver names | Use consistent method receiver names | Functionality | |
ST1019-Importing the same package multiple times | Go allows importing the same package multiple times, as long as different import aliases are being used. That is, the following bit of code is valid: import ( "fmt" fumpt "fmt" format "fmt" _ "fmt" ) However, this is very rarely done on purpose. Usually, it is a sign of code that got refactored, accidentally adding duplicate import statements. It is also a rarely known feature, which may contribute to confusion. | Functionaility |