search

rubocop / rubocop-jp

A place for RuboCop discussions in Japanese
55 stars 2 forks source link

Rails/DynamicFindBy の誤検出 #54

Closed znz closed 3 years ago

znz commented 5 years ago

rubocop-rails 2.0.1 の Rails/DynamicFindBy で

が誤検出されました。

とりあえず Exclude でファイルごと除外していますが、 tokens gem と似た感じで独自定義の find_by_なんとか も引っかかっていたので、許可するメソッド名を指定したいです。

find_by_id の方は ActiveRecord の方は指摘して欲しいので、 capybara を使った spec の方を別の書き方にした方が良いのかもしれないと思いました。

koic commented 4 years ago

Tokens の find_by_valid_token の方は .rubocop.yml に以下の指定をすることで解決しそうです。

Rails/DynamicFindBy:
  Whitelist:
    - find_by_valid_token

https://github.com/rubocop-hq/rubocop-rails/blob/v2.5.2/config/default.yml#L163-L169

なお Rails/DynamicFindBy の Whitelist は、次の RuboCop Rails のリリース (おそらく 2.6.0) に含まれる予定の https://github.com/rubocop-hq/rubocop-rails/pull/69 で非推奨となり、 AllowedMethods というパラメータ名が推奨になる予定です。

Capybara の find_by_id については、偽陰性があるかもしれませんがレシーバーを指定しない形での find_by_id は検出しないといったデフォルトで有効のオプショナルを用意することで回避できる気がしました。実装について考えてみます。現状のワークアラウンドとしては Rails/DynamicFindBy への Exclude パラメータで回避したいテストのディレクトリパスを指定するといった方法が考えられます。

koic commented 3 years ago

Capybara の find_by_id については、偽陰性があるかもしれませんがレシーバーを指定しない形での find_by_id は検出しないといったデフォルトで有効のオプショナルを用意することで回避できる気がしました。実装について考えてみます。

https://github.com/rubocop/rubocop-rails/pull/466 にて、レシーバーなしの find_by_xxx 呼び出しは、ApplicationRecord (ActiveRecord::Base) を継承したクラスの中で使っている場合を除いて受け入れるようにしました。次にリリースする RuboCop Rails 2.10.0 で適用する予定です。リリースまでお待ちください。

koic commented 3 years ago

Rails/DynamicFindBy cop についての調整を含んだ、RuboCop Rails 2.10.0 をリリースしました。 https://github.com/rubocop/rubocop-rails/releases/tag/v2.10.0