こんにちは、プロダクトエンジニアのkitazawaです。
みなさんRuboCop使っていますか?静的コード解析でさまざまなチェックをしてくれて便利ですよね。 私が担当しているプロダクトでももちろん利用しており、とても活躍しています。
そんな便利なRuboCopですが、既存のルールだけではなく自分でルールを作ることもできます。 今回はルールを自作した話と、その作ったルールrubocop-rspecに取り込んでもらった話を紹介します。
間違ったテストコードの発見
ある日、コードを修正しているときにRSpecで以下のようなテストが書いてあることに気付きました。
it { expect(do_something).kind_of? Foo }
このテストはうまく動いていなかったのですが、理由がわかるでしょうか?
すぐにわかった方は流石です!実は .to be_kind_of
であるべきところが .kind_of?
になっています。
これではテストは実行されず、 kind_of?
が呼び出されて false
を返すだけのコードになっていました。
私は最初見たときには気付かず、失敗すると思っていたテストが成功してやっと気付きました。
よく間違えそうなコードに思えたので既存のルールがないか探してみました。
RuboCopには標準のルール以外にextensionという、外部ライブラリに対応したルールを持つ拡張ライブラリがあります。 そして、RSpecに対応したextensionとして、rubocop-rspecがあります。 ここにルールがないか調べてみました。しかし見つかりません。 社内のメンバーにも聞いてみましたが誰も知らず、どうやら既存のルールは存在しないようでした。
ところで、このコードはなんだか機械的に判断できそうに見えませんか? ルールがないなら自分で作ってしまえば良さそうです!
ちなみにRuboCopのルールはCopと呼ばれているため、以降はルールをCopと表現します。
Copの自作方法
実はRuboCopのCopを自作するのは意外と簡単で、以下のような決まったインターフェースを持つクラスを作成するだけです。
RuboCop::Cop::Base
を継承するMSG
定数でエラーメッセージを定義するon_***
コールバックをメソッド定義し、ルールに違反していたらadd_offense
を呼び出す
これらの条件を満たすサンプルコードを書いてみます。
例えば lib/custom_cops
というディレクトリに sugoi_cop.rb
というファイルを以下のように作成します。
コールバックのメソッドはいくつかありますが、ここでは on_send
を定義します。
module CustomCops class SugoiCop < Rubocop::Cop::Base MSG = "違反時のメッセージ" def on_send(node) return unless check_something add_offense(node) end end end
そして .rubocop.yml
で require
します。
require: - ./lib/custom_cops/sugoi_cop
たったこれだけです! 肝心のチェックをどう書くかですが、詳しいことはRuboCopのドキュメントを読んでみてください。
間違ったテストコードを検知するCopの作成
さて、ここまででざっくりCopの作り方がわかったので、今回の問題になったコードについてのCopを考えてみたいと思います。
RSpecの場合テストを実行するには expect
, is_expected
の後に .to
, .not_to
, .to_not
メソッドを呼び出し、引数にマッチャーを指定します。
expect(foo).to eq 42
このような形式です。そのため、 .to
, .not_to
, .to_not
を呼び出していないコードをチェックすればよさそうです。
上記のコードをパースすると、次のようなASTになっています。このパース結果が重要なので覚えておきます。
$ ruby-parse -e 'expect(foo).to eq 42' (send (send nil :expect (send nil :foo)) :to (send nil :eq (int 42)))
on_send
コールバックは多く呼ばれるため、定数 RESTRICT_ON_SEND
で使用するメソッド名を制限することで最適化できます。
def_node_matcher
はRubyをパースした結果のNodePatternとマッチするか確認するメソッドを定義できます。
ここで先程パースした結果を元に、違反しているコードを見つけるためのパターンを定義します。
NodePatternの詳細はrubocop-astのドキュメントを参照してください。
これらを組み合わせると以下のようなコードができあがります。
RESTRICT_ON_SEND = [:expect].freeze # @!method expectation_without_runner?(node) def_node_matcher :expectation_without_runner?, <<~PATTERN (send (send nil? :expect ...) !{:to :not_to :to_not} ...) PATTERN def on_send(node) expectation_without_runner?(node.parent) do add_offense(node.parent.loc.selector) end end
この他にも is_expected
を使った場合や、ブロックを渡したパターンを考慮する必要がありました。
自作したCopを早速プロダクトで実行してみると、いくつか問題を見つけることができました。
また、RuboCopはCIで実行しているため、今後同じ間違いをしなくなる点もメリットです。
rubocop-rspecへのコントリビュート
さて、私のプロダクトではチェックできるようになりましたが、今回のCopは特にプロダクト固有の事情はないので他のプロダクトでも利用できそうです。
さらに言うと、全RSpec利用者にとって価値がありそうです!つまり rubocop-rspec
に追加できるとよさそうに思いました。
ということで出したPRが以下になります。
github.com
大量のレビューをもらっていて少し恥ずかしいですが、無事マージしてもらえました。 レビューを受けて感じたのは、変更内容が必要最小限であることの大切さでした。 日頃気を付けているつもりではありますが、変更内容が多いとレビューの負担が上がってしまいます。 また、コードを読んだ時の認知負荷も高くなります。 これはOSSかどうかに限らず、日々の開発の中でも意識していけると良いと思いました。
ちなみに、マージされたのがちょうどメジャーバージョンアップであるrubocop-rspec 3.0を出すタイミングだったため、デフォルト有効な状態で入っています。
おわりに
人の目でレビューするだけでは見つけられない問題もRuboCopで自動化することで見逃すことがなくなります。 アプリケーション固有のルールについても簡単に追加することができるので、ぜひ皆さんもCopを作ってみてください。
We Are Hiring!
SmartHR では一緒に SmartHR を作りあげていく仲間を募集中です!
最近、社内ではOSS貢献を支援する有志の会が立ち上がったので、気軽に相談する場があります! OSS貢献に興味のある方も、そうでもない方も少しでも興味を持っていただけたら、カジュアル面談でざっくばらんにお話ししましょう!