Sider Blog

コードレビュー自動化サービス「Sider」を運営するSider株式会社のコーポレートブログです。



RuboCop vs Rails Best Pratices それぞれの特徴。初心者はどう使う?

目次

開発をする際にテストを利用しする事は、今日最早あたりまえになったと思います。 また、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 以外にも mongoidmongomapper にも対応しています。 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の大まかな理由は、

  1. Stringをfrozen指定するCommentが入っていない(Style系Cop)
  2. hashのsyntaxがruby 1.9系の書式(Style系Cop)
  3. 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と連携出来るようです。

  • handlerを追加する事により、SublimeText2とシームレスに使えるようです。
  • tmbundleを追加する事により、Textmate2とシームレスに使えるようです。
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 modeldefault_scope is evil など、railsに特化したメッセージが見受けられます。

メッセージの詳細や対処方法は、 rails-bestpractices.comを参考にとの事。

少し面白い機能として、 $ bin/rails_best_practices -f html . するとrails_best_practices_output.htmlが出力され、warning種別毎にリストを表示したりする事が可能になります。

warningに対応してみる

上記のwarningの大まかな理由は、

  1. default_scope is evil : default scopeを使っている。
  2. isolate seed data : migrationにseedを書いている。
  3. move model logic into model : いわゆるfat controller
  4. move code into helper : viewの複雑なcodeをhelperへ逃がせてない。
  5. 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:61lost_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_count5 にすれば、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を行なう環境を簡単に得る事が出来ます。

リポジトリ数無制限の新しい料金プランの提供を開始。旧プランからの移行のお願い

こんにちは。SideCIの角です。いつもSideCIをご利用いただきありがとうございます。

SideCIは料金プランを2018年5月1日より新しい体系に改定いたします。新しい料金プランでは、

  • プライベートリポジトリ数ではなくユーザ数に応じた料金体系となり、開発チームの規模に応じて料金が変動する
  • アップグレード、ダウングレードの際に、利用料金の変更が日割り計算される

といった特徴がありますので、新プランへの移行をお願いします。(すでにご契約いただいているプランは当面の間そのままお使いいただけますのでご安心ください。)

料金プランの改定内容

旧プラン(2018年4月30日まで)

  • ユーザ数: 無制限
  • プライベートリポジトリ数: 解析が可能なプライベートリポジトリ数をプランに応じて制限
  • パブリックリポジトリ: 無制限
  • プラン: マイクロプラン3,200円/月(税込み)から提供

新プラン(2018年5月1日から)

  • ユーザ数: プライベートリポジトリの解析を利用したいユーザ数に合わせて購入が必要
  • プライベートリポジトリ数: 無制限
  • パブリックリポジトリ: 無制限
  • プラン: プライベートリポジトリへアクセスするユーザ1人につき1,500円/月(税込み)

料金プラン改定の背景

GitHubの料金プランは、2016年5月にリポジトリ数ベースからユーザ数ベースに変更されており、開発者の人数と比べたときに、多くのリポジトリを運用している開発チームが多数存在します。一方で、SideCIではリポジトリ数に応じて利用料金が決まる、リポジトリ数ベースの料金プランをこれまで採用してきました。この、利用料金の計算に対するギャップがSideCIをご利用いただく際の障壁の一つとなっていると私たちは考えており、今回の料金プラン変更へとつながりました。

また、SideCIの提供において必要となる計算機リソースは、Pull RequestのOpenされる数や更新される数と比例します。これは、その開発チームの持つリポジトリの数ではなく、所属している開発者の数と強く関連しています。そのため、リポジトリ数ではなく、それにアクセスするユーザ数に応じた料金プランがSideCIのサービス提供に対して適正であると考えています。

新プランへの移行のお願い

旧プランでSideCIをご利用いただいているみなさまにも、新プランへの移行をお願いします。

  • 新しい料金プランでは、プライベートリポジトリを無制限に登録可能ですので、今までSideCIを使えていなかったリポジトリでも、SideCIを気軽に導入いただけるようになります
  • GitHub上のOrganizationへの登録ユーザ数ではなく、実際にSideCIを利用するユーザのみを課金対象としています

こうしたメリットがありますので、お手数ではございますが、現在ご契約いただいているお客様も、新プランに移行いただけましたら幸いです。

その他の新機能

また、SideCIでは5月1日より、次の機能の提供を開始します。

  • SideCIのWebサイトから領収書を確認、印刷できるようになります
  • 料金プラン変更時(ユーザの増減時)に日割り計算を行ないます

従来、請求書及び領収書はメールで送信されておりました。このメールの宛先変更や、再送付はお問い合わせいただく事が必要でしたが、今後は上記のように、Web UIから簡単に確認が行えるようになります。

新料金プランへの移行について

現在、SideCIで有料プランをご利用いただいているお客様については、当面は旧プランでのご利用を継続していただけます。5月1日以降は、旧プランへの変更および旧プランの間でのアップグレード、ダウングレードや新規の利用開始ができなくなりますので、ご了承ください。

新プランに移行される際には、SideCIのWebサイトから変更が可能です。(これまでPayPalを利用して利用料金のお支払いをされていた場合には、改めてクレジットカードの登録が必要になりますので、ご了承ください。)

画面上での操作について

次のような手順でプランを変更いただけます。(5月1日以降に以下のような画面が適用されます。)

オーガニゼーション設定ページに遷移していただくと、契約中のプランが次のように表示されます。 f:id:sideci:20180425174248p:plain

上記の「Upgrade」ボタンを押して、プラン変更を進めて下さい。クレジットカードが未登録の場合にはクレジットカード登録画面が現れます。

なお、領収書の確認、印刷などは次のような画面で行う事ができます。

f:id:sideci:20180425174843p:plain Invoice 一覧画面

f:id:sideci:20180425174858p:plain Invoice

よくある質問

旧プランを新しく契約することはできますか?

5月1日以降、旧プランを契約することはできなくなります。ご了承ください。

旧プランを現在契約中です。このプランはいつまで利用可能ですか?

現在(2018-04-25)のところ、旧プランの廃止時期は決まっておりません。
旧プランの廃止の際には、その3ヶ月前を目処に告知いたします。

旧プラン体系での契約の変更または新規の契約はできますか?

5月1日以降、旧プランでの契約の変更や新しく契約することはできません。新プランへの移行をお願いします。

ユーザ数はどうやって計算されますか?

まず、SideCIのプラン設定画面で、お客様に必要なユーザ数をご指定いただきます。
お客様の指定したユーザ数を上限として、SideCIの設定画面から、プライベートリポジトリへのアクセスを許可したいユーザに対して権限を付与できます。
プライベートレポジトリへアクセスできるSideCIユーザの数は、お客様によるSideCI上での操作以外によって、変更されることはありません。つまり、新しくユーザがサインアップしたとしても、SideCIの利用料金は自動では変更されません。

ユーザ数の増減はどう処理されますか?

日割りの計算が行われます。月の途中でいつでもユーザ数を増やすことや減らすことが可能です。

RuboCopコミッタ、Pockeが語るBatsov像とアドバイス -SideCI技術顧問就任記念インタビュー

SideCIでは、2018年4月より、RuboCopコミッタのPockeこと、桑原 仁雄氏を技術顧問に迎えることになりました!Pocke氏は、2015年9月にSideCIに入社して以来、SideCIが提供する解析器の拡充などに携わってきました。この4月よりクックパッド株式会社に転職し、同時にSideCIの技術顧問に就任しました。SideCIではこれを記念し、Pocke氏にインタビューを行いました。和やかな雰囲気の中、RuboCopから、Batsov氏のこと、そして未来のコミッタたちへのアドバイスなどを、ざっくばらんに話してくれました。ぜひご覧ください!

目次

f:id:sideci:20180308172024j:plain

RuboCop自体の話

- まずは、基本的な質問から始めましょう。RuboCopとは何でしょう?

RuboCopは一番括りの狭い言葉でいうとRubyの静的解析器なんです。それは何故かというと、静的解析器の中にスタイルチェッカーとLinterあとその他諸々がいっしょくたになったものだからです。その他諸々って言うのは、例えばメトリクスを測るようなものとかです。 特徴的なところでは、RuboCopのReadmeの一番最初に映画のロボコップのセリフが引用されているんです。「Role models are important.」

f:id:sideci:20180417115626p:plain 出典:http://rubocop.readthedocs.io/en/latest/

確かRuby Style Guideの方にも同じセリフが引用されているはずです。

RuboCopに関わり始めてからコミッタになるまで

-どういう経緯でRuboCopに関わるようになったんでしたっけ?

2015年の秋にSideCIに入ってからRuboCopに関わるようになりました。2016年に初めてプルリクを送ったんです。最初は割とどうでもいいプルリクを投げていたんですけど、そのうちRuboCopって結構バグがあって面白いって気がついて、ひたすらバグを直していた気がします。

- RuboCopでは主にどういう仕事をしていますか?

地味にバグを直していますね。

f:id:sideci:20180417115921p:plain Fix Style/RedundantReturn cop for empty if body #3607”(https://github.com/bbatsov/rubocop/pull/3607)

例えば、このPRでは、Rubyではifの中身が空のことがあるんですけど、そうするとRuboCopがクラッシュするっていう問題があって、それを直すものです。

他には、ちょっと前にRSpecをtest-queueで速くするっていうのをやりました。 https://github.com/bbatsov/rubocop/pull/4544

当時CIがめちゃくちゃ遅くて辛かったんですけど、結構速くできたので良かったですね。大体2〜3倍くらいは速くなりました。

-コミッタになったときの経緯とかは、なにかきっかけがあるんですか?

特にきっかけは無かったと思います。ある日突然Invitationが届いていたみたいな感じで。READMEのヒストリでPockeと検索すると出てくるのが、8ヶ月前ですね。 f:id:sideci:20180417120641p:plain なにか特別なことをやっていたわけでもありませんが、でも一番プルリクを送っていた時期だとは思います。

- コミッタになる前となった後で関わりかたになにか変わったところはありますか?

あまりないです。外に向けてコミッタやっているってことを言えるようになったっというのと、あとはIssueにラベルをつけられるようになったのが一番の違いです。

前に「5個ぐらい同じIssueが立っている」みたいなことがあって、それは辛かったですね。もちろん、一瞬でCloseできるので大きな問題ではないのですが。コミッタになったことで、Closeとかラベル付けとかできるようになってしまって、自分の問題になってしまった。ちょっと検索してみて欲しいって気持ちはあります。

Batsovの話

-Batsovはどんな人ですか?

ヨーロッパ圏でVPoEやってる方で、確か人材系の会社だった気がします。会ったことはないんですが、去年はEuRukoでトークしてましたね。今は積極的に自分でRuboCopのコードを書くということはしていなくて、でも別にRuboCopに興味を失ったということでもないみたいでレビューはがんがん来ます。プルリク等を見ていても反応は早い方で、帰ってこないときは旅行中とかそのような時だけ。そうでないときはこまめに返ってきてます。 全体的にマメで親切な人だという印象はありますね。レビューしてても、なにか言ったら、同じことを同じコードについて全部繰り返して書くとか。忘れなくて良いです。プルリクもほとんどマージしていて、そういう意味でも、寛容というか、全て受け入れてるなって気がしますね。大きすぎるとか、説明がないとか、そういうPRはどうしようもないので、pingを打って返ってこなかったらCloseするとかしますけど。

-RuboCopの名前とかさっきのREADMEの引用とか、ロボコップからの借用が多いような思いますが、誰の趣味なんでしょう?

Batsovの趣味だと思います。さっきの「Roll models are important.」はRubyStyleGuideにもありますし。あれもBatsovがやった仕事です。 面白かったのだと、RuboCopのルールのグループが分かれていますが、例えばStyleとかLintとか、あのグループのことを昔はcop_typeと言っていたんですけど、ある日突然departmentに変わったんですよ。 <<PR: Replace cop_type with department https://github.com/bbatsov/rubocop/pull/3842>>

これは、コップ(警察官)の部署という意味らしいんです。コミッショナー(所長)の下にteamがいたり、警察関連の言葉が用いられているんです。他にも、forceとかoffenceとか。forceってプログラミング用語とは関係がなくて、変数のトレースとかするものをさしています。構造を覚えるのが大変ですね。 (出典:2016/12/30 https://github.com/bbatsov/rubocop/pull/3837#issuecomment-269750599  この中でBatsovは” And I think we've strayed from our fun police references! cop_type? A cop should have a department, not a type! :-)”と述べている。

RuboCopの開発に参加するには

-RuboCop自体のコードレビューというのはどんな形で運用されているのですか?実際にレビューする人される人って側でのコメントを聞かせてください。

特に決められたレビュアーとかはいなくて、誰かが見るみたいな運用をしています。ただ、マージは基本的にBatsovがすることにはなっています。で、結構コミッタじゃない人もレビューしていて、活発な感じで動いています。

あんまり厳格なルールはないのですが、PRのテンプレートがリポジトリに入っているので、できるだけそれに沿って欲しいというところぐらいですね。Issueを上げてからPRを作る、みたいな作法のプロジェクトもありますが、RuboCopはそうではない。テストは追加して欲しいですけど。

レビューで気にするところは結構人によって異なっていて、結構コーディングスタイル的なところを突っ込む人もいます。名前とか英語とかも見られますね。私も、最初のころは全然英語ができなくって、Offenceのメッセージに「その英語おかしいよね」とか言われるのは結構ありました。

-新人コントリビュータへのアドバイスみたいなものはありますか?

私は、正しいRubyのコードで走らせた時にクラッシュしないかどうか、割と重視してみています。なので、一回作ったものをなるべく多くのコードで試した方がいいっていうのがアドバイスですね。私がきちんとやるときには、ruby/spec(github.com/ruby/spec)や、GitLab(github.com/gitlabhq/gitlabhq)とかの大きなRubyのプロジェクトで試すようにしていて、クラッシュしないかとある程度正しいオフェンスが出てるかを見るようにしています。

-「こういうのが手をつけやすそう」みたいなものはありますか?

RuboCopって、デフォルトの設定じゃないと簡単にクラッシュするので、ちょっと変わった設定をすると意外と簡単に問題が見つかります。RuboCop自体はRuboCopでチェックしてるんですが、ほぼデフォルトの設定になっていて、だからデフォルトの設定だとさすがになかなか問題は見つからない。

あとは、さっきのruby/specで、そこには結構面白いコードがたくさん入っているので、RuboCopにそこのコードを読ませると簡単にRuboCopが壊れます。

他にはこういうのがあります(#4910)。

f:id:sideci:20180417121108p:plain https://github.com/bbatsov/rubocop/issues/4910

これは「Documentationに例が欲しい」Issueを立てたら、結構みんなやってくれて。コメント足すだけなんで、簡単にできるといえば出来るんですけど。これがないと、テストケースを見ないと、そのコップが何をするかわからないという状況だったので。これなんか、手が付けやすかったんじゃないかと思います。

Issueはもうずっとつまっていて、まあ300件ぐらいオープンなものがあるんですけど、割とずっと積もっているままになっているのが多いです。なので丁寧に見ていけば、なにかやりやすいものがあるかもしれませんね。

- RuboCopの開発チームでこういうことをやっていきたいみたいなテーマはあったりしますか?

最近ですと「bump to 1.0」っていうIssueがあって、これ本当にスタートさせるのかな、と思ったりしています。

f:id:sideci:20180417121430p:plain bump to 1.0 のスクリーンショット

今は0.53.0なんですが、Batsovのコメントによると、まだ1.0にはしないって言ったりとかしている。これも品質が低いとしか言っていないのでなにをやっていきたいのかはっきりとはわからないんですが、まだまだ壊したいんじゃないですかね。そろそろ1.0を出して、壊れたら2.0を出すようにするとかしてもらいたいんですけど……

個人的に今やりたいのはオートコレクトですね。「ruby/specを入力にして、RuboCopでオートコレクトして、その結果がテストを通るかどうか」っていうのをやってみたいですね。現状はruby/specを入力しても全然ちゃんと動かないので。

後はちょっと使いにくいところがあって、「ソースコードの一部だけを選択的にオートコレクトしたい」みたいな要望があるんですけど、そういうのをサポートしたいですね。

- RuboCopの開発者って多分日本にも何人かいると思うんですけど、その人たちと何かオフラインで会って話したりしますか?

割と会うことがあります。最近プルリク投げているkoicさん (github.com/koic)、結構よくあっていて、RuboCopの話をすることがあります。懇親会とかが多いんですが、去年のRubyKaigiの時は、こういうの作ろうと思ってるんだけど、どう?みたいな相談をされて、一緒に話しながらプルリクを見ていったことがありました。

RuboCop利用者へ

-どういう風にRuboCopを使ってもらいたいみたいとか、こうやって風に使ってみたらいいよみたいとか、アドバイスはありますか?

RuboCopに期待しすぎないで欲しいっていうのはあります。銀の弾丸だと思っている人がいて。で、とりあえず入れたらコードがよくなるって思っている人がいると思うんですけど、そうではなくて、RuboCopにどうさせたいか、っていう目的がないと、ちゃんと使いづらいっていうのがあると思います。

例えば、RuboCopでコーディングスタイルを統一したいという目的があるんだったら、それ用のコーディングスタイルを作るとこから始めるか、RuboCopのコーディングをそのまま受け入れるみたいな選択が必要になります。なので、入れただけだと、警告が大量に出て失敗するっていう状況だけになるかなと思います。

RuboCopを使って何をやりたいのかというのをまず考えて、その上でちゃんと設定しないといけない。

-具体的には、それぞれどうやって設定を詰めて行ったらいいのでしょうか?

WEB+DB PRESSに「Rubyドキドキ探検隊」という連載を2017年からしているんですが、Vol.102で解説を書いたので、読んでいただければと(笑)。

ざっくりというなら、まずスタイルの方は、ひたすら頑張って直していくしかありません。.rubocop_todo.yml (http://rubocop.readthedocs.io/en/stable/configuration/#automatically-generated-configuration) っていう仕組みがあるので、それを使えば良いと思います。 これは、一回、今のソースコードで出る警告をToDoとして書き出した上で、警告は出ないようにするという仕組みで、まずはじめに出た警告を一旦見なかったことにして、少しずつ自分たちのルールを作って行きながら、適用をしていくということができる。結局その作業をしなければいけないのでいけないので、どこかで自力で頑張らなきゃいけないということになります。

バグを見つけるというのはもっと簡単で、単にDisabledByDefaultってオプションがあるので、一旦全部バッて無効にしてバグを見つける系のLint系のルールだけを有効に設定するのがいいです。 ruby/specの設定がちょうどこの使い方なので、参考になるかもしれません。

- 逆にRuboCop利用者にお願いしたい事はありますか?重複してバグを報告しない、以外で(笑)

バグは出来れば報告してほしいですね。日本人向けのことを言えば、rubocop-jp(github.com/rubocop-jp/issues)というorganizationを作ったので、そちらにバグを報告していただいても良いです。 これはvim-jp(github.com/vim-jp)を参考にしていて、rubocop-jpに上がってきたものを英語化して本家に投げるところまでできれば、理想的なんじゃないかと思います。

私がそもそも英語を書きたくないっていう思いがあって、でも英語でやらないと本家のIssueにあげられないじゃないですか。で、そうすると自分の中だけに溜まっていくIssueっていうのが大量に増えてきて、それを管理するのが嫌になってきたので、rubocop-jpを作ったというのも側面にあります。

SideCIについて

-SideCIでRuboCopを使うと便利だと思うんですが、その辺りを少し話していただけますか?

現状で言えば、割とLint系ってどうしてもfalse positiveが出るものだと思うんですけど、SideCIで動かしていると、そういうものでも割と気軽に有効にしていけるっていうのは、あると思います。

SideCIがない場合には、ローカルで動かすかCIの中で実行するかしかないんですが、ローカルでいちいち実行するのは面倒なのでCIでやろうかっていう話になると思います。ところが、それだと1か0かみたいな所があって、RuboCopがコケるかコケないかの二つの状態しか持てない。これは、不便なことが結構あるんじゃないかなと思います。SideCIがあると、RuboCopが出した指摘をCloseするかしないか選択できるので、「この警告は許容しておく」っていう状態が出来るので柔軟性が高くて、使いやすい状態になると思います。

(編集者注:SideCIではRuboCopなどのLINTツールに検出された問題を修正するのか、それとも今回は対応せずに見送るのかを、個々に設定することができます。実際に検出された問題を確認しながら、それぞれを修正するかしないか決められるので、より積極的にツールの検査を有効にすることができます。)

あとは今は実装できていない未来の話をすると、オートコレクトももっと上手くできると思っていて、自動で修正したいんだけどどう修正したら良いのかは自明ではなかったりするときに、インタラクティブに修正していけるようなUIがあると良いのかなと思います。

f:id:sideci:20180308172951j:plain

インタビューの最後には、SideCIのCTOの松本とPocke氏で記念撮影をしました。 Pockeさん、これからもどうぞよろしくお願いします!

Reactの開発チーム内でのJavaScriptの静的解析器ESLintの使われ方、設定、独自プラグイン

目次

f:id:sideci:20180411110620p:plain ESLintの公式ロゴ。引用元https://eslint.org/docs/about/

静的解析、いわゆるLintと呼ばれるツールを使用されたことはあるでしょうか?

設定に基づきソースコードを解析して、チームのコーディング規約から外れた記述があれば、警告やエラーを出力してくれ、場合によっては自動的に修正もしてくれる頼れるツールです。

言語により様々なLintが開発・提供されているのですが、それらを使用する際このような疑問・課題を感じたことは無いでしょうか?

  • 初めてこの言語を使用するのだが、この言語の一般的なコーディングスタイルはどんなのだろう?
  • とりあえずデフォルトのLintの設定を使っているがどうにもチェックが厳しすぎる。設定を変えたいのだが、この設定を変えることでバグを見逃したりしないだろうか。

一つの言語でも様々なコーディング規約が提案されており、特に初めての言語を使用するときなど、どのコーディング規約に沿って良いのか判断が難しい場合があるかと思います。

そこで、有名なOSSで使用されているLintの設定を調査して、どのような設定やコーディング規約が使われているのか見てみましょう。

JavaScriptでよく使われているLintであるESLintを対象として、JavaScriptを使用しているOSSで使用されているコーディング規約を見ていきたいと思います。

今回はReactに焦点を当てて使用されているESLintのルールを見ていきます。

ESLintの概要と特徴

ESLintはJavaScriptのための静的解析ツールです。

ESLintは特定のコーディング規約に依存せず、個別のコーディング規約はユーザーは自由に有効無効が設定可能です。 このように、カスタマイズ性が非常に高いのが特徴です。

豊富なデフォルトで定義されているルールと呼ばれるコーディング規約や、ユーザー独自で定義できるルールを駆使して、柔軟にコーディング規約を設定することができます。

また、サードパーティなどが公開している「Google JavaScript Style Guide」や「Airbnb JavaScript Style Guide」等、有名なコーディング規約も設定一つで再利用が可能です。

さらに、それらのコーディング規約に従いつつも、特定のルールを特定のファイルのみ有効/無効にする、などと言った設定も簡単に行うことができます。

全くどのような設定にすればわからないという場合は、最初にESLint公式が提供しているGetting Startedを参考にして設定すると、ESLintが推奨するコーディング規約を設定することできます。

ESLintのルールの見方

ESLintのコーディングルールの設定は非常に簡単です。 簡単な例として、ステートメントの末尾にセミコロンを付けるというルールを設定するconfigを定義してみましょう。

{
    "rules": {
        "semi": [2, "always"]
    }
}

個別のコーディングルールは、設定ファイル内のrules内で定義することが可能です。 今回の場合はsemiと呼ばれるルールを設定しており、ESLintがデフォルトで提供しています。 ESLintでは2をERROR, 1をWARN, 0をルールのOFFと定義されています。 今回の例の場合は、ステートメントにセミコロンがついていないとlint実行時にERRORになるように設定しています。

また、ESLintは他にも大量のコーディングルール群を提供しており、それらを利用してルールのカスタマイズをする事ができます。

サードパーティが提供している設定の利用

有名なコーディング規約は、既にパッケージとして提供されている場合あります。 例えば、先述したGoogle JavaScript style guideのルールを適用したい場合は、npmに公開されているeslint-config-googleというパッケージをインストールすることで使用できます。

{
  "extends": "google",
}

と記述すること、でユーザーは、一から設定せずともGoogle JavaScript style guideのコーディング規約の設定を利用できます。

このルール群は、ESLint自体のデフォルトで提供されている設定もありますが、上記で紹介したeslint-config-googleように、npm moduleとしてサードパーティが提供しているベース設定を利用することができます。 また自身で開発して外部に公開したり、複数のプロジェクトで使いまわすこともできます。 このルールの設定群はsharable configといいます。

「基本的にGoogle JavaScript style guideに従いたいが、どうしてもステートメントの最後にセミコロンは付けたくない」という場合(Google JavaScript styleはセミコロンをつける設定を推奨しています)は、以下のようにrulessemiの設定をすれば簡単に実現が可能です。

{
  "extends": "google",
  "rules": {
    // Additional, per-project rules...
    "semi": [2, "never"]
  }
}

Plugin

ESLintが提供しているデフォルトのルールは、基本的なルールは網羅していますが、JavaScriptが使用できる範囲は非常に広いです。 そのため、デフォルトのルールの中には、自分が使用したいルールが存在しない場合があるかもしれません。 そのような場合、ESLintではルール自体を独自に開発することができます。 ESLintは独自のルールを第三者が開発できるようにするため、Pluginという仕組みを提供しています。 npmでeslint-pluing-*と検索すると、サードパーティーが提供するカスタムプラグインが大量にあるのが確認できます。

もし、ESLintが提供しているデフォルトのルールの中に、使用したいルールが存在しない場合プラグインの方も見てみると良いでしょう。

使い方もsharable configと同様に簡単に設定できます。 例えばReactのコードを静的解析をしたい場合、eslint-plugin-reactというプラグインをインストールし、下記のように設定することでReact独自の構文の静的解析をすることができます。

{
  "extends": "google",
  "plugins": [
    "react"
  ],
  "rules": {
    "semi": [2, "never"],
    "react/display-name": 2
  }
}

上記のルールを設定することで、React component内でdisplayNameという変数の宣言忘れを防ぐ事ができます。 このようにESLintは、一つ一つのルールについても簡単に設定・再利用することが可能です。

ここまで、ESLintはカスタマイズ性が非常に高く、様々な設定が可能であることを説明しました。 しかし、ユーザーとして気になるのはどんなルールを適用すればよいかいうことではないでしょうか。

「うちのプロダクトではこんなコーディング規約で開発しているけど他のプロジェクトではどんな規約を使ってるのだろう?」などの疑問を感じたことはありませんか?

タイトルにあるように今回の記事では、有名なフロントエンドフレームワークの一つであるReactで使用している静的解析ルールを調査して、どのようなコーディング規約を設定しているのか見ていきましょう。

React

概要

 ReactとはFacebook社が主導で開発を行っているフロントエンドライブラリです。

公式のドキュメントにはReactのキーワードとして以下の3つのキーワードが紹介されています。

Declarative (宣言的) 宣言的プログラミングが可能。

Component-Based (コンポーネントベース) コンポーネントの考え方により複雑なUIの構成を簡単に実現可能。

Learn Once, Write Anywhere(一度学べば、どこでも書くことができる) Reactで使用した技術はWebフロントエンドだけでなく、React Nativeのようなモバイルアプリにも使用可能。

という特徴があります。

ESLint in React

React内で定義されている、大枠のeslintの設定ファイルはこちらになります。

fbjs

上記設定ファイルの初めの方を見てましょう。 fbjsをextendsしていることがわかります。

module.exports =  {
  extends: 'fbjs',
}

Reactでは、eslint-config-fbjsというsharble configをベースルールとして使用しています。

fbjsは、Facebook社が内部的に使用していたESLintの設定をsharable configとしてまとめたものです。 その特徴として

  1. project毎に特殊なケースを設定しない
  2. Facebook独自のルールは設定しない

とあります。

ですが、実は二番目の特徴については、Facebookの内部の設定のためにいくつか独自のルールが設定されています。

ここに設定されているextendConfigによって、若干Facebook用の設定がされているようです。

Plugin in fbjs

次にfbjs自体に設定されているルールを見ていきましょう。

fbjsのESLintの設定ファイルでは、下記のように使用するプラグインが指定されています。

plugins: [
  'babel',
  'flowtype',
  'jsx-a11y',
  'react',
  'relay',
],

それぞれについて簡単に解説します。

  • babel
    • eslint-plugin-babelBabelのためのルールセットです。 Babelは、ECMAScript2015/2016/2017やflowtypeで書かれたコードを、一般的なブラウザがサポートしている形式のJavaScriptに出力することができるトランスパイラです。
  • flowtype

    FlowはJavaScriptで静的型付けを可能にするツールです。静的型付けのチェックはFlow側に任して、ESLintではすべてのjsファイルに対して、@flowアノテーションがつき、Flowで型をチェックをしているかをチェックしています。

  • jsx-a11y
    • jsx-a11yは、react-a11yというReactの潜在的なアクセシビリティの問題についてのルールが定義されているプラグインです。
  • react
  • relay
    • relay-pluginは、RelayとよばれるJavaScriptフレームワークを静的解析するためのプラグインです。

さて、この唐突に出てきたfbjsというリポジトリですが、Facebook社開発のプロダクトチームが素早く、安心してコードを書くためのFacebook社のためのライブラリ、モジュールという位置づけのようです。 ESLintのshareable config以外にも様々なスクリプトがありますが、本番環境のコードに適用すると、思わぬ問題を引き起こすかもしれませんので注意してください。

また、同リポジトリにおいて、よりモダンなコードベースに対応するためのeslint-config-fbjs-opensourceが開発されており、今後こちらのshareable configをベースとした開発がメインになるかもしれません。

ReactのESLint

上記で説明したfbjsをルールのベースとして、それ以外に記載されたルールがReactの独自のルールになります。

Plugin in React

ReactのESLintの設定ファイルでは更にpluginが追加されています。

plugins: [
    'jest',
    'no-for-of-loops',
    'react',
    'react-internal',
  ],

それぞれについて簡単に解説します。

  • Jest

JestはJavaScriptのためのテストフレームワークです。 もちろんFacebook謹製。 "zero-configuration"を哲学として掲げており、create-react-appreact-native initでプロジェクトを作っていると、既に適切な設定がその時点でされるようになっています。 特にReactやReact Nativeを使用している場合、非常に導入しやすいテストフレームワークです。

このルールは'**/__tests__/**.js'のファイルパスのみ適用されています。

  • no-for-of-loops

no-for-of-loopsfor (...of)を抑制するルールです。

  • react-internal

npmにプラグインとしてはあげられていませんが、react内で独自に定義されているカスタムプラグインです。

エラーメッセージのフォーマットに関するルールや、Boolean,String, Numberのプリミティブ型のコンストラクタを抑制するルールなどが追加されています。

その他の基本的なルールについて

ここまで、fbjsとReactが使用しているESLintのpluginについて一通り見てきました。

他にも、ESLintが提供しているデフォルトのルールによって、静的解析の設定がされています。 ルールの概要を説明すると下記の様なルールが設定されています。

  • インデントの指定なし
  • セミコロンをつけることを推奨
  • ブロックや関数の後ろ、キーワードの前後にはスペースを入れる
  • 比較演算子には==!=よりも===!==を使用する
  • ファイル最後は改行で終わる
  • 文字列は基本的にシングルクオートだが、JSXの場合はダブルクオートを使用する
  • 使用しない変数の宣言は許可しない

一つ一つ解説していきたい所ではあるのですが、なんとその数約140個ものルールが細やかに設定されています。 そのため、今回ざっと特徴だけの説明に留めておきたいと思います。

静的解析の対象となるソースコード

この設定ファイルを元に、Reactでは3ケースに分けて静的解析が実施されています。

eslintrc.esnext.js

ここで、上記で設定したルールの他に、varの使用を許可しないというルールを追加しています。

大部分のファイルが、このルールで静的解析されています。

具体的な適用範囲は、Reactのリポジトリからみて、packages/*/*.js,packages/*/src/**/*.js,packages/events/**/*.js,packages/shared/**/*.js,scripts/flow/*.js, scripts/rollup/shims/**/*.jsです。

eslintrc.es5.js

ecmaScript5で開発されたコードは、このルールで静的解析されます。

ルールとして追加されているのは、use strictを強制している点と、parserがbabelからespreeに変更されている点です。

適用範囲はpackages/*/npm/**/*.jsです。

eslintrc.default.js

ECMAScript2017に対応しているコードに適用させるルールです。 前述したeslintrcに加えて、以下の3点がルールとして追加されています。

  • varの使用を許可しない。
  • strictを有効にする。
  • ECMAScript6から導入されているRest PropertiesSpread Propertiesのサポートを有効。

また、eslintrc.es5.jsと同様に、parserがbabelからespreeに変更されています。 適用範囲はeslintrc.esnext, eslint.es5.jsが適用されているファイル以外のJavaScriptのコードが対象となっています。

Reactで使用されているESLintの使い方まとめ

以上が現在Reactに設定されているESLintのルールになります。

ここまでの話をまとめると

  • 基本的にはFacebook社内で使われていたルールをOSS化した(fbjs)が主。これはfacebookが作っているJSライブラリで広く使われている様子。但し、その他の開発チームが使う事を前提としたものではない。

  • Reactでは、上記に加えて、Jestなどのいくつかのプラグインが使われている。

  • ecmaScriptのバージョンが混在しているが、ちゃんとバージョンごとの静的解析ルールを作成してすべて静的解析を行っている。

無ければ独自でルールを開発したり、使用しているフレームワーク毎にプラグインを開発していたりで非常に多くのルールが設定されていました。 この多岐に渡るルールを自動化することで、JavaScriptのようなインタプリタ型言語による大規模開発において、バグを事前に潰すことに一役買っているのだろうと感じました。

おわりに

SideCIではESLintを用いたJavaScriptプロジェクトのコードレビューに対応しています。 SideCIを利用して、GitHub上でPullRequestと連携した自動レビューをプロジェクトメンバー全体で共通して使うと、コードレビューが非常に便利になります。

是非ご活用ください。

「SideCIのようなツールが開発プロセス中に存在するのは、ぼくが理想とする世界の一部を実現している」(まつもとゆきひろ氏)

Ruby25周年を記念して学生時代からのRubyファンを公言するSideCI株式会社代表 角 幸一郎がRubyのパパである、まつもとゆきひろ氏にインタビューを行いました。

角 : Ruby25周年おめでとうございます。これまでRubyを開発していて嬉しかったことは、どんな時ですか?

f:id:sideci:20180302130652p:plain (左 : SideCI株式会社代表 角 幸一郎 右 : Ruby開発者 まつもとゆきひろ)

まつもと : Rubyユーザーの皆様からこんないいことがありました!という報告が来るのは嬉しいですね。例えば、プログラミングを楽しくないなと思ってた学生がRubyを使うことによって楽しくできるようになったとか、Rubyを使っていい仕事をやり遂げたとか、給料があがりましたなど、そう言った話を結構聞きます。いい事柄ばかりでRubyの広告みたいになってしまいましたが、こういったプラスの現象を実現できているのは素晴らしいですよね。

角 : それは間違いないですね。私もRubyがなかったら卒業論文を無事に書き終えることができた気がしません。学生時代からRubyには、随分お世話になっていますし、SideCIのプロダクトも当初はRailsのコードをターゲットにして作られたものでした。

Rubyで大きな節目となった時期は?

角 : Rubyが広く普及していく過程で大きな節目になったのは、まつもとさんはいつだと考えますか。

まつもと : 1999年に私と石塚圭樹氏で書いたRubyの本が日本語で出て、2000年にはDave Thomas, Andy Huntが書いた『Programming Ruby』が出版されました。翌年2001年には初めてとなるRubyConfが開催されています。21世紀に入った頃が1つの大きな節目だと言えますね。

角 : Rails 1.0が誕生した頃ですか?

まつもと : Railsはまだまだ先の話で、2004年7月にRails 0.5.0がリリースされています。私の記憶によれば、2001、2003年とデンマークで開催されたJAOO Conferenceというイベントに呼ばれて出かけに行きました。2001年の時にDave Thomasと私、さらに学生ボランティアとして参加していて当時はPHPプログラマーだったDavid Heinemeier Hansson(DHH)の3人で言語について何か話をしたような気がします。

角 : おおっ。すごい。それがRailsにつながった。

まつもと : そうかもしれませんね。その後、DHHは37signals(現Basecamp)の仕事をリモートでやっていた時に、Rubyで書いたWeb用のフレームワークをリリースしますが、それがRailsだったと。

角 : なるほど。

まつもと : 2004年くらいまでは、Rubyなんて聞いたことない、もしくは聞いたことあるけど使ったことないとか、自分達と違うどこかの世界で頑張ってるんだね。これからも頑張ってよ。という感じで、どこに行っても他人事みたいな感じの対応が多かったですけど、2005年くらいからはRuby使ってますというような人も増えてきて、個人的にはちゃんと使ってもらえてるということを実感できて嬉しかったですね。

角 : そこからは、皆さんが知るように誰もが知るプログラミング言語になっていったんですね。私がRubyを使い始めた2009年にはもう既に一般的な言語という感じでした。

まつもと : そうですね。Rubyがテクノロジー的にホットという意味では大体ピークは2009年くらいだった気がします。いまはRubyはプログラミング言語として、ごくごく当たり前な存在になっていますよね。ただ、この当たり前の存在というのは怖くて、いまは当たり前なんだけど、新たな技術の登場で将来的にはフェードアウトしてしまうかもしれない。いまRubyのコア開発チームで努力してるのは、次の25年のために、技術に追従しながらどのように新しいRubyを提供していくかということです。昔よくつかっていた、と言われるのではなくて、昔からあるけど今も使っているよという存在にしたいと思いますね。

f:id:sideci:20180302130642p:plain

コーディング規約は不要? まつもと氏が考える「基準」とは

角 : ここからは弊社のプロダクトであるSideCIと少し関係がある内容を質問させていただきたいと思います。まつもとさんのコーディング規約に関する個人的な考え方を教えていただけませんか?

まつもと : あまり私自身はコーディング規約を気にするタイプではないですね。でもコーディング規約にこだわる人はいっぱいいますよね。

角 : いっぱいいますね。

まつもと : コーディング規約を決めてくれないと仕事できないよ!っていうような人もいて、「君、本当に仕事してる?(笑)それは自分で考えようよ」と思っちゃう。ただ、インデントの幅とかスペースかタブかみたいな話は、修正するとdiffが大量に出てくるのでそれは事前に考えておいたほうがいいと思いますね。

角 : オートフォーマットで触ってないところもガーッと変わってしまったりしますよね。

まつもと : そうですね。それだけはよくないと思うんだけど、無駄なdiffが発生しないようにすればコーディング規約は特に必要ないんじゃないかなと思ってしまいます。例えば、変数名は必ず長い表現にしようとか。3回しか出てこない変数が長い名前だろうが、iだろうがどうでもよくない?と考えてしまうんです。

角 : 3回だったら全く関係ないですね。もしi がグローバルでどこでも使われていたらそれは問題ですけど、そんな馬鹿げたコードは書かないと思います。

まつもと : それは普通に考えたらわかることですよね。仮にそう書いちゃったとしても、コードレビューの時にダメだっていう理由があるわけだから、「これだけじゃわからん」みたいに指摘されちゃえばいいわけです。そう考えるとコーディング規約はそこまで重要じゃないんじゃないかなと思ってるんですよね。

f:id:sideci:20180302130701p:plain

Rubyコミュニティーの一部でもRuboCop万歳みたいな人たちが一定の割合でいて、でもRuboCopのデフォルトだと僕の好みとだいぶ違ったりして、どうして?となるようなルールも結構入ってる。もちろんRuboCopのような静的解析ツールの価値はわかるしRuboCopの人たちを尊敬してもいます。

角 : デフォルトのRuboCopは私も苦手ですね。SideCI社内のSlackでもこのプルリクエストがRuboCop本体にマージされちゃうのかぁ。という苦言のようなコメントが出ることはありますね。

まつもと : 基本的にプログラマは自主的な側面があると思うので、自分の事は自分で決めたらいいんじゃないかな。赤ん坊を扱うみたいに1から10までこれに従っとけばいいんだというのは、プログラマとしてまともに扱われてないんじゃないかと思います。私はそう扱われなくないし、だからこそ、他の人に対してもそういう扱いをしたくないと考えています。

角 : なるほど。

まつもと : ただ、ツールの支援で色々面倒くさいことが減るという明確な理由があれば、それでチェックすればいいしそれに従えばいいんです。

角 : SideCIに関して言えば、これでどうだろう?というSideCIの提案に対してユーザーがそれを受け止めるかどうかを決めてもらう形にしているので、従えというのは全く言わないスタンスのプロダクトになっています。

まつもと : それは素晴らしい!

角 : それでも、書き方に悩む人っていうのはやっぱり一定数いると思うんですよね。極論ですが、そういう人はどういう風に書けばいいと思いますか?自由に好きに書けというのか、それ以外なのか。

f:id:sideci:20180302130710p:plain

まつもと : 場合にはよりますけど、仕事のソフトウェアに関しては、まわりの人が書くコードから逸脱しないほうがいいんじゃないかと思いますね。個人的な開発の場合は自分で好きなように書けばいいと思います。酷い内容を書いて酷い目に遭うのは自分なので(笑)

角 : 私が最近よくお客様から相談を受けることに、こんなのがあります。コードレビューをしてくださいと依頼があってコードがあがってきた時、コードの機能要件は満たしているし、ちゃんと動くのだけど、コピペが多いなど保守性が低いコードで困る、というですね。こういう場合、まつもとさんならどう対応しますか?

まつもと : そういった場合は情け容赦なくリジェクトするようにしていますね。

角 : 情け容赦なしのリジェクトですか(笑)

まつもと : これこれこういう理由で採用できないのでリジェクトという感じですね。パフォーマンス上の懸念がありますとか、コピペによる保守性の低さが気になりますとか言ってリジェクトすることが多いです。場合によっては自分で引き取った後、書き直してコミットという場合もありますね。

仕事だと締め切りがあるのでその辺をどうするかというのは悩ましいことですが、仕事でも、オープンソースでも、コミットしたコードを自分が将来的にメンテナンスするはめになるけど、それをできるのか?っていうのが私の基準です。

角 : それがYesだったらマージすればいいし、Noだったらリジェクトすればいい。

まつもと : そうそう。特にオープンソースの場合はプルリクエストが来るということはある意味、将来、仮にそのコードから問題が発生した場合でも、必ずその人がメンテナンスをしてくれるとは限らないんですよね。

角 : 連絡が取れないとか、してくれない可能性も高いですね。(笑)

まつもと : そうなるとコードを引き受ける決心の問題もレビューの中に含まれているんです。だから、ゴミを投げつけるとか、プルリクエストに対して酷い表現をする人たちもいましたけど、実際にはそういった側面もあると思います。仕事であれば、君がプルリクエストしたんだから自分でやってくれと言えるかもしれないけど。

角 : OSSだとそういう側面が強いですね。

まつもと : 自分がメンテナンスするコードとして受け入れることができるか、できないのかというのは私の中の1つの基準になっていますね。

f:id:sideci:20180302130630p:plain

プログラミングはよりインタラクティブになっていく

角 : SideCIは、コードレビュープロセスの一部を機械に任せて人は楽をしながらより生産性が高い業務に注力することができる開発補助ツールです。SideCIに応援のメッセージがあればコメントをいただけませんか?

まつもと : 未来のソフトウェア開発というのはよりインタラクティブになっていくと思ってるんです。文法的に間違っているときはシンタックスエラーにはなるんだけど、そういうことではなくて、これはきっと間違いだけど、こういう意味なの?という感じでコンピュータが教えてくれる。それを参考にしながら書けばより品質の高いソフトウェアが作れたりとか、より早い段階で間違いを見つけることができたりする。Rubyというコードは今のコンパクトさのまま、それに付随して例えば外側に型推論の情報が来るとかそんな風になっていくんじゃないかという感じです。人間はRubyのコードしか見ない。知りたかったらIDEとかエディターに聞くと、この変数の型はこうですよとか、呼び出せるメソッドは何ですよとか、さらには、あなたの書いたコードは全体的に矛盾してるよと教えてくれたり。

そういう観点で言うと、SideCIのようなコードレビューを補助してくれるツールがソフトウェアの開発プロセス中に存在するというのは、ぼくが理想とする世界の一部を実現してるのかなという感じですね。

角 : ありがとうございます。

まつもと : 今日私が講演で触れたlanguage server protocolにも関係してくることなのですが、開発者とコンピュータがイントラクトする接点が未来永劫ずっと同じ箇所なのかはわからないのだけど、ソフトウェアに対する理解に関してコンピュータが支援してくれる、書くときにより正しいソフトウェアとなるように手助けしてくれることはソフトウェア開発の理想だと思います。せっかくコンピュータはどんどん賢くなっているので、それをプログラマーが楽になる方向に使っていけたらいいですね。

(取材日 : 2018年2月23日)