タイミーでバックエンドエンジニアをしている新谷 id:euglena1215 です。
今回は社内で決めたコーディングルールに強制力を持たせるために CustomCop を作った話を紹介します。
背景
タイミーの Rails アプリケーションには /app/services ディレクトリがあり、 Service クラスが存在しています。
これまで社内で Service クラスは、なるべく使わない方が好ましいものの、どんな時に使っていいかは特段明言されていない状況でした。
その結果かは分かりませんが、一部の機能では Service クラスを多用し Service クラスが Service クラスを呼んでいるなど複雑になっており、コードリーディングの負荷が高まっていました。
この現状に課題感を持った @rhiroe が以下のような問題提起を行いました。
この問題提起を受け、チーム横断の技術領域ごとのトピックについて話し合うMTGで今後の Serivce クラスの運用方針は以下のように決まりました。
- Serviceクラスは基本的に積極的な利用は推奨しない。
- Service クラスは Controller や Sidekiq Worker から呼ぶものと位置付ける。それら以外から呼びたくなった場合は Model の作成を検討すること。
方針についてバックエンドエンジニア全体に周知は行われましたが、あくまで「きちんと覚えておきましょうね」に留まり、仮に実装者とレビュアーが思い出すことができなければそのままマージされてしまいます。
そのため「このルールを機械的に適用できるといいよね」という話があり、用途に合わせた CustomCop を作ることになりました。
特定のsuffixを持つクラス以外からのサービスクラス呼び出しを警告する cop
早速ですが、CustomCop の実装をまるっと公開します。
# frozen_string_literal: true begin require 'rubocop' rescue LoadError return end module CustomCops # 特定のsuffixを持つクラス以外からサービスクラスを呼び出した場合に警告します。 # AllowSuffix は config パラメータで設定できます。 # # @example # # bad # # AllowSuffix に Service が含まれていない場合 # class SampleService # def call # Service2Service.new.call # end # end # # # good # # AllowSuffix に Service が含まれている場合 # class SampleService # def call # Service2Service.new.call # end # end # class AllowServiceCallClassSuffix < RuboCop::Cop::Base MSG = '特定のsuffixを持つクラス以外からサービスクラスを呼び出すことはできません。%<suffixes>s のみが許可されています。' # ① 許可されている Suffix を持つクラスの定義かどうか # ② クラス定義の中で更にクラスかモジュールを定義しているかどうか # この cop で重要なのは最も深いクラス定義での情報なのでネストしているものは無視(≒許容)する def_node_matcher :define_allow_class?, <<~PATTERN { (class (const ... #allow_class_name_suffix?) ...) (class const ... {class module}) } PATTERN # (send (const nil? #service_class_name?) :new ...) => FooService.new(...) # (send (const cbase #service_class_name?) :new ...) => ::FooService.new(...) # (send (const const #service_class_name?) :new ...) => Bar::FooService.new(...) def_node_matcher :call_service_class?, <<~PATTERN (send (const {nil? cbase const} #service_class_name?) :new ...) PATTERN # on_class はクラス定義が行われた時に呼び出されるメソッド def on_class(node) # 許容されるクラスであれば無視する return if define_allow_class?(node) node.each_descendant(:send) do |send_node| # サービスクラスの呼び出しをしていれば警告する add_offense(send_node, message:) if call_service_class?(send_node) end end private # AllowSuffix で指定した suffix を警告メッセージに加えたかったので上書きしている def message format(MSG, suffixes: allow_suffixes.join(', ')) end # call_service_class? マッチャで使っている def service_class_name?(name) name.to_s.end_with?('Service') end # define_allow_class? マッチャで使っている def allow_class_name_suffix?(name) allow_suffixes.any? { |suffix| name.to_s.end_with?(suffix) } end def allow_suffixes # `cop_config` で rubocop.yml で指定した設定を取得できる @allow_suffixes ||= cop_config['AllowSuffix'] || [] end end end
rubocop.yml には以下のような記述を行います。
CustomCops/AllowServiceCallClassSuffix: AllowSuffix: - Controller - Worker
どんな cop かを軽く説明します。
rubocop.yml で指定した AllowSuffix を suffix に持つクラスからの Service クラスの呼び出しを許容し、それ以外のクラスからの呼び出し時には警告を出します。
タイミーでは、Service クラスに XXXService.call のような特異メソッドを用意せず、 XXXService.new.call というようにインスタンスを作成した上で呼び出すことが通例となっているため XXXService の new メソッドが呼び出されたことを検知しています。
ここは、各社の通例に合わせてもいいですし、任意のメソッドを呼び出したことを検知しても良いと思います。
また、 service_class_name? メソッドではクラス名の suffix が "Service" かどうかをチェックしていますが、ここを変更すれば Service クラス以外にも適用可能です。
Model, View, Controller 以外のレイヤを採用している Rails アプリケーションであれば、Service クラス以外でも呼び出し箇所を制限したいことがあるかもしれません。
結果どうなったか
上記の CustomCop をタイミーの Rails アプリケーションに導入した結果、違反している箇所が67件見つかりました。違反しているファイルを rubocop_todo.yml から眺めてみると、Model から Service を呼び出しているケースと Service から Service を呼び出しているケースが半々くらいのようです。
2024/01/24 時点では、導入したばかりでこれらの違反を解決するための動きは取れていません。 ですが、増やそうとすると RuboCop に怒られるのでこれ以上増えることはないのではないかと考えています（そう信じています）。
タイミーのプロダクトは 1つのモノリスな Rails アプリケーションの上で動いています。その中での67件が多いか少ないかはみなさんの判断にお任せします。我々はこれが伸びしろだと捉え、がんばっていこうと思います。
