Rubocop: find_by_* cop

[dkniffin]

I'd like to suggest a new cop, which will look for instances of the old rails dynamic finders (ie: find_by_id) and prefer the non-dynamic version: (find_by(id: ...))

Edit: whoever implements this, be careful of instances such as find_by_sql, which is a legit method. Not sure if there are others.

[bbatsov]

Sounds reasonable to me.

[pocke]

It's nice cop. :+1: I'll implement this.

[bbatsov]

@pocke Great!

[nbulaj]

What about the situations when somebody adds a method like:

def self.find_by_name(value)
  where("json_column -> 'key1' ->> 'key2' = ?", value).first
end

This is not a deprecated dynamic finder method and it could not be rewritten using default Rails find_by syntax

[dkniffin]

@nbulaj I think that's a rare (although definitely legit) case, and I'd use rubocop:disable Rails/DynamicFindBy above that.

[pocke]

@nbulaj
We can take two steps to avoid the problem.
First, @dkniffin says, disable the cop by inline comment.

Second, add find_by_name to whitelist.
https://github.com/bbatsov/rubocop/blob/6a3442b/config/default.yml#L1239-L1241

# In .rubocop.yml
Rails/DynamicFindBy:
  Whitelist:
    - find_by_sql
    - find_by_name

[nbulaj]

@pocke is there are a few methods like that - yes, it will solve the problem. Time will tell :)

[paulodeon]

I believe this cop is not correct.

find_by_column_name dynamic finders are still ok

http://guides.rubyonrails.org/active_record_querying.html#dynamic-finders

[pocke]

@paulodeon This cop does not say "dynamic finders don't work". I think it is a style cop. It is not a lint cop. Dynamic finders works well, but this cop enforces find_by(name: name) style.