find_by_* cop #3455
Comments
|
Sounds reasonable to me. |
|
It's nice cop. 👍 I'll implement this. |
|
@pocke Great! |
Resolve rubocop#3455 Feature ---- This cop checks dynamic `find_by_*` method. ```ruby User.find_by_email(email) User.find_by_name!(name) User.find_by_email_and_name(email, name) User.find_by(email: email) User.find_by!(name: name) User.find_by(email: email, name: name) ``` Note ----- When arguments size and column name size are different, the cop registers an offence. However, doesn't auto-correct. e.g. ```ruby User.find_by_name_and_email(name) ```
Resolve #3455 Feature ---- This cop checks dynamic `find_by_*` method. ```ruby User.find_by_email(email) User.find_by_name!(name) User.find_by_email_and_name(email, name) User.find_by(email: email) User.find_by!(name: name) User.find_by(email: email, name: name) ``` Note ----- When arguments size and column name size are different, the cop registers an offence. However, doesn't auto-correct. e.g. ```ruby User.find_by_name_and_email(name) ```
|
What about the situations when somebody adds a method like: def self.find_by_name(value)
where("json_column -> 'key1' ->> 'key2' = ?", value).first
endThis is not a deprecated dynamic finder method and it could not be rewritten using default Rails |
|
@nbulaj I think that's a rare (although definitely legit) case, and I'd use |
|
@nbulaj Second, add # In .rubocop.yml
Rails/DynamicFindBy:
Whitelist:
- find_by_sql
- find_by_name |
|
@pocke is there are a few methods like that - yes, it will solve the problem. Time will tell :) |
Resolve rubocop#3455 Feature ---- This cop checks dynamic `find_by_*` method. ```ruby User.find_by_email(email) User.find_by_name!(name) User.find_by_email_and_name(email, name) User.find_by(email: email) User.find_by!(name: name) User.find_by(email: email, name: name) ``` Note ----- When arguments size and column name size are different, the cop registers an offence. However, doesn't auto-correct. e.g. ```ruby User.find_by_name_and_email(name) ```
|
I believe this cop is not correct.
http://guides.rubyonrails.org/active_record_querying.html#dynamic-finders |
|
@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 |
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.The text was updated successfully, but these errors were encountered: