CI書いててgo vetで怒られたところ
go vetとは、goのコードを静的解析し、怪しい記法を指摘する標準組み込みのツールです。要するにlinterです。
既存のGoプロジェクトもそれなりのサイズになって、新しく入るメンバーも増えてきました。
そこで、コードレビューや各自のオペレーションではうっかり漏れることもある記法チェックは自動で回しておくか…という向きになり、もともとCIで回していた go test
に加え、go generate ./...
や go vet ./...
等の処理もあちこちの自動テストに貼っていっています。
(go test
時に go vet
も実行されるイメージだったのですが、勘違いだったようで。)
明示的に実行すると見落としていた良くないコードがポロっと出てきたので、やはりgo vet
は必須ツールとして開発フローに入れたほうがいいな、ということで、感想までに記事に残しておきます。
どういうものを検出してくれるのか
実際に発生していたもので紹介してみようと思います。
うっかり未到達のコード
for {
select {
case event := <-w.Event:
callback(event)
case err := <-w.Error:
log.Error(err)
case <-w.Closed:
return
log.Info(`process end`) //これ
}
}
構造体タグの記法のミス
type AssetImage struct {
Type string `json:"type" validate:"required"`
Resource Resource `json:"resource",validate:"required"` //こういうの。謎の,が入ってしまっている
SelectedFrame *int `json:"selected_frame,omitempty"`
}
type HandlerConfig struct {
Log struct {
Level string `yaml:"Level"`
} `yaml:Log` //ここ。""がない
PreparedDeliveryConsumerConfig SqsPreparedDeliveryConsumerConfig `yaml:"PreparedDeliveryConsumerConfig"`
}
これは割と危険なので検出してくれると便利ですね。
fmt.Sprintf等の不正な使用
a := fmt.Sprintf("%s", 1) // %d でとってない
fmt.Errorf(a) //破棄された値の使用
こういうやつです。
キー名を指定しない構造体の初期化
こういう報告が出ている場合
dao/330_review_history_test.go:224: database/sql.NullString composite literal uses unkeyed fields
該当箇所では以下のような構造体宣言が行われています。
SampleUser.Mail = sql.NullString{"update reject mail", true}
明示的にkey名も指定することが推奨されています。
関係ないけど mysql.NullXXX系をうまくラップした gopkg.in/guregu/null.v3
というやつがあって、ポインタとの相互変換や便利なファクトリ、MarshalJSONなどもうまいこと実装していておすすめです。
sync.Mutex
go vet ./... [fix/refactor-for-vet]
# bitbucket.org/xx/xx/redis
redis/handler.go:42: assignment copies lock value to redisMap[conf.Name]: bitbucket.org/xx/xx_core/vendor/github.com/garyburd/redigo/redis.Pool contains sync.Mutex
redis周りのラッパーライブラリを作ってて発生しました。ここでは sync.Mutex
を持っているredis.Poolインスタンスをmapに持つことでエラーが起こっています。
mutexを使ったものはポインタ渡しで引き回しましょう。
precommitやCIに追加しておこう
弊社一部プロジェクトでは、AWS CodeBuildを使ってプルリクやmasterの自動テストを回しています。
BitbucketでPull Requestが発行されたり、masterの更新があると勝手に処理を始めてくれます。
この手のやつ。
version: 0.2
phases:
install:
commands:
- echo start
pre_build:
commands:
- echo prebuild
build:
commands:
# go
- dep ensure --vendor-only=true
- go test ./...
- go vet ./...
- go generate ./...
- go fmt ./...
- git diff --exit-code --quiet
- if [ $? -eq 0 ]; then echo "ok"; else echo "git diff error"; exit 1; fi
post_build:
commands:
- echo done
最近はBitbucketも webhookやらApi gatewayやら Lambdaやら用意しなくても、ビルドバッジの表示やSlackへの通知がコンソールから簡単にできて楽ですね。
今回はここに go vet
を貼ってまわりました。
各種サードパーティのLinterを同梱した gometalinter
とかも新規プロジェクトではおすすめですが、現行のプロジェクトでは流石にうるさいかな…ということで、全面的には採用していません。
ただ、特に色々なアプリケーションから触れる基幹ライブラリに関しては、これからも入念に色々やっていこうかなあ、と思っています。