目次
開発をする際にテストを利用しする事は、今日最早あたりまえになったと思います。
また、GitHubやBitBucketといったWebサービスの普及により、merge前にcode reviewやテストを行なう事もあたりまえになったと思います。
それでは、テストで担保出来ない点はどうでしょう。例えば
- コードの複雑性
- 変数名の妥当性
- フレームワークに沿ったコーディング
- 紛らわしい評価のされるコード
- etc.
などはテストでは担保されませんし、reviewerの負担になります。
では、どのようにしてその負担を減らす事が出来るでしょうか。
この記事では、Ruby on Rails のプロダクトをターゲットに、これらの unit testを走らせる前にreviewの支援を行なうツール について論じてみようと思います。
どのようなツールがあるか
現在、Rubyにおいては各種そのような支援プロダクトがあると思いますが、今回は利用者も多いと考えられる、
を試していきます。
RuboCop
特徴
Copと飛ばれる各種検査モジュールがあり、それらが表記揺れであったり、文法であったり、methodの複雑性の検査を行ないます。
また、 .rubocop.yml と呼ばれるファイルでそれらの on / off, 閾値の設定などが可能となっています。
さらに、 auto-correct という機能があり、RuboCopが判断出来る範囲で規約通りに修正してくれます。
rails_best_practices
特徴
rails-bestpractices.com というrails applicationを実装する際のベストプラクティス集サイトがあります。このベストプラクティスに沿っているかを検査する機能を持ちます。
Ruby on RailsのMVCの静的解析などが可能となっています。
また Ruby on Rails標準のORMである ActiveRecord 以外にも mongoid や mongomapper にも対応しています。
template engineも erb はもちろん、 slim, haml, rabl 等豊富です。
試してみる。
対象
- ruby 2.4.2
- ターゲットはrailsアプリの代表格で、gem / rails_best_practices を採用していない Redmineの3.4.3を対象とします。
- redmineを動作させる事は目的としませんので、起動までは確認しません。
RuboCop
導入
Gemfileの書き換え
group :testに
gem 'rubocop', require: false
を追加します。
database.ymlの作成
bundle install 時にGemfileでdatabase.ymlを見ているので追加します。
production: &production adapter: sqlite3 database: db/redmine.sqlite3 development: <<: *production test: <<: *production
Gemの導入
$ bundle install --path=vendor/bundle --binstubs
環境によっては事前にImageMagick6を入れておく必要があります。
作動させてみる
$ bin/rubocop -P
Pオプションで並列実行されます。rubocopは比較的重い処理の為、CPUが多数ある環境では付けておくと良いでしょう。
さて結果は、
以上略 db/migrate/20110226120112_change_repositories_password_limit.rb:1:1: C: Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true. class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration ^ db/migrate/20110226120112_change_repositories_password_limit.rb:3:54: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => nil, :default => '' ^^^^^^^^^ db/migrate/20110226120112_change_repositories_password_limit.rb:3:69: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => nil, :default => '' ^^^^^^^^^^^ db/migrate/20110226120112_change_repositories_password_limit.rb:3:81: C: Metrics/LineLength: Line is too long. [82/80] change_column :repositories, :password, :string, :limit => nil, :default => '' ^^ db/migrate/20110226120112_change_repositories_password_limit.rb:7:54: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => 60, :default => '' ^^^^^^^^^ db/migrate/20110226120112_change_repositories_password_limit.rb:7:68: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => 60, :default => '' ^^^^^^^^^^^ db/migrate/20110226120112_change_repositories_password_limit.rb:7:81: C: Metrics/LineLength: Line is too long. [81/80] change_column :repositories, :password, :string, :limit => 60, :default => '' ^ 979 files inspected, 64504 offenses detected
読者のみなさんを驚かせてしまったかもしれませんが、細かい設定をしないでrubocopを作動させると、非常に多数の違反を検知し、今回は64504件の違反（ちなみに原文のoffenseは犯罪の意味もあり、cop(警官)ならではのジョークを感じます）という結果となりました。
failに対応してみる
上記のOffenceの大まかな理由は、
- Stringをfrozen指定するCommentが入っていない(Style系Cop)
- hashのsyntaxがruby 1.9系の書式(Style系Cop)
- 1行が長すぎる(Metrics系Cop)
になります。
まずは、これらに対応してみましょう。
diff --git a/db/migrate/20110226120112_change_repositories_password_limit.rb b/db/migrate/20110226120112_change_repositories_password_limit.rb index 1ad937c7d..0d244f031 100644 --- a/db/migrate/20110226120112_change_repositories_password_limit.rb +++ b/db/migrate/20110226120112_change_repositories_password_limit.rb @@ -1,9 +1,10 @@ +# frozen_string_literal: true class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration def self.up - change_column :repositories, :password, :string, :limit => nil, :default => '' + change_column :repositories, :password, :string, limit: nil, default: '' end def self.down - change_column :repositories, :password, :string, :limit => 60, :default => '' + change_column :repositories, :password, :string, limit: 60, default: '' end end
そして再実行。
$ bin/rubocop db/migrate/20110226120112_change_repositories_password_limit.rb Inspecting 1 file C Offenses: db/migrate/20110226120112_change_repositories_password_limit.rb:2:1: C: Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments. class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration ^ db/migrate/20110226120112_change_repositories_password_limit.rb:2:1: C: Style/Documentation: Missing top-level class documentation comment. class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration ^^^^^ 1 file inspected, 2 offenses detected
修正した事により、他の問題が出たようです。
- MagicComment後に空行が入ってない。(Layout系Cop)
- TopLevelのclassに対してドキュメントが記述されていない。(Style系Cop)
上記ではさらに細かい部分が検知されていますが、これらも検知されないように、さらに対応してみましょう。
diff --git a/db/migrate/20110226120112_change_repositories_password_limit.rb b/db/migrate/20110226120112_change_repositories_password_limit.rb index 1ad937c7d..29c91ad56 100644 --- a/db/migrate/20110226120112_change_repositories_password_limit.rb +++ b/db/migrate/20110226120112_change_repositories_password_limit.rb @@ -1,9 +1,12 @@ +# frozen_string_literal: true + +# @class Abracadabra class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration def self.up - change_column :repositories, :password, :string, :limit => nil, :default => '' + change_column :repositories, :password, :string, limit: nil, default: '' end def self.down - change_column :repositories, :password, :string, :limit => 60, :default => '' + change_column :repositories, :password, :string, limit: 60, default: '' end end
そして再実行。
$ bin/rubocop db/migrate/20110226120112_change_repositories_password_limit.rb Inspecting 1 file . 1 file inspected, no offenses detected
ようやくrubocopを納得させました。
ちなみに --auto-correct オプションを使う事により、殆どの内容を自動修正する事が可能でした。
$ bin/rubocop -a db/migrate/20110226120112_change_repositories_password_limit.rb Inspecting 1 file C Offenses: db/migrate/20110226120112_change_repositories_password_limit.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true. class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration ^ db/migrate/20110226120112_change_repositories_password_limit.rb:2:1: C: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments. class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration ^ db/migrate/20110226120112_change_repositories_password_limit.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment. class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration ^^^^^ db/migrate/20110226120112_change_repositories_password_limit.rb:3:54: C: [Corrected] Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => nil, :default => '' ^^^^^^^^^ db/migrate/20110226120112_change_repositories_password_limit.rb:3:69: C: [Corrected] Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => nil, :default => '' ^^^^^^^^^^^ db/migrate/20110226120112_change_repositories_password_limit.rb:7:54: C: [Corrected] Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => 60, :default => '' ^^^^^^^^^ db/migrate/20110226120112_change_repositories_password_limit.rb:7:68: C: [Corrected] Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => 60, :default => '' ^^^^^^^^^^^ 1 file inspected, 7 offenses detected, 6 offenses corrected
設定ファイルでコーディングスタンダードを定める
.rubocop.yml を設定をする事により、重要視しないCopを無効化したり、Metrics系等の閾値を調整したりする事が可能です。
プロジェクトは規約を遵守する為でなく、効率良く開発する為に規約が存在すると私は考えるので、この全てを押し付けず調整出来る機能はありがたいです。
先程のsourceを元に戻し、以下の内容で rubocop.yml を作ってみましょう。ただし、この内容は私が妥当と思う内容では決して無く、あくまでテスト用です。
Style/Documentation: Enabled: false Layout/EmptyLineAfterMagicComment: Enabled: false Metrics/LineLength: Max: 100
実行結果は以下の通りです。
$ bin/rubocop db/migrate/20110226120112_change_repositories_password_limit.rb Inspecting 1 file C Offenses: db/migrate/20110226120112_change_repositories_password_limit.rb:1:1: C: Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true. class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration ^ db/migrate/20110226120112_change_repositories_password_limit.rb:3:54: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => nil, :default => '' ^^^^^^^^^ db/migrate/20110226120112_change_repositories_password_limit.rb:3:69: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => nil, :default => '' ^^^^^^^^^^^ db/migrate/20110226120112_change_repositories_password_limit.rb:7:54: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => 60, :default => '' ^^^^^^^^^ db/migrate/20110226120112_change_repositories_password_limit.rb:7:68: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax. change_column :repositories, :password, :string, :limit => 60, :default => '' ^^^^^^^^^^^ 1 file inspected, 5 offenses detected
設定したCopが外れていますね。
実際どのような設定項目があるかは、一番簡単な方法はRuboCopの
- config/disaled.yml
- config/enabled.yml
を見る事でしょう。
対応しているチェック種類
rubydocを見るのが速いでしょう。
Layout等の表記揺れから、Lintなどのバグの温床チェック、Security周り、Railsまで、かなり多岐に渡ります。
少し覗いてみると、
- Layout
- インデントなどのレイアウトに関するCop
- Lint
- 文法的に間違いでは無いが、紛らわしい書式に関するCop
- Metrics
- classやmethodの複雑度に関するCop
- Naming
- methodや定数、ファイル名などの命名に関するCop
- Performance
- 性能に関するCop
- Rails
- Rails特有の記述に関するCop
- Security
- セキュリティホールになりやすい記述に関するCop
- Style
- 細かめの記述方法に関するCop
- Bundler
- bundlerのGemfile等の書式に関するCop
- Gemspec
- Gemspecの記述に関するCop
などがあります。
触れてみた感触
上記でも述べたように、チェック項目がかなり多岐に渡っています。また、開発活発度もかなりのものです。
ただ、基本方針として、かなり固い方向に初期設定を振っているので、開発内で意見調整をしながら.rubocop.ymlを随時チューニングする必要があると感じました。
auto-correctによる自動修正は非常に強力で、開発者からcoding時のわずらわしさから相当開放し、レビュー時に、もっと本質を見る事が可能になりそうです。
多用はあまり推奨しませんが、前述のように規約を守ることよりも効率よく開発することを優先したい場合に使える、部分的にチェックを無効にする方法も、簡単に記載します。
# rubocop:disable Metrics/MethodLength, Metrics/AbcSize def foo ながーーーいmethod end # rubocop:enable Metrics/MethodLength, Metrics/AbcSize
rails_best_practices
導入
Gemfileの書き換え
group: testに
gem 'rails_best_practices'
を追加します。
SublimeText2やTextmate2との連動
筆者は生粋のvimmerの為、詳しくは調査していませんが、下記のように各editorと連携出来るようです。
databases.ymlの作成とGemの導入
rubocopと同様です。
production: &production adapter: sqlite3 database: db/redmine.sqlite3 development: <<: *production test: <<: *production
$ bundle install --path=vendor/bundle --binstubs
作動させてみる
以下いくつか抜粋。
$ bin/rails_best_practices . Source Code: |===================================================================================================================================================================| /Users/foobar/vcswork/redmine/app/models/enumeration.rb:21 - default_scope is evil /Users/foobar/vcswork/redmine/db/migrate/007_create_journals.rb:34 - isolate seed data /Users/foobar/vcswork/redmine/app/controllers/account_controller.rb:61 - move model logic into model (@token use_count > 4) /Users/foobar/vcswork/redmine/app/views/imports/settings.html.erb:9 - move code into helper (array_count >= 3) /Users/foobar/vcswork/redmine/app/views/wiki/rename.html.erb:14 - remove trailing whitespace Please go to https://rails-bestpractices.com to see more useful Rails Best Practices. Found 1316 warnings.
なにも設定しないと全部で1316件のwarningを検知したようです。これはrubocopよりは少ないと言えます。 が、move code into model や default_scope is evil など、railsに特化したメッセージが見受けられます。
メッセージの詳細や対処方法は、 rails-bestpractices.comを参考にとの事。
少し面白い機能として、 $ bin/rails_best_practices -f html . するとrails_best_practices_output.htmlが出力され、warning種別毎にリストを表示したりする事が可能になります。
warningに対応してみる
上記のwarningの大まかな理由は、
default_scope is evil: default scopeを使っている。isolate seed data: migrationにseedを書いている。move model logic into model: いわゆるfat controllermove code into helper: viewの複雑なcodeをhelperへ逃がせてない。remove trailing whitespace: 不要な行末スペースがある。
になります。各warningに関するページは warning名 rails-bestpractices で検索しましょう。
それではこれらに対応してみましょう。
default scope is evil
ActiveRecordのdefault scopeを使っています。これは要件によって対応方法が変わるので、画一的に対応は不可能です。
ただし、以下の理由により、利用する事は好ましくないと言えましょう。(詳細は上記のweb page参照の事)
- unscopeを使って解除しない限り、default scopeをoverrideする事が出来ない。
- modelのinitializationに影響を与えてしまう。
isolate seed data
migration内でrecordをイジるのは、recordの宣言箇所の一貫性の意味で好ましくありません。schema変更とseed dataの投入は分けましょう。
具体的にはrecordの挿入は db/seeeds.rb に任せましょう。
$ git diff --cached diff --git a/db/migrate/007_create_journals.rb b/db/migrate/007_create_journals.rb index 63bdd2374..6fa28c61c 100644 --- a/db/migrate/007_create_journals.rb +++ b/db/migrate/007_create_journals.rb @@ -27,13 +27,6 @@ class CreateJournals < ActiveRecord::Migration Permission.create :controller => "issues", :action => "history", :description => "label_history", :sort => 1006, :is_public => true, :mail_option => 0, :mail_enabled => 0 - # data migration - IssueHistory.all.each {|h| - j = Journal.new(:journalized => h.issue, :user_id => h.author_id, :notes => h.notes, :created_on => h.created_on) - j.details << JournalDetail.new(:property => 'attr', :prop_key => 'status_id', :value => h.status_id) - j.save - } - drop_table :issue_histories end diff --git a/db/seeds.rb b/db/seeds.rb new file mode 100644 index 000000000..f8f66c2f5 --- /dev/null +++ b/db/seeds.rb @@ -0,0 +1,8 @@ +# data migration +IssueHistory.all.each {|h| + j = Journal.new(:journalized => h.issue, :user_id => h.author_id, :notes => h.notes, :created_on => h.created_on) + j.details << JournalDetail.new(:property => 'attr', :prop_key => 'status_id', :value => h.status_id) + j.save +}
move model logic into model
本来modelに寄せるべきロジックがcontrollerに寄っています。いわゆるFat Controllerです。業務ロジックは業務ModelとしてModelに寄せましょう。
warningになっているapp/controllers/account_controller.rb:61のlost_passwordは下記に示しますが、実際長く、メッセージ通り、 @token が4回以上使われています。
ここのrefactoringには @token もですが、そこから取り出されている @user も込みで改修を考える必要があるでしょう。
def lost_password (redirect_to(home_url); return) unless Setting.lost_password? if prt = (params[:token] || session[:password_recovery_token]) @token = Token.find_token("recovery", prt.to_s) if @token.nil? || @token.expired? redirect_to home_url return end # redirect to remove the token query parameter from the URL and add it to the session if request.query_parameters[:token].present? session[:password_recovery_token] = @token.value redirect_to lost_password_url return end @user = @token.user unless @user && @user.active? redirect_to home_url return end if request.post? if @user.must_change_passwd? && @user.check_password?(params[:new_password]) flash.now[:error] = l(:notice_new_password_must_be_different) else @user.password, @user.password_confirmation = params[:new_password], params[:new_password_confirmation] @user.must_change_passwd = false if @user.save @token.destroy Mailer.password_updated(@user) flash[:notice] = l(:notice_account_password_updated) redirect_to signin_path return end end end render :template => "account/password_recovery" return else if request.post? email = params[:mail].to_s user = User.find_by_mail(email) # user not found unless user flash.now[:error] = l(:notice_account_unknown_email) return end unless user.active? handle_inactive_user(user, lost_password_path) return end # user cannot change its password unless user.change_password_allowed? flash.now[:error] = l(:notice_can_t_change_password) return end # create a new token for password recovery token = Token.new(:user => user, :action => "recovery") if token.save # Don't use the param to send the email recipent = user.mails.detect {|e| email.casecmp(e) == 0} || user.mail Mailer.lost_password(token, recipent).deliver flash[:notice] = l(:notice_account_lost_email_sent) redirect_to signin_path return end end end end
move code into helper
view層に対して複雑なロジックや大きめなコードがあります。viewの見通しが悪くなる為、helperに寄せましょう。
$ git diff diff --git a/app/helpers/imports_helper.rb b/app/helpers/imports_helper.rb index 39f3b95ab..934b1d262 100644 --- a/app/helpers/imports_helper.rb +++ b/app/helpers/imports_helper.rb @@ -44,4 +44,12 @@ module ImportsHelper [format, f] end end + + def formatting_for_options_separator(var) + options_for_select([[l(:label_comma_char), ','], [l(:label_semi_colon_char), ';']], var.settings['separator']) + end + + def formatting_for_options_wrapper(var) + options_for_select([[l(:label_quote_char), "'"], [l(:label_double_quote_char), '"']], var.settings['wrapper']) + end end diff --git a/app/views/imports/settings.html.erb b/app/views/imports/settings.html.erb index 374dba5b1..c00ffbcfb 100644 --- a/app/views/imports/settings.html.erb +++ b/app/views/imports/settings.html.erb @@ -5,13 +5,11 @@ <legend><%= l(:label_options) %></legend> <p> <label><%= l(:label_fields_separator) %></label> - <%= select_tag 'import_settings[separator]', - options_for_select([[l(:label_comma_char), ','], [l(:label_semi_colon_char), ';']], @import.settings['separator']) %> + <%= select_tag 'import_settings[separator]', formatting_for_options_separator(@import) %> </p> <p> <label><%= l(:label_fields_wrapper) %></label> - <%= select_tag 'import_settings[wrapper]', - options_for_select([[l(:label_quote_char), "'"], [l(:label_double_quote_char), '"']], @import.settings['wrapper']) %> + <%= select_tag 'import_settings[wrapper]', formatting_for_options_wrapper(@import) %> </p> <p> <label><%= l(:label_encoding) %></label>
remove trailing whitespace
不要な末尾のスペースが存在しています。
$ git diff diff --git a/app/views/wiki/rename.html.erb b/app/views/wiki/rename.html.erb index ac88fd4bf..602850d13 100644 --- a/app/views/wiki/rename.html.erb +++ b/app/views/wiki/rename.html.erb @@ -11,7 +11,7 @@ <p><%= f.text_field :title, :required => true, :size => 100 %></p> <p><%= f.check_box :redirect_existing_links %></p> <p><%= f.select :parent_id, - content_tag('option', '', :value => '') + + content_tag('option', '', :value => '') + wiki_page_options_for_select( @wiki.pages.includes(:parent).to_a - @page.self_and_descendants, @page.parent),
その他
列挙したのは極く一部のものです。railsを構成する、Model / View / Controller から細かいhelper、routingなど多岐に亘ります。
一度 https://rails-bestpractices.com/ を見てみるのも良いかもしれません。
設定ファイルを調整してみる
設定ファイルを作成する事により、解析内容をカスタマイズする事が可能です。
まず雛形を作りましょう。 $ bin/rails_best_practices -g を実行すると config/rails_best_practices.yml が生成されます。
AddModelVirtualAttributeCheck: { }
AlwaysAddDbIndexCheck: { }
#CheckSaveReturnValueCheck: { }
............
............
MoveCodeIntoHelperCheck: { array_count: 3 }
MoveCodeIntoModelCheck: { use_count: 2 }
MoveFinderToNamedScopeCheck: { }
MoveModelLogicIntoModelCheck: { use_count: 4 }
NeedlessDeepNestingCheck: { nested_count: 2 }
............
............
RemoveUnusedMethodsInControllersCheck: { except_methods: [] }
RemoveUnusedMethodsInHelpersCheck: { except_methods: [] }
RemoveUnusedMethodsInModelsCheck: { except_methods: [] }
ReplaceComplexCreationWithFactoryMethodCheck: { attribute_assignment_count: 2 }
............
UseScopeAccessCheck: { }
UseTurboSprocketsRails3Check: { }
不要なテストがある場合、#や行を消す事により、対象テストをキャンセルする事が可能となります。
また、defaultで入っている属性(オプション)の値を替えたり、配列に値を追加する事により、Checkのカスタマイズが可能となります。
たとえば
MoveModelLogicIntoModelCheck: { use_count: 4 }
の use_count を 5 にすれば、controller内でmodelのあるModelの利用回数が5回以上の場合にwarningになるようになります。
また、全てのCheckには ignore_files オプションを設定する事が可能で、
DefaultScopeIsEvilCheck: { ignored_files: 'user\.rb' }
LongLineCheck: { max_line_length: 80, ignored_files: ['db/migrate', 'config/initializers'] }
のように、1エントリか、配列を受ける事が可能になっています。
対応しているチェック種別
- Move code from Controller to Model
- Move finder to named_scope (rails2 only)
- Use model association
- Use scope access
- Add model virtual attribute
- Replace complex creation with factory method
- Move model logic into the Model
- Check return value of “save!”
- RESTful Conventions
- Overuse route customizations
- Needless deep nesting
- Not use default route
- Restrict auto-generated routes
- Model
- Keep finders on their own model (rails2 only)
- The law of demeter
- Use observer
- Use query attribute
- Remove unused methods in models
- Protect mass assignment
- Destroy return value (disabled by default)
- Mailer
- Use multipart/alternative as content_type of email
- Migration
- Isolating seed data
- Always add database index
- Use say with time in migrations
- Controller
- Use before_filter (disabled by default)
- Simplify render in controllers
- Remove unused methods in controllers
- Helper
- Remove empty helpers
- Remove unused methods in helpers
- View
- Move code into controller
- Move code into model
- Move code into helper
- Replace instance variable with local variable
- Simplify render in views
- Not use time_ago_in_words
- Deployment
- Dry bundler in Capistrano
- Speed up assets precompilation with turbo-sprockets-rails3
- Other
- Remove trailing whitespace
- Remove tab (disabled by default)
- Hash syntax (disabled by default)
- Use parentheses in method definition (disabled by default)
- Long line (disabled by default)
- Not rescue exception
触れてみた感触
Railsに関しては、MVCからmigration、helper、capistranoなど、ほとんどを含んでおり、かなりリッチな作りになっているように思われる。
ただし、 開発はあまり活発とは言えず、また、モトネタとなる Rails Bestpractices 自体も更新が止まっているようです。
とは言うものの、現状、提供されているCheckはRails5系でも有用に思われます。
結論
両方使え
筆者の個人的な感想を含んだ結論ですが、 RuboCop はrubyのcodeの静的解析を主に、rails_best_practices はその上に乗るrailsの静的解析を主に置いたツールと言えるでしょう。 ようするに 対象とするレイヤーが異なる のです。
双方のツールを導入し、それぞれのレイヤーのテストを重なる部分はあれど、補完しあえば良いかと考えます。
重要な点は、 解析ツールに合わせたコードを書くのでは無く、プロジェクトに必要とされるコードを素早く書く という事だと思います。ツールの結果に合わせてコードを曲げる(読みにくくしたり、リファクタリングの際にエンバグしたり)のは本末転倒と言えるでしょう。
これらの運用方法の検討に時間をかける余裕がない場合、 SideCI 等のcloud solutionを考えてみるのも良いかもしれません。SideCIではgithubやslackと連動させ、静的解析の結果をreviewer/revieweeへ通知し、reviewを行なう環境を簡単に得る事が出来ます。
