every Tech Blog

株式会社エブリーのTech Blogです。

トップ > golangci-lint を整備したらレビュー時間が短くなった話

golangci-lint を整備したらレビュー時間が短くなった話

はじめに

こんにちは、@きょーです！普段はDELISH KITCHEN開発部のバックエンド中心で業務をしています。

チームに join した内定者のサポートをしているのですが成長ぶりが凄まじく驚くばかりです。その成長を近くで眺めるのが最近の趣味です。

この記事の対象者

  • レビューで不要な空行やマジックナンバーなどの実装ではなくコードスタイルに関する指摘をしたこと・受けたことがある人
  • golangci-lintがリポジトリに入っているが、既存の設定状態で使っているだけで見直しできていない人

導入

僕が関わっているプロジェクトでは、golangci-lintを使用していますが、ほぼプロジェクトに導入された時の設定のままで、あまり見直しができていなく、既存の設定値は以下のようになっていました。

run:
  timeout: 10m
  issues-exit-code: 0

linters:
  disable-all: true
  fast: false
  enable:
    - bodyclose
    - errcheck
    - goconst
    - gocritic
    - gofmt
    - goimports
    - gosec
    - gosimple
    - govet
    - ineffassign
    - misspell
    - nakedret
    - prealloc
    - staticcheck
    - typecheck
    - unconvert
    - unparam
    - unused

issues:
  exclude-use-default: false
  exclude-rules:
    - path: _test\.go
      linters:
        - errcheck
        - scopelint
        - unparam
        - staticcheck
    - linters:
        - gosec
      text: "G401:"
    - linters:
        - gosec
      text: "G501:"
  exclude:
    - Error return value of `.*.Close` is not checked

上記の内容を見てみるとホワイトリスト形式で書かれていることがわかります。このコードは 5 年前の当時すでに大きくなっていたリポジトリにgolangci-lintを追加する形で導入されていて、開発速度を落とさず必要最低限のlinterを導入したいという思いからブラックリスト形式ではなくホワイトリスト形式を採用したのだと思います。

レビューをする中で以前の必要最低限で入れていたlinterでは検出できないコードスタイルについて指摘をしたり受けたりしたことがありました。例えば ↓ のような点です。

  • 関数定義やif、for 文の最初や最後に不要な空行がある
  • マジックナンバーを使っている
  • 構造体のフィールドとフィールドにつけるタグが一致していない

新しいメンバーが加わるたびに同様の指摘をする可能性があり、別のチームでも同じようなことが起きるかもしれません。そのたびに同じようなコミュニケーションが発生するのは、ちりつもでかなりのコストになりますし、組織として知見を蓄積できていないとも言えます。そのためこの状況を改善したいと考えるようになりました。

そもそも、こういった指摘は本来linterが行うべきことではないでしょうか？そこで、実際にした・受けたレビューをもとに、golangci-lintにどのようなlinterを追加し改善していったのかについて知見を共有したいと思います。

golangci-lintとは？

golangci-lint.run

様々なlinterを一元管理・実行することが出来るツールです。設定したコードスタイルに沿っていないコードを検出してくれるので、統一感のあるコードを書けるようになります。特徴として ↓ のようなものがあります。

  • 早い

    • 並列にリンターを実行
    • Go ビルドキャッシュを再利用
    • 分析結果をキャッシュ

  • 組み込めるlinterが豊富

    • 100 以上
    • ダウンロード不要

  • 主要な IDE と統合

    • VSCode
    • GoLand
    • Vim
    • GitHub Actions

  • etc...

主要な設定は ↓ の通りです。詳細は公式に書いてあるのでそちらをご覧ください。

# 適用したいlinterの設定
linters:
  option: value
# linterごとの設定
linters-settings:
  option: value
# linterの報告に関する設定
issues:
  option: value
# 出力に関する設定
output:
  option: value
# 実行に関する設定
run:
  option: value
# 報告の重要度に関する設定
severity:
  option: value

弊社ではgithub actionsでgolangci-lintを入れていてpushしたら走るように設定しています。

実際のレビューに基づきgolangci-lintに追加したlinterたち

whitespace

説明

関数やif、forの最初や最後に不要な空行がないかをチェックするlinterです。

設定できる値は下記のようになっていて、ifの条件が複数行になった場合に最初の行を空行で始めるかどうかの設定等もできるようです。

linters-settings:
  whitespace:
    # Default: false
    multi-if: true
    # Default: false
    multi-func: true

実際のレビュー

関数の下の空行

行末の変な空行

導入後

下記のコードには、関数定義の後と関数最後の行に不要な空行があります。これをlinterが検出してくれます。

func sample() { // unnecessary leading newline (whitespace)

    fmt.Println("Hello, world!")

} // unnecessary trailing newline (whitespace)

linterが検出してくれるので下記のようにすぐ修正できます

func sample() {
    fmt.Println("Hello, world!")
}

mnd

説明

マジックナンバーを検出するlinterです。

設定できる値は下記のようになっていて、チェックする項目（引数、代入、switch・if 文や returnの値）を設定できたり、検出を無視する数字やファイル、関数を指定できるようです。

linters-settings:
  mnd:
    # Default: ["argument", "case", "condition", "operation", "return", "assign"]
    checks:
      - argument
      - case
      - condition
      - operation
      - return
      - assign
    # Default: []
    ignored-numbers:
      - "0666"
      - "0755"
      - "42"
    # Default: []
    ignored-files:
      - 'magic1_.+\.go$'
    # Default: []
    ignored-functions:
      - '^math\.'
      - '^http\.StatusText$'

実際のレビュー

マジックナンバー

導入後

下記のように引数に代入するときや数字同士を比較する時に値をそのまま使っていないかlinterが検出してくれます。

func sample() {
    hoge := someFunc(60) // Magic number: 360, in <argument> detected (mnd)
}

linterが検出してくれるので下記のようにすぐ修正できます

func sample() {
    value := 60
    hoge := someFunc(value)
}

tagliatelle

説明

構造体のフィールド名とタグをチェックするlinterです。

設定できる値は下記のようになっていて、フィールド名とタグの名前を同じものとなるように設定したり、camelやsnakeケースなどタグのスタイルを設定できるようです。

linters-settings:
  tagliatelle:
    case:
      # Default: false
      use-field-name: true
      # Default: {}
      rules:
        # Any struct tag type can be used.
        # Support string case: `camel`, `pascal`, `kebab`, `snake`, `upperSnake`, `goCamel`, `goPascal`, `goKebab`, `goSnake`, `upper`, `lower`, `header`
        json: camel
        yaml: camel
        xml: camel
        toml: camel
        bson: camel
        avro: snake
        mapstructure: kebab
        env: upperSnake
        envconfig: upperSnake

実際のレビュー

構造体のフィールド

導入後

下記の構造体ではIDというフィールド名のjsonタグがhogeになっています。期待している値はidと言うタグなので、これをlinterが検出してくれます。

type Sample struct {
    ID string `json:"hoge"` // json(snake): got 'hoge' want 'id' (tagliatelle)
}

linterが検出してくれるので下記のようにすぐ修正できます

type Sample struct {
    ID string `json:"id"`
}

まとめ

この記事では、golangci-lintを活用してコードレビューの効率を向上させる方法を紹介しました。golangci-lintの整備により、不要な空行やマジックナンバー、構造体のタグの不一致といった問題を自動で検出できるようになりました。これにより、レビューの際には本質的なロジックに集中できるようになり、開発プロセス全体の質を向上させることができました。

今後は、golangci-lintの設定をさらに最適化し、他のプロジェクトにも展開することで、組織全体の開発効率をさらに高めていきたいと考えています。

同様の課題を抱える方は、ぜひgolangci-lintの導入・整備を検討してみてください。もし初期段階のプロジェクトであればブラックリスト形式で導入を検討するのもいいかもしれません。

参考資料