ピクスタ開発部で毎日ヒィヒィ言いながらエンジニアをやっております @muramurasan です。
今回はPIXTAのとあるリポジトリにおいて、未使用のメソッドを削除しようとした際、gemを組み合わせることで、効率的かつ安全に削除することができたという話をしたいと思います。
よくやる方式
外部の勉強会などで、「未使用のメソッドを削除する際にどうしているか？」ということを聞いた際、よく聞くのが「未使用らしきコードを見つけ次第、ロギングを行うメソッド呼び出しを挟み込んでいく」というものでした。
この方式は、動的なメソッド呼び出しにも当然対応できますし、お手軽なので、一般的に好まれているようです。
問題点
ただし、この方式では以下の問題点があると私は考えています。
- そもそも、未使用らしいメソッドを見つけるのが大変
- プロダクションコードを汚してしまう
これらの問題を解決するために、PIXTAでは debride というgemと、okuribito_rails というRails Engine を組み合わせて使うことにしました。
静的解析で未使用の可能性があるメソッド一覧を取得する 〜 debride 〜
debride は簡単に説明すると、静的解析により「未使用の可能性があるメソッド一覧を取得できる」というgemです。
下記のように、解析をかけたいフォルダを指定してコマンドを実行すると、「未使用の可能性があるメソッド一覧」をつらつらと表示してくれます。
% debride lib These methods MIGHT not be called: MyClass good_method lib/some/file.rb:16 bad_method lib/some/file.rb:20 ...
このdebrideを使えば、よくやる方式の問題点であげた「そもそも、未使用らしいメソッドを見つけるのが大変」を解決できそうです。
debrideの長所
何より、手っ取り早く「未使用の可能性があるメソッド一覧を取得できる」ことでしょう。
加えて精度が高く、解析速度も非常に速い(軽い)優秀なgemです。
gemのインストール後は、コマンド一本で解析を走らせることができるため、非常にお手軽なのも魅力的です。
debrideの短所
本家githubのDESCRIPTIONにも記載されている通り、あくまで「未使用の可能性がある」に留まっているのが惜しいところです。
Analyze code for potentially uncalled / dead methods, now with auto-removal.
RubyKaigi 2016でも作者本人が言っていましたが、どうしても動的なメソッド呼び出しには対応できないのが悩み所とのことです。
そのため、debrideの解析結果をそのまま鵜呑みにしてメソッドを削除してしまうと、思わぬしっぺ返しを食らうことになるかもしれません。
動的監視で本当にそのメソッドが使われていないか確認する 〜 okuribito_rails 〜
そこで、上記で述べたdebrideの問題点を解決するために筆者が開発したのが、okuribito_rails という、メソッド呼び出しの動的監視を行うRails Engineです。
okuribito_railsは、yamlで指定したメソッドの呼び出しを監視し、メソッド呼び出しが行われた際にその情報をDBに登録します。
DBに登録された情報は、後からWebUIを通じて一覧・検索することができます。
これらの機能を言い換えるならば、debrideで未使用と検出したメソッド群が、本当に未使用なのかを見定めることができるRails Engineと言うことができます。
okuribito_railsの長所
外からyamlで監視したいメソッドを指定することができるため、「プロダクションコードを変更せずにメソッド呼び出しを監視できる」ことが強みと言えます。
okuribito_railsを使えば、よくやる方式の問題点であげた「プロダクションコードを汚してしまう」を解決することができます。
また、メソッド監視に使うyamlの作成が面倒くさそう......と思われるかもしれませんが、debrideの解析結果をそのままokuribito_rails向けのyamlに変換してくれるgem*1も用意してあるため、出力されるyamlをそのまま使うことができます。
okuribito_railsの短所
恐らく、ここまで読んでくださった方の多くが心配されていると思うのですが、メソッド呼び出しフック時のオーバーヘッドは気にならないレベルです。少なくともnewrelicで監視した範囲では、目立ったパフォーマンス低下は見られません(※)でした。
(※ 少しでもパフォーマンス低下を抑えるために、once_detectという、Railsアプリケーション起動中、各メソッド一度だけメソッド呼び出しを検出するコンフィグを有効にしておく必要があります)
他、短所といえば、migrationを利用しているため、本体側で利用しているDBにテーブルが寄生してしまうことです。
これは運用上NGと判断するプロジェクトもあるかと思われますので、代替手段の提供を現在検討中です。
また、監視開始はRails起動直後に走るため、設定がダメだと、そもそもRails起動時点でコケる可能性があります。十分に手元の環境で試してから本番への投入をご検討ください。
他にも、Helperやマクロ(before_actionなど)で指定されるメソッド呼び出しはうまく呼び出し検出してくれないなど、まだまだ不安定なところがありますが、大半の呼び出し方はカバーできていますので、サポートできていない呼び出し方にさえ気をつければ、十分に実用できるかと思います。
PIXTAで採用した、未使用のメソッド削除までのフロー
ここまで debride と okuribito_rails の紹介をさせていただきましたが、PIXTAで採用した、未使用のメソッド削除までのフローは以下の通りです。
- debrideにソースコードを食わせて、未使用の可能性が高いメソッド一覧を取得する！ (debride2okuribito でokuribito_rails向けのyamlを取得する！)
- okuribito_rails の監視に投入し、しばらく監視する！(弊社例では3週間ほど)
- okuribito_railsでも呼ばれていない、と判断されたメソッド群を消す！
1, 2 のフェーズはほぼ工数かけずに行けるため、実際にはdebrideで解析してokuribito_railsの監視に入れた後は、ほったらかしでOKでした。こうすることで、ほぼ工数をかけずに「恐らく安全に消せるメソッド群」を簡単に炙り出すことができました！
なお、実際に削除したメソッド数は97、テストコードを含めた削除行数は約1000にのぼりました。
もちろん、一斉削除後の不具合報告は一切ありません。
この負債返還を軽いと見るか、大きいと見るかは......読者の方にお任せします！
まとめ
debrideとokuribito_railsを組み合わせることで、効率的かつ安全に未使用のメソッドを削除できることを説明してきましたが、如何でしたでしょうか？
デッドコード削除というのは、放置されがちなリファクタリング業の中でも、更に放置されやすい問題です。
(動的メソッド呼び出しが存在するRubyにおいては、なおのことですよね)
デッドコードは放置すると、毒のように可読性を下げていく要因になるものです。この期に一気に大掃除してしまいませんか？
その際、今回提案した手法を、アプローチの一つとしてご検討いただけると幸いです。
さあ、デッドコードとはサヨナラして、優しい世界を手に入れましょう！
ピクスタでは一緒に働くメンバーを募集しています！ recruit.pixta.co.jp
