これは Livesense Advent Calendar 2024 DAY 1 の記事です。
はじめに
普段アルバイト事業部で主にマッハバイトの開発をしている@ayumu838です。
マッハバイトでデッドコードを削除したく、やり方を導入してみました。
また今回の内容については2024/11/28に行われたyabaibuki.dev#3で発表した内容の詳細版になります。
当日の発表資料はこちらです。
なぜやるのか
- マッハバイト自体が歴史が長いプロダクトになる
- 2006年にジョブセンスとしてリリースし、当初は素のPHP、その後symfonyを経て2014年ごろからRuby on Railsにリプレイスを開始し、2022年に完了した
- リプレイス開始から10年経っており、何が不要かどうかもわからなくなっている
それを踏まえたうえで不要なコードを削除しつつ、削除漏れがないような体制を構築したいです。
不要なコードを検知する
まず不要なコードを削除するにあたってどうやって不要なコードを検知するかを検討しました。
そこで、Ruby2.6で導入された一度でも実行された行を記録するoneshot coverageを利用することにしました。
実は以前導入を検討したことがあったのですが、oneshot coverageの形式が以下の形となっており、ログファイルに出力されます。
{ファイル名=>{oneshot_lines => [実行された行数] }
しかし、以下の問題点があり導入を一度やめていました。
- 可視化する方法が見えてなかった
- ログの保存先はどうするのか
- コードが変わった場合追随できるのか
今年になりあたらめて調査をすると、DIGGLE株式会社さんの以下の記事を見つけました。
ちょうど問題点も同じような形であり、自分たちのプロダクトにも導入しようと思いました。
導入時にあたり上記記事で基本的なことは説明されていますので、細かいことは省略させていただきます。
導入前はredisに保存するということで容量の心配から二の足を踏んでいたのですが、実際に開発環境に導入してみてどれぐらい容量を使うかを調べました。
おおよそ1000ファイル、25,000行のカバレッジを記録するにあたり0.3MiB程度となっていました。
この程度であれば許容範囲なので実際に導入するに至りました。
また、処理時間の増加についてもほとんどオーバーヘッドがなくその点が問題ないことを確認しています。
また、上記記事に無いはまりどころとしては、redisの接続先情報を環境ごとに config/environments/配下に配置されているファイルを使って保持していました。
公式のREADMEにあるようにconfig/ 直下に にcoverband.rbをおくと設定ファイルの読み込み順の関係から、environments配下で設定されている値は使われません。
そのため、config/initializers/coverband.rbを設置し動かすことにしました。
oneshot coverageが有効になっていない状態ではうまく動いているように見えましたが、有効にするとエラーになってしまいました。
原因を深掘りしていくと、 以下のリンクの処理でCoverage.start が config/initializers/coverband.rb が読み込まれる前に実行され、oneshot coverageが有効にならない状態でcoverageを計測し始めているのが原因でした。
そのため、config/coverband.rb ではuse_oneshot_lines_coverageを有効にし、その他の設定は config/initializers/coverband.rbですることにより解決しました。
最終的以下のようなファイル構造になります。
config/ ├── coverband.rb ├── initializers/ │ ├── coverband.rb
実際に導入してみて、以下のように表示されます。
ここで不要な行だけではなく、一度も呼び出しがないviewやrouteが検知できるようになりました。
実際に削除していく
oneshot coverageとcoverbandで不要なコードなどの検知が出来るようになりました。
そこから実際に削除をしたいとなったときに以下のユースケースになるとしました。
- 呼び出しがない行を削除した結果、不要になったroutesも一緒に消したい
- 呼び出しがないroutesを削除した結果、不要になったcontrollerやactionも一緒に消したい これに対して検知する仕組みを考えました。
routesの取得についてはRouting Error画面にある Controller#Action の形式で表示されいるので、何か手段があるはずだということで調べました。
当初は rails routes に -E オプションの結果である
Prefix | hoge Verb | GET URI | /hoge Controller#Action | hoge#index
の結果を利用することを考えましたが、パース関係が複雑になりそうだったので別手段を探すことにしました。
Railsコンソールから実行できるroutes関係のメソッドで何か取得できると考えドキュメントを探しました。
最終的に、 Rails.application.routes.routesで取得できる、 ActionDispatch::Journey::Routesを利用する形になりました。
Rails.application.routes.routes だと複数になるので、その中の個別要素である、
ActionDispatch::Journey::Routeを見ると#defaultsに以下のような欲しい情報がありました。
{
:contrller => {contrller名},
:action => {action名}
}
ひとまず、欲しいroutes関係の情報が取得できたのでcontrollerやactionについて考えみます。
こちらは存在しないケースを含めて取得したかったので、やや力技ではありますがファイルを直接見ることにしました。
controllerについては、上記で取得できた controller名 を使って app/controllers/#{controller名}_controller.rbとすることで取得ができそうです。
actionについても同様に、上記のファイルからdef {action名} が存在しているかを確認すればできます。
欲しい情報については取得できることがわかったので、これをCIに組み込む方法について考えてみます。
CIに組み込むにあたり、Rails.application.routes.routes を使っているので、railsが使える必要があります。
コマンドラインツール - Railsガイドを見ると rails runnerが使えそうだったので、それを使い実行させることにしました。
また、CI化において以下の機能も追加したく実装しています。 - エラーメッセージをPR上に表示する - 特定のエラーを無視できるようにする エラーメッセージWorkflow commands for GitHub Actionsを参考にしました。
実際に表示する形式は以下のように、
::error file={ファイル名}, title={タイトル}
行については存在しないケースを検知するため省略しています。
Railsでデフォルトで作られるものだったり、継承のためだけに作るものがあるので、特定のケースを無視する機能を作ることにしました。
特定のエラーについても無視できるようにするにあたり、設定項目は以下の3つにしています。
name: 無視したいケース comment: 理由やなぜ無視しているのかのコメント expire: いつまで無視するか
expireについては開発中で存在していないため、一時的に無視したい場合に永続的に無視し続けることを防ぐために追加しました。
# frozen_string_literal: true module Scripts class UnusedControllers class << self def search_all if not_defined_routes.present? || not_defined_controllers.present? || not_defined_actions.present? # controllerのみ存在し、routeが存在しない場合 not_defined_routes.each do |route| puts "::error file=app/controllers/#{route}_controller.rb,title= route is not defined::#{route} is not defined in config/routes.rb" end # routeのみ存在し、controllerが存在しない場合 not_defined_controllers.each do |controller| puts "::error file=config/routes.rb,title=controller is not defined::#{controller}_controller is not defined in app/controllers" end # routeにのみ存在し、controllerに存在しないactionがある場合 not_defined_actions.each do |controller, action| puts "::error file=app/controllers/#{controller}_controller.rb,title=action is not defined::#{controller}##{action} is not defined" end exit 1 end end private def route_contollers_and_actions return @route_contollers_and_actions if @route_contollers_and_actions @route_contollers_and_actions = {} Rails.application.routes.routes.each do |route| controller = route.defaults[:controller] action = route.defaults[:action] # 空白とactive_storageやrailsなどで使われるcontrollerは除外 next if controller.nil? next if controller.start_with?('active_storage/', 'rails/', 'action_mailbox/') @route_contollers_and_actions[controller] ||= [] @route_contollers_and_actions[controller] << action end @route_contollers_and_actions end def route_contoller_names @route_contoller_names ||= route_contollers_and_actions.keys end def controller_filenames return @controller_filenames if @controller_filenames @controller_filenames = Dir.glob('app/controllers/**/*_controller.rb').map do |filename| filename.gsub('app/controllers/', '').gsub('_controller.rb', '') end.compact.uniq @controller_filenames end def not_defined_controllers key = 'not_defined_controllers' results = route_contoller_names - controller_filenames results.reject do |result| find_json_with_key_name(key, result).present? end end def not_defined_routes key = 'not_defined_routes' results = controller_filenames - route_contoller_names results.reject do |result| find_json_with_key_name(key, result).present? end end def not_defined_actions return @not_defined_actions if @not_defined_actions key = 'not_defined_actions' @not_defined_actions = [] route_contollers_and_actions.each do |controller, actions| file_name = "app/controllers/#{controller}_controller.rb" next unless File.exist?(file_name) file_content = File.read(file_name) actions.each do |action| next if file_content.include?("def #{action}") value = "#{controller}##{action}" next if find_json_with_key_name(key, value) @not_defined_actions << [controller, action] end end @not_defined_actions end def read_ignore_json @ignore_json ||= JSON.parse(File.read('lib/scripts/ignore_unused_controllers.json')) end def find_json_with_key_name(key, name) return if read_ignore_json[key].nil? read_ignore_json[key].find { |record| record['name'] == name && (record['expire'].nil? || Date.parse(record['expire']) >= Time.zone.today) } end end end end
また、特定のケースのみ無視したい場合のファイルはJSONで以下のように持たせています。
{ // controllerのみ存在し、routeが存在しない場合 "not_defined_routes": [ { "name": "application", "comment": "applicationは継承元のため", "expire": null // nullの場合永続的に無視する } ], // routeのみ存在し、controllerが存在しない場合 "not_defined_controllers": [ ], // routeにのみ存在し、controllerに存在しないactionがある場合 "not_defined_actions": [ ]
これを実際にGitHub Actionで動作させるために
check-unused-controller:
runs-on: ubuntu-latest
env:
RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }}
RAILS_ENV: test
steps:
- uses: actions/checkout@v4
- uses: ruby/setup-ruby@v1
with:
ruby-version: ${{ env.RUBY_VERSION }}
bundler-cache: true
- name: Check unused controllers
run: bundle exec rails runner Scripts::UnusedControllers.search_all
と設定してCIに組み込んでいます。
おわりに
この仕組みを導入するにあたり、本番でエラーとなっている箇所を発見するなど意図しない嬉しい副作用がありました。
CIに組み込んだので今後は防ぐことができ、また削除がしやすくなったのでとてもいい感じになりました。
