動機
- コーディング規約等がない無秩序に書かれたRailsアプリケーションをなんとかしたい
- 現状のコードをあるコーディング規約にしたがって書くように統一したい
- これ以上コーディング規約外のコードを増やしたくない
前提
- 既存のRailsアプリケーションがあるがrubocopは導入されていない
ありがちなこと
- rubocop入れたら尋常じゃない量の違反が見つかってどうしたらいいかわからない
bundle exec rubocop -D -R
# ...
629 files inspected, 8475 offenses detected
- --auto-correctの差分多すぎてちゃんとうごくのか検証,レビューできない
git diff --shortstat
381 files changed, 4285 insertions(+), 4388 deletions(-)
導入手順
前述のように差分多すぎたり,違反出すぎて一括で直すことが困難なので段階的になおしていく
1. rubocopのインストール
何はともあれrubocop入れないとことが始まらないので導入
Gemfileにrubocopを追加
gem 'rubocop'
インストール
bundle install
2. とりあえずrubocopを実行してみる
bundle exec rubocop -R -D
これまでrubocopが使われていなかったので,前述のように大量の警告がでるかと思います.
3. Severityの高い警告を修正する
rubocopの警告はR/C/W/E/F(Refactor, Convention, Warning, Error, Fatal)のレベルにわけられています.
今回の自分のプロジェクトではW以上が10数個程度でたので,まずそれに関してのみ愚直に修正を行いました.
( エラーの数や度合いによってレベルの基準点は任意に設定できますが,ここでは例としてWを基準とし,それ以上のレベルのルール違反はすでに修正済みという前提となります)
4. 既存のrubocopの警告を(一旦)無視する
3.でクリティカルなものに関しては修正してたものの,依然として警告が大量にあるので警告を一旦無視するため以下のコマンドを実行
bundle exec rubocop -R --auto-gen-config --exclude-limit 999999
これを実行することで,既存の警告を全て除外する設定ファイルが生成されるのでこれを .rubocop_todo.yml
として保存する.
(--execute-limit
は除外する個数制限をしているものですが,ひとまずは実質無制限で無視しています.)
.rubocop_todo.ymlは「現状違反しているが一旦は例外的に除く違反リスト」なので,これはすなわち「将来直すべき違反」ということになり,このファイルを消してもrubocopでエラーがでないことが最終ゴールとなる.
bundle exec rubocop -D -R -c .rubocop_todo.yml
とすると, .rubocop_todo.ymlにしたがってルール違反を無視してくれるのでこのタイミングではかならず全てのルールがvalidになるはずです.
5. 特定のSeverity以上のエラーはCI上でエラーにする
3.でW以上の警告を修正しましたが,今後の開発であらたにWのものが増えてしまっては意味がないので,CI上でW以上だった場合を検知するためCI上で以下のスクリプトを実行する.
# fail-levelは 3.で直したレベルを設定しておく
bundle exec rubocop -R -D --fail-level W
--fail-level
を指定しておくと,そのレベル以上の違反があるとrubocopコマンドがエラー(ステータスコードが0以外)となるので,その場合にCI上でテストを失敗扱いにするなどして検知ができる.
これを利用することで,例えば,githubのPull Requestが上がってきたコードをチェックし,エラーであれば(=failt-levelがしきい値以上の違反があれば)テスト失敗扱いにしてマージさせないといったことができる.
ひとまずこでW以上のエラーに関してはチェックされ,これ以上増えることはないようになった
6. .rubocop_todo.yml 内の違反を1つずつ駆逐していく
前述のとおり,rubocop_todo.ymlはものすごい量の違反がでていると思うので一度に全部治すのは困難.
なので1つのCOP(ルール)ごとに解決していく.
下準備のため最初の1回のみ下記を実行していきます.
空の .rubocop.yml ファイルを作成
touch .rubocop.yml
下記を .rubocop_todo.yml の先頭に追加
inherit_from: .rubocop.yml
これで .rubocop_todo.ymlと.rubocop.ymlに記述されたルールが両方適用されるようになる.
なぜ,わざわざ2つ用意したかというと.rubocop_todo.yml は「将来直すべきルール違反」だったものに対して,.rubocop.ymlは「新しく定義したコード規約」を書くためです.
上記の用途を念頭においた上で,下記のようなサイクルを繰り返してルールを作って行きます.
ルールを作っていくサイクル
下記フローを繰り返しながら1つずつ徐々にルールを作っていく
- .rubocop_todo.ymlにある1つのCOP(ここでは例としてStyle/SpaceAroundOperators)について,Excludeしている部分を削除する
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -1092,22 +1092,6 @@ Style/SpaceAfterComma:
Style/SpaceAroundEqualsInParameterDefault:
Enabled: false
-# Offense count: 50
-# Cop supports --auto-correct.
-# Configuration parameters: MultiSpaceAllowedForOperators.
-Style/SpaceAroundOperators:
- Exclude:
- # 無視しているファイル名一覧
- 該当COPのSeverityをエラーになるレベル(今回の場合W)まで上げたものをrubocop.ymlに記述
+Style/SpaceAroundOperators:
+ Severity: warning
+ AllowForAlignment: true
上記 AllowForAlignment にあるようにプロジェクト内のルールを変更したければここで変更する
- 下記を実行すると先ほど削除した部分のルール違反がでるようになる
bundle exec rubocop -D -R -c .rubocop_todo.yml --only Style/SpaceAroundOperators
- (auto-correct可能なCOPであれば)下記を実行.不可能であれば手動でルール違反を直す
bundle exec rubocop -D -R -c .rubocop_todo.yml --only Style/SpaceAroundOperators --auto-correct
- 差分を確認しPull Requestを出す -> レビュー -> マージ
サイクルがひとしきり回った後
- CIのfail-levelを下に落とし,さらにルールを厳しくしていく.
- CIの全体のfail-levelを厳しくした際は rubocop_todo.yml にある Severityで不用になったものは削除してもよい
上記手法のメリット
- 「将来なおしていくもの」と「あたらしく作ったルール」をわけて考えることができる
- 1COPずつ徐々に「将来なおしていくもの」から「新しく作ったルール」に移行していくことができる
- 変更は少しずつ段階的にコード規約統一が図れる
- 一気にやるのは不安なレビューやデプロイを細かくわけられる
- ルール違反を直している途中にフィーチャーの開発などで新しい違反が増えていくことが防げる
- 通常の開発と並行して進められる
- 1サイクル単位で改善していくので時間的に間が空いても大丈夫