7年間放置されたRuboCopをチームワークと自動化の二刀流で改善した話

こんにちは。デジカルチームでソフトウェアエンジニアをしている武井です。

デジカルチームでは、クラウド型電子カルテ「エムスリーデジカル」を開発しており、メインのAPIサーバーとしてRuby on Railsを採用しています。

digikar.m3.com

今回、長年放置されてきたRuboCopの設定の見直し、運用の改善をチームで協力して行いました。この記事では改善の過程の一部始終をご紹介できればと思います。

この記事でも触れる「リファクタリングデー」の後に打ち上げで行った焼肉を楽しむ様子

RuboCopの形骸化
チーム全員でルールの見直し
違反箇所を3種類に分類
1. autocorrectできsafeな違反は一括修正
2. autocorrectできるがunsafeな違反は自動化して段階的に改善
3. autocorrectできない違反はチームワークで解決
まとめ
We are hiring!!

RuboCopの形骸化

RuboCopはRubyのコード品質を維持するための静的解析ツール・コードフォーマッターです。多くのRubyプロジェクトで採用されている、人気の高いツールです。

github.com

しかし、コード品質の維持に役立つ一方で、実際の運用においては次のような課題に直面することが少なくありません。

RailsやRubyのバージョンアップ時に違反が発生しがちで、一時的に回避するためにルール(Cop)自体を無効化する
既存コードの違反箇所を列挙し一時的に無効化する役割の.rubocop_todo.ymlに多くの違反が記録されたまま、なかなか対応できない
開発時に改修箇所で違反が出てもさまざまな要因から「todoに追加していったん回避！」と目をつぶる

多くの企業、チーム、OSSなどの組織でこういった課題への対応に苦労しているのではないでしょうか。

私たちのプロジェクトでも同様の状態に陥りかけており、ルール違反を許容する風土や、ルール自体への不信感が生まれ始めていたため、RuboCop改善プロジェクトを始めることにしました。

チーム全員でルールの見直し

RuboCopの設定ファイルである.rubocop.ymlの現状を確認したところ、チームの開発スタイルに適さないルール設定や、十分な検討なしに設定されたルールが散見されました。古いものでは、7年前のRubyバージョンアップの際に一時的に無効化されたルールがそのまま放置されていました。

# NOTE: Rules below are temporary disabled for smooth version upgrade. Consider if we should enable or not.

Style/FrozenStringLiteralComment:
  Enabled: false
Style/BlockDelimiters:
  Enabled: false
# 同様に Enabled: false にしているルールがいくつか続く、、、

このようなメンテナンスされていないルールに従って違反箇所を修正しても意味がありません。

そこでまずは現状のルールを見直し、必要なルールを整理することから始めました。

こういった場面ではRubyやRuboCopに詳しいメンバーが代表してルールを決めることもあると思いますが、今回は一人一人の納得感醸成のためにも、チーム全員で集合知的にルール設定するアプローチをとることにしました。

デジカルチームでは定期的に「リファクタリングデー」と題した出社推奨日を設けており、この日のテーマとして無効化されたルールについて手分けして調査、議論していきました。

現在無効化されているルールをリスト化し、ルールごとに担当者を割り当てて簡単な内容の調査と無効化/有効化すべきか(閾値を設定するタイプのルールであれば適正な値)を提案してもらい、それをもとにチーム全員で議論しました。

一連の議論を通して、コードの書き方に関するメンバーそれぞれの価値観を知ることができて面白かったです。またルールを調査する中で言語特有の注意点を知る機会もあり、チームとして大きな学びになりました。

このようにチーム全員が参加してルールを整理することで、納得感があり、従う意味があると思えるルールを作り上げることができたと感じています。

違反箇所を3種類に分類

ルールを整理できたので、ルールに沿うようにコードの違反箇所を修正していきます。

rubocop --auto-gen-config コマンドを実行し、改めて違反箇所を.rubocop_todo.ymlに列挙したところルール違反は次の3種類に分類できることがわかりました。

autocorrectできsafeな違反 (rubocop -a で直せる)
autocorrectできるがunsafeな違反 (rubocop -A で直せる)
autocorrectできない違反

各違反に対して適切な修正方法を検討し、順番に対処していきました。

1. autocorrectできsafeな違反は一括修正

はじめに1つ目のautocorrectできsafeな違反をコマンドで一括修正しました。多くはインデントの修正や不要な空白の削除など、動作に影響のないものでした。

修正対象が非常に多く数百ファイルに及んだため、ざっと流し見する程度ですがチームで手分けしてレビューを実施しました。

2. autocorrectできるがunsafeな違反は自動化して段階的に改善

2つ目のautocorrectできるがunsafeな違反は、rubocop -Aのコマンドで修正できるものの常に正しく直せるわけではなく動作確認やレビューが必要になります。

これらを一括で修正すると、レビューが大変になるだけでなく、不具合発生時の原因特定も難しくなります。そこで修正を自動化し段階的に改善していく仕組みを作ることにしました。

具体的には一定数のautocorrectを適用するMerge Request(GitHub における Pull Request)を作成するBotを作り、定期的に稼働させることにしました。

このautocorrectを定期的に実行するアイデアはSTORESさんの取り組みを参考にさせていただきました。 product.st.inc

Botの具体的な処理としては、

次のようなRubyのコード*1で修正対象のルールを.rubocop_todo.ymlから一時的に削除し
rubocop -A <対象のファイルパス> でautocorrect
rubocop --auto-gen-config で.rubocop_todo.ymlを再作成
gitコマンドやgitlabのAPIでMerge Request作成

といった流れになっています。

MAX_FILES = <一回で修正するファイル数>

todos = YAML.load_file('./.rubocop_todo.yml')
config = RuboCop::ConfigLoader.default_configuration

cops = todos.map do |cop, files|
  ["RuboCop::Cop::#{cop.gsub('/', '::')}".constantize.new(config), files['Exclude']]
end

res_name = []
res_path = []

cops.each do |cop, files|
  # 自動修正可能なCopのみ対象
  next unless cop.correctable?

  res_name << cop.name
  files.each do |path|
    res_path << path
    break if res_path.size >= MAX_FILES
  end

  break if res_path.size >= MAX_FILES
end

# 修正対象のcopを.rubocop_todo.ymlから一時的に削除
rejected = todos.reject { |cop| res_name.include?(cop) }
File.write('./.rubocop_todo.yml', rejected.to_yaml)