あぼぼーぼ・ぼーぼぼ

のんびり生きたい

Goのリンターstaticcheckのルールを全部読んだからいくつか紹介

Goのリンターの1つであるstaticcheckのルールを全部読みました。

staticcheck.io

全部書くとただのコピーになってしまうので、その中からかいつまんでいくつか紹介します。

SA1 – Various misuses of the standard library

SA1004 - Suspiciously small untyped constant in time.Sleep

time.Sleep関数に渡されたtime.Durationはナノ秒単位なので、あまりにも小さい値の場合はバグの原因として注意してくれるようです。他の言語だとこういうAPIの場合ミリ秒とかだったりしますからね。意図的に小さい値を使いたい場合はtime.NanosecondeをかければOK。

time.Sleep(1) // NG
// main.go:19:13: sleeping for 1 nanoseconds is probably a bug; be explicit if it isn't (SA1004)

time.Sleep(1 * time.Nanosecond) // OK

SA1005 - Invalid first argument to exec.Command

exec.Command関数の第一引数がシェルコマンドだったらその次の引数がプログラム名やパスじゃないと注意してくれる。ほぇ〜。ただ第二引数以降に入ってる場合は注意なし。

exec.Command("ls arg1") // NG
// main.go:8:15: first argument to exec.Command looks like a shell command, but a program name or path are expected (SA1005)

exec.Command("ls ./") // OK

SA1006 - Printf with dynamic first argument and no further arguments

ユーザー入力などの動的な文字列をフォーマットとしてfmt.Printf関数に渡すのを注意してくれる。この例だと別のルール(SA5009)にも引っ掛かりますね。

s := "Interest rate: 5%"
fmt.Printf(s) // NG
// main.go:9:2: printf-style function with dynamic format string and no further arguments should use print-style function instead (SA1006)
// main.go:9:13: couldn't parse format string (SA5009)

fmt.Printf("%s", s) // OK

SA1008 - Non-canonical key in http.Header map

http.Headerのキーは大文字始まりで正規化されていて、http.Header.Addのような用意されている関数経由ならそうなるが、mapに直接追加するなどするとそうならないので注意してくれる...って書いてあるけど手元で試してみたところ引っかからなかった。

追加時ではなく参照するコードに対してチェックしているようだった。

h := http.Header{}
h["etag"] = []string{"1234"} // staticcheckでは怒られない
h.Add("etag", "5678")
// map[Etag:[5678] etag:[1234]]

_ = h["etag"] // NG
// main.go:12:6: keys in http.Header are canonicalized, "etag" is not canonical; fix the constant or use http.CanonicalHeaderKey (SA1008)

_ = http.CanonicalHeaderKey("etag") // OK -> "Etag"

SA1019 - Using a deprecated function, variable, constant or field

deprecatedになったAPIの利用を注意してくれる。すごくありがたいけどどうやって検知してるんだろう?と思ってtestdataコードを読むと各バージョンごとにルールを作ってるみたいでした。

https://github.com/dominikh/go-tools/blob/master/staticcheck/testdata/src/CheckDeprecated_go119/CheckDeprecated.go

SA1024 - A string cutset contains duplicate characters

strings.TrimLeftとstrings.TrimPrefixの違いが分かる。strings.TrimLeftやTrimRightは第二引数に取り除きたい文字のセットを渡すので、重複がいらない。

fmt.Println(strings.TrimLeft("42133word", "12334")) // NG
// word
// main.go:10:44: cutset contains duplicate characters (SA1024)

fmt.Println(strings.TrimLeft("42133word", "1234")) // OK
// word

SA1028 - sort.Slice can only be used on slices

sort.Slice関数の第一引数xはanyなんですが、GoDocにもあるとおりスライスじゃないとpanicしてしまう。こういうのを注意してくれるのは助かりますね。

https://pkg.go.dev/sort#Slice

SA2 – Concurrency issues

SA2001 - Empty critical section, did you mean to defer the unlock?

Lock直後に書かれたUnlockにdeferが付いてなければ注意してくれる。ただそれが有用な場合もあるとのことで、追々理解したい。

SA4 – Code that isn't really doing anything

_人人人人人人人人人人人人_
> 何もしていないコード <
 ̄Y^Y^Y^Y^Y^Y^Y^Y^Y^Y^ ̄

SA4系のルールは全てありがたいと思いました。

SA4023 - Impossible comparison of interface value with untyped nil

ほぇぇとなったやつ。インタフェース変数にnil値を格納するとインタフェース変数はnilではない。ここもうちょっとちゃんと理解したい。

type MyInterface interface{}

func nilOrNotNil() any {
    var p *MyInterface = nil
    return p
}

func main() {
    if nilOrNotNil() != nil {
        fmt.Println("not nil") // reached
    }
}
// main.go:13:5: this comparison is always true (SA4023)

SA5 – Correctness issues

SA5003 - Defers in infinite loops will never execute

deferはその関数がスコープなので、無限ループの中では絶対に実行されない。

SA5011 - Possible nil pointer dereference

ポインタがデリファレンスされる際にnilの可能性があるコードを注意してくれる。

SA6 – Performance issues

SA6005 - Inefficient string comparison with strings.ToLower or strings.ToUpper

strings.EqualFold関数は文字の大文字小文字を区別せずUnicode標準のCase Foldingという操作で比較を行うためstrings.ToLowerやToUpperを行ってから比較するより速い。実際に以下のようなコードで測ってみると速度的にメモリ的にも優れていますね。

var s1 = "HoGe"
var s2 = "hOgE"

func Benchmark_ToLower(b *testing.B) {
    for i := 0; i < b.N; i++ {
        strings.ToLower(s1)
        strings.ToLower(s2)
    }
}

func Benchmark_CaseFolding(b *testing.B) {
    for i := 0; i < b.N; i++ {
        strings.EqualFold(s1, s2)
    }
}
% go test -bench . -benchmem
goos: darwin
goarch: arm64
pkg: sample
Benchmark_ToLower-8             31507820                37.83 ns/op            8 B/op          2 allocs/op
Benchmark_CaseFolding-8         124359349                9.807 ns/op           0 B/op          0 allocs/op
PASS
ok      sample  4.479s

SA9 – Dubious code constructs that have a high probability of being wrong

「間違っている可能性のある」コードを教えてくれる。ただし他のルールに比べてこれは偽陽性の可能性があるとのこと。

SA9005 - Trying to marshal a struct with no public fields nor custom marshaling

非公開のフィールドに対してencoding/jsonなどによるmarshal、unmarshalは効かないのでそれを検知してくれる。これはgo vetでも同様のチェックを行ってくれますよね。

S1 – Code simplifications

S1012 - Replace time.Now().Sub(x) with time.Since(x)

time.Now().Sub(x)よりもtime.Since(x)。このルールに限らず標準パッケージの使い方によってよりシンプルに書ける方法が見つかるので読んでいて楽しい。

S1025 - Don’t use fmt.Sprintf("%s", x) unnecessarily

fmt.Sprintfを使う必要がない場合を検知してくれる。これ結構嬉しいかも、その変数がStringerインタフェースを実装してるのかどうかを毎回完璧に把握するの大変だと思うし。

ST1 – Stylistic issues

ここからデフォルトでは無効化されているルールが登場しますが、見た感じ全て有効化して問題ない気がしました。

ST1000 - Incorrect or missing package comment (non-default)

デフォルトでは無効化されているルール。パッケージコメントをCodeReviewCommentsに記載されているルールで縛るかどうか。

github.com

ST1003 - Poorly chosen identifier (non-default)

デフォルトでは無効化されているルール。変数名やパッケージ名を、Effective GoやCodeReviewCommentsに記載されているルールで縛るかどうか。

これは有効化してもいいんじゃないですかね、mixed-capsとかinitialismsとかが他のリンターで検知できるなら別ですけど。基本この辺りの細かい書き方はEffective GoやCodeReviewCommentsに従って書く人による違いをできるだけなく無くした方が「読みやすい」コードになると思うです。

QF1 – Quickfixes

自動で直されるやつ。

QF1012 - Use fmt.Fprintf(x, ...) instead of x.Write(fmt.Sprintf(...))

こういうのは知らないと気づかないだろうしリンターにチェックしてもらえるなら学習になると思うです。


というわけで全ルールを読んで、その中からかいつまんで紹介しました。知らなかった標準パッケージのAPIを知れたり、Goの仕様を知れたり、Goに限らず「こう書いた方がシンプルだよね」って書き方を再確認したりできました。Go初学者は目を通してみると良いんじゃないでしょうか 👍