• home
  • CI書いててgo vetで怒られたところ

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とかも新規プロジェクトではおすすめですが、現行のプロジェクトでは流石にうるさいかな…ということで、全面的には採用していません。
ただ、特に色々なアプリケーションから触れる基幹ライブラリに関しては、これからも入念に色々やっていこうかなあ、と思っています。