こんにちは osyoyu です。RuboCop は好きですか？
B/43の開発の現場でもRuboCopは活用しており、ご多分に漏れず .rubocop.yml がすくすくと育てられていました。コードの一貫性はよく保たれていた…… のですが、 # rubocop:disable がそれなりの頻度で出現したことからも、適用されているルールが最良のものかは少しばかり疑問があった、という具合です。
そこで、今回は “unconfigurable” をウリにしているRuboCopルールセットであるStandard (standardrb/standard) を採用し、全コードをこのルールに適合するよう修正しました。Standardは少々 “強い” 名前ですが、コミュニティの中でコレ一強・一択というものでもなく、あくまでひとつのオプションという立ち位置です。
しかしそれなりに大きなコードベースではこの修正も当然大きく（+25,252 -20,230 行）、一筋縄ではゆかないものでしたので、この記事では移行にあたっての工夫を紹介します。RuboCopを導入していないところから、rubocop-rails-omakase等を導入する場合も参考になるかもしれません。
コードスタイルについて、チームの合意を得る
RuboCopのルールを変えるならば、なにはともあれチームの合意を取るべきでしょう。ルールの変更について Issueを立てて気持ちを述べてみたところ、”良い” と感じるスタイルは想像以上に異なることが分かりました。
議論のなかで、既存のルールセット（Standard, cookpad/styleguide, thoughtbot/guides）に乗っかる案を示してみたところ、思いのほか良い反応を得られました。同僚の三谷さんの「綺麗な形が人それぞれなので、好みは持ち出さずルールと言われたものに従う方が感情の揺れがなくなって良い」という意見には納得ですね。みなbikeshedは避けたい。
ということで、あとはどのルールセットに乗っかるか、ですが、最終的には先述のStandardに落ち着きました。
理由をご説明する前に、イメージしやすいよう、差分をひとつ紹介します。B/43を支えるcore-apiから少しコードをもってきて、適当に変数名や値を変えたものです。登場する単語からなんとなくクレジットカードシステムの風を感じるかもしれませんが、本物とはさほど関係ないので読み解かなくても大丈夫です。気になる方はぜひご入社ください。
class BalanceTransaction has_one :authorization_request_history, - through: :authorization_request_history_balance_transaction_mapping + through: :authorization_request_history_balance_transaction_mapping - validates :amount, numericality: { only_integer: true } - validates :currency, inclusion: { in: BalanceTransaction.currencies.keys } - validates :source_type, inclusion: { in: BalanceTransaction.source_types.keys } + validates :amount, numericality: { only_integer: true } + validates :currency, inclusion: { in: BalanceTransaction.currencies.keys } + validates :source_type, inclusion: { in: [ + "authorization_hold", + "authorization_release", + "authorization_revert", + "deposit", + "refund", + "withdrawal" + ] } def capture! - klass = case clearing.transaction_code.to_sym - when :presentment - Presentment - when :presentment_reversal, :return - Reverse - when :return_reversal - ReverseCancel - else - raise ArgumentError - end + klass = + case clearing.transaction_code.to_sym + when :presentment + Presentment + when :presentment_reversal, :return + Reverse + when :return_reversal + ReverseCancel + else + raise ArgumentError + end end end
このdiffを見たとき、それぞれ思うところがあったのではないでしょうか。 case (や if) の桁数が減って読みやすくなった、いや validates の第2引数が不揃いになって読みづらくなった、そもそもRuboCopなくてもいいでしょ……。などなど。かくいう私も末尾カンマ愛好家なので、つけさせてほしいなと思っています（Standardでは許容されません）。
Standardの “良さ” はまさにここにあります。Standardではカスタマイズ不能なルールを天下りに与えられます。個人の感情に左右されず、bikeshedな議論を許さない、これがStandardの大きな特長と言えるでしょう。その上で…… 私の主観ではありますが、コミュニティで主流なコードスタイルから大きく離れていないように感じるのも +1 です。
また、autocorrect可能なルールのみが定義されているのもすぐれたポイントです。この方針がエディタのformat on save機能と非常に相性がよく、ruby-lspユーザーである私はファイルを保存するだけでRuboCopに怒られることがまったくなくなりました。
Standard導入 as a RuboCopのいちルール
さて、ゴールが決まったところで、実際に移行作業をはじめてゆきます。
Standardはカスタマイズ不能なのが良さと語ったばかりですが、理想ばかりでは食えないのが人間というもの。
通常、StandardはRuboCopを完全に置き換える形で導入します。コマンドも rubocop の代わりに standard が提供され、設定ファイルも .standard.yml になります。RuboCopを完全に隠蔽することで、カスタマイズの禁止を実現しているのです。
が、今回はスムーズな移行が優先事項でした。そのため、カスタマイズを可能にするべく standard コマンドを使うのではなく、あえてRuboCopのいちルールとしてStandardのルールセットを読み込む方式を採りました。Standard的には推奨されないやり方ですので、プロジェクトの立ち上げ当初であれば素直に standard のツールセットを使うのが良いでしょう。
また、この方式を採用することで、社内の開発者のRuboCopを使ったワークフローを壊さずに済みました。「困難は分割せよ」ではないですが、スタイルの変更とツールセットの変更を分けるのは良い戦略でした。
- gem 'rubocop' + gem 'standard' # 依存で rubocop を連れてくる
require: - standard - standard-custom # - standard-performance # 移行当初は使わない inherit_gem: standard: config/base.yml standard-custom: config/base.yml # standard-performance: config/base.yml # 移行当初は使わない
この状態で rubocop を実行すると、大量のoffenseが検出されるはずです。が、autocorrectするのはグッとこらえて、まずは地盤を整えていきます。
既存のrubocop:disableコメントの除去
それなりに開発を続けてきたコードベースであれば、rubocop:disable コメントもそれなりにあるものと思います。まずはこれらを整理するのが大事です。Standardのルールセットでそもそも有効になっていないcopのdisableコメントは単なるノイズですし、対になる rubocop:enable コメントで無効であるべきcopが意図せず有効化されてしまい、offenseを引き起こすこともあります。
B/43のコードベースの rubocop:disable コメントを観察したところ、Metrics 配下のルールが無効化されているケースが特に多いことが分かりました。 Metrics/MethodLength などのデフォルトの閾値が少し厳しく、偽陽性と判断されて無効化されるケースが多かったようです。
これはsedでザクっと撤去してしまいました。
# 範囲にかかる disable/enable の除去 rg --files-with-matches '^\s*# rubocop:(disable|enable) Metrics' | xargs -- sed -i '' -E '/^ *# rubocop:(disable|enable) Metrics/d' # 1行にかかる disable の除去 rg --files-with-matches '\s+# rubocop:disable Metrics' | xargs -- sed -i '' -E 's/( +)# rubocop:disable Metrics\/.*$//'
コードスタイルの変更にあたってはコードベースの観察が何よりも大事です。
いざ --autocorrect & スクリプト化
地盤が整ったところで、いよいよ rubocop --autocorrect の時間です。
--autocorrect も一発でいけるとは限りません。元のsyntaxによっては無限ループに陥ってしまい完了しないこともあるので、コードベース全体を一発でautocorrectせず、ディレクトリ単位などで作業を進めつつgit commitするのがよいでしょう。
実際に作業していると、autocorrect一発では理想の状態にはたどり着かず、本当にいろいろな理由でやり直しを迫られます。その最大の理由が、コードスタイルの変更作業中にも他の変更が次々と main ブランチにmergeされていくことです。autocorrectをかけられる段階まで来ていたら、実行すべきコマンドはある程度見えているはずなので、git commitまで含めてスクリプトにしてしまうのがおすすめです。コードレビューも変更手順（スクリプト）に対して行うほうが、巨大になる実差分よりもラク・有意義というものです。
autocorrectを超えたスタイル修正
RuboCop (Standard) のautocorrectは強力ですが、ときどき微妙な修正結果に出会うことがあります。
# Before method_call(:long_long_long_long_argument_1, :long_long_long_long_argument_2) # After 1: rubocop -a method_call(:long_long_long_long_argument_1, :long_long_long_long_argument_2) # After 2: 手動修正後 method_call( :long_long_long_long_argument_1, :long_long_long_long_argument_2 )
After 1もAfter 2もRuboCopに許容されるスタイルですが、After 2のほうが視線移動が少なく、読みやすいと判断しました（bikeshedポイントではありますね）。量が非常に多かったため、awkを用いて修正しました。
rg --files-with-matches '^[^#]+\([^)#]+,$' | xargs -- sed -i '' -E 's/\(([^)#]+)$/(\n\1/'
ところでawkを用いた修正は当然safeではなく、意図しない破壊が起きていないかを確認すべきです。Rubyでは文を ; で明示的に区切らない都合、改行の挿入は特にパース結果の変化を引き起こす可能性も高いです。
今回はPrismを用いてawkの実行前後でASTを比較し、問題のない変換だけをしていることを確認する方法がとれます。Prism 0.27.0以上ではASTのNodeに #=== メソッドが定義されており、これを使うことでlocationの変化を無視して、あくまでsyntacticに等価なプログラムであるかを検証できます。
require 'prism' # awk 適用前のファイルをパース before = Prism.parse_file("before/app/controllers/application_controller.rb") # awk 適用後のファイルをパース after = Prism.parse_file("after/app/controllers/application_controller.rb") # パース結果が不一致であれば意図せぬ破壊をしている raise unless before === after
rubocop --autocorrectをgit blameから除外する
GitHubのBlameビューを開いたとき、ファイル全体を覆う rubocop -a コミットに遭遇したことのない歴史探検家はいないものと思います。
ここで役に立つのが .git-blame-ignore-revs ファイルです。このファイルに記述されたコミットハッシュはBlame時に無視され、関心が低いであろう rubocop -a コミットを非表示にできます。
# Migrate to Standardrb
2b0a7dd627c85ece795b65efb188b97438c144d1
コミットの整理 & デプロイ
この記事では一直線に書いていますが、実際にはいくらかの試行錯誤がありました。先述したとおり、そうしている間にも main ブランチ側は進んでゆき、それらにもautocorrectをかけねばならなくなります。
そこで、スタイルの変更の適用日を宣言し、最終的な作業が完了するまで Pull Request のマージを含む main ブランチへのコミットをしないようお願いしました。
あとは事前に作成したスクリプトをmergeの直前に main から切ったばかりのブランチに対して実行するだけです。わくわく。
この過程で生まれた差分はすべてコード的に等価なはずですが、何が起こるか分からないのが +25,252 -20,230 の規模の変更。他の差分が混ざらないよう注意し、デプロイ前後でメトリクスの変更がないことを見届けてggでした。
この記事ではある程度規模の大きいコードベースに対して、RuboCopのルールを大幅に変更したときの手順をご紹介しました。面倒な部分もありましたが、とても気持ちよくコーディングに集中できるルールに落ち着いたと感じております。
スマートバンクでは .rubocop.yml が大好きなエンジニアを募集しています。
