業務的な部分もRuboCopに警告させる

Ruby による開発で、静的にコードをチェックして書き方を強制させる RuboCop はみんな使ってると思う。 RuboCop がデフォルトで提供しているコップ(RuboCopの世界観では各リンターをこう呼ぶ)は割と簡単に作ることが出来るので、プロジェクトごとの決まりごとや業務的な部分をチェックするコップを作ってチェックさせることができる。

たとえば

すべてのコントローラで、あるヘッダをつけたりつけなかったりしたい。ヘッダをつけるかつけないかはそれぞれの箇所で開発者による判断が必要。

あるヘッダを X-foo としたとき、こんなモジュールを用意して

Copied!

module FooHeader
  def set_foo_header
    headers['X-foo'] = 'foo'
  end
end

コールバックでセットしたり

Copied!

class HogeController < ApplicationController
  include FooHeader
  before_action :set_foo_header, only: [:hoge]
  
  def hoge
    # X-foo ヘッダが必要
  end

  def fuga
    # X-foo ヘッダは不要
  end
end

# ...全てのコントローラについて X-foo ヘッダが必要かどうか判断して実装する

上記とは反対の実装(deny listっぽい実装)もできる。

Copied!

# 基底コントローラで
class ApplicationController < ActionController::Base
  before_action :set_foo_header!
end

class HogeController < ApplicationController
  include FooHeader
  skip_before_action :set_foo_header, only: [:fuga]
  
  def hoge
    # X-foo ヘッダが必要
  end

  def fuga
    # X-foo ヘッダは不要
  end
end

# ...全てのコントローラについて X-foo ヘッダが必要かどうか判断して実装する

問題点

allowlist っぽい実装にしても denylist っぽい実装にしても、将来追加されるエンドポイントに関してはどっちかを判断することを忘れる可能性がある。

RuboCop でやらせられないかな？

これらの要件についてRuboCopでカスタムcopを作成して警告させられないだろうか？ RuboCop なら CI で動かすのが容易なので、将来追加されるコントローラにたいしてもチェックを強制させられるはず。

上記の例を静的解析可能な形にするには、コールバックではなくて素朴に各エンドポイントのメソッドで set_foo_header をコールするようにして、メソッドごとに呼び出し状況をチェックすればいけそう。

ChatGPTに聞いてみたらこんな実装を出してくれた

your_path_to_custom_cops/set_foo_header.rb

Copied!

# frozen_string_literal: true

require 'rubocop'

module RuboCop
  module Cop
    module MyCustomCop

      class SetFooHeader < Base
        MSG = '対象のコントローラのfooヘッダの必要かどうかを確認し、`set_foo_header` か `no_foo_header` のどちらかを呼び出して下さい'

        def on_class(node)
          return unless controller_class?(node)

          node.each_descendant(:def) do |def_node|
            add_offense(def_node, message: MSG) unless calls_set_foo_header?(def_node)
          end
        end

        private

        def controller_class?(_node)
          processed_source.path.include?('app/controllers')
        end

        def calls_set_foo_header?(node)
          node.each_descendant(:send).any? do |send_node|
            [
              :set_foo_header,
              :no_foo_header
            ].include?(send_node.method_name)
          end
        end
      end
    end
  end
end

rubocop.yml

Copied!

require:
  # ...
  - 'your_path_to_custom_cops/set_foo_header.rb'
# ...
MyCustomCop/UnsubscribeHeader:
  Enabled: true

いいじゃん。動く。

Copied!

class HogeController < ApplicationController
  include FooHeader
  
  def hoge
    # X-foo ヘッダが必要
    set_foo_header
  end

  def fuga # rubocop:disable MyCustomCop/SetFooHeader
    # X-foo ヘッダは不要
  end
end

開発者による判断が絶対必要だから、 rubocop:disable するんじゃなくて必要ないってことを明示するメソッドを呼ばせてどちらかを必ず呼ぶ、と言うチェックでもいいかもしれない

Copied!

module FooHeader
  def set_foo_header
    headers['X-foo'] = 'foo'
  end

  def no_foo_header
    nil
  end
end

class HogeController < ApplicationController
  include FooHeader
  
  def hoge
    # X-foo ヘッダが必要
    set_foo_header
  end

  def fuga
    # X-foo ヘッダは不要
    no_foo_header
  end
end

問題点

今回のコップは public メソッドだけを対象にしたいんだけど、そう言うことはできないらしい。なので private メソッドは rubocop:disable していく必要がある。

Unfortunately, static analysis cannot be able to determine whether my_method is public (in other words, Published Interface) or private.

rubocop/rubocop/issues/10487 より

まとめ

• 業務的な部分を RuboCop にチェックさせることができた
  • 静的解析させるために工夫する必要はある
  • 無理なものもある
• ChatGPTに聞くと簡単なチェックはすぐ作ってくれる

次回予告

RuboCop の静的解析を理解する
ChatGPTに任せてたらつまんないので。