僕がrubocopに送ったPRが v0.47.0 に取り込まれました。
個人的に便利機能だと思うのでこの場を借りて軽く紹介したいと思います。
Rails/ReversibleMigrationについて
Railsのmigrationファイルで change メソッドの中に書いたmigrationコマンドがreversible *1かどうかをチェックするためのcopです
具体例
https://github.com/bbatsov/rails-style-guide#reversible-migration より抜粋
これだと drop_table :users だけだと逆方向のmigration（create_table）でどのようなカラムでテーブルを作ればいいか分からないのでreversibleではありません。rake db:rollback や rake db:migrate:down した時にエラーになります。
# bad class DropUsers < ActiveRecord::Migration def change drop_table :users end end
こういう場合、upメソッドとdownメソッドでそれぞれmigrationを書いてあげないと rake db:rollback や rake db:migrate:down が正しく実行されません
# good class DropUsers < ActiveRecord::Migration def up drop_table :users end def down create_table :users do |t| t.string :name end end end
http://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters.html#method-i-drop_table にも書いていますが、 drop_table にブロックを渡すとchangeで逆方向のmigrationを補完する時に create_table の引数扱いになりカラムが作られます。
# good # In this case, block will be used by create_table in rollback # http://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters.html#method-i-drop_table class DropUsers < ActiveRecord::Migration def change drop_table :users do |t| t.string :name end end end
どうして作ったか？
慣れてくればreversibleなコマンドかどうかは Railsの気持ちになって考えれば だいたい推測はつきます。
例えば
add_column :users, :name, :string
の逆は
remove_column :users, :name
ですが、
remove_column :users, :name
の逆方向を考えた時にどんな型でカラムを作ればいいかの情報がこれだけでは推測できないのでreversibleじゃないといった感じです。
微妙に迷った時も全部 http://api.rubyonrails.org/classes/ActiveRecord/Migration/CommandRecorder.html に書いてあるのでこれを読めば分かります。
しかしコードレビューの度にそれをいちいち指摘するのは面倒なので自動化するために Rails/ReversibleMigration を作成しました。
検出精度について
さっきのリファレンスを全部読んだ上でchangeメソッド内の下記を検出するようにしています
- 絶対にreversibleじゃないmigrationコマンド
- change_table
- change_table_comment
- execute
- remove_belongs_to
- 特定の条件を満たさないとreversibleじゃないmigrationコマンド
- drop_table
- change_column_default
- remove_column
- remove_foreign_key
ただし特例として reversibleブロックがある場合は reversibleであるとみなして検出対象外にしています。
詳細は下記参照
auto correctについて
rubocopのcopでは autocorrectメソッドを定義すればauto correct時の実装を定義することができますが、down（逆方向）のmigrationは過去のmigrationファイルを探して人間が書く必要がありそうな気がしたのであえて実装していません。
Cop開発Tips
次回以降自分がrubocop触りやすくするために開発手順とか参照すべきソースをメモっときます。
前提知識
日本語だとここが参考になりました。
rubocop静的解析したいソースをruby-parseでS式を出す
ruby_parser というRubyのソースコードを解析するためのgemを使います。rubocopのソースをcloneしていればbundle installで入るので別途gem installする必要はありません。
例えばこんなファイルを作っといて
class ExampleMigration < ActiveRecord::Migration def change # ブロックがあるのでreversible drop_table :users do |t| t.string :name end # ブロックがないのでrubocopで警告を出したい drop_table :users end end
ruby-parseに食わせてS式を出します。
bundle exec ruby-parse /tmp/migration.rb (class (const nil :ExampleMigration) (const (const nil :ActiveRecord) :Migration) (def :change (args) (begin (block (send nil :drop_table (sym :users)) (args (procarg0 :t)) (send (lvar :t) :string (sym :name))) (send nil :drop_table (sym :users)))))
rubocopのcopに on_def や on_send などのメソッドを定義することによって、rubocopで解析中にS式のその要素と対応するメソッドが実行されます。
今回の場合はメソッド呼び出し drop_table や change_table などのメソッド呼び出しをフックしたかったので on_sendメソッドを作りました。
binding.pryなどでとめてnodeの中身を確認
on系のメソッドに binding.pry や binding.irb を書いて
def on_send(node) binding.pry
任意のspecを実行
$ bundle exec rspec -- spec/rubocop/cop/rails/reversible_migration_spec.rb:85 Run options: include {:focus=>true, :locations=>{"./spec/rubocop/cop/rails/reversible_migration_spec.rb"=>[85]}} exclude {:broken=>#<Proc:./spec/spec_helper.rb:32>} Randomized with seed 63344 From: /Users/sue445/workspace/github.com/bbatsov/rubocop/lib/rubocop/cop/rails/reversible_migration.rb @ line 120 RuboCop::Cop::Rails::ReversibleMigration#on_send: 119: def on_send(node) => 120: binding.pry 121: return unless within_change_method?(node) 122: return if within_reversible_block?(node) 123: 124: check_irreversible_schema_statement_node(node) 125: check_drop_table_node(node) 126: check_change_column_default_node(node) 127: check_remove_column_node(node) 128: check_remove_foreign_key_node(node) 129: end [1] pry(#<RuboCop::Cop::Rails::ReversibleMigration>)> node => s(:send, nil, :drop_table, s(:sym, :users)) [2] pry(#<RuboCop::Cop::Rails::ReversibleMigration>)>
慣れていないうちは1回1回 binding.pry で止めてnodeの中身を確認しつつデバッグしていくのがいいと思います。余談ですがデバッグに慣れていなさすぎて1日 1migrationコマンド対応が限界でした('A`)
S式にマッチさせるmatcherを書く
cop内に
def_node_matcher :drop_table_call, <<-END (send nil :drop_table ...) END
のようなmatcherを書くことによって、
drop_table_call(node) do # drop_table メソッドの呼び出しがあればこの中が評価される end
のように処理を挟むことができます。
matcherの書き方は https://github.com/bbatsov/rubocop/blob/master/lib/rubocop/node_pattern.rb のコメントを要熟読
matcherに $_ や $... を書くことにより、マッチした文字列を変数として取得することもできます。
$_ は単一nodeを取得
# remove_foreign_keyに渡された引数の2つ目のみを取得するmatcher def_node_matcher :remove_foreign_key_call, <<-END (send nil :remove_foreign_key _ $_) END
remove_foreign_key_call(node) do |arg| # $_ の内容がブロック引数のargに入ってくる if arg.hash_type? # remove_foreign_keyの第2引数がHash（テーブル名以外）の時に警告を出す add_offense( node, :expression, format(MSG, 'remove_foreign_key(without table)') ) end end
$... は以降のnodeを全部取得
# remove_columnに渡された引数を全部取得するmatcher def_node_matcher :remove_column_call, <<-END (send nil :remove_column $...) END
remove_column_call(node) do |args| # argsに引数全部入ってくる if args.to_a.size < 3 # 引数が3個未満の時に警告を出す add_offense( node, :expression, format(MSG, 'remove_column(without type)') ) end end
*1:rake db:rollback や rake db:migrate:down した時に逆方向のmigrationを自動補完する
