Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New cop for Hash#dig #5332

Closed
Ana06 opened this issue Dec 27, 2017 · 5 comments
Closed

New cop for Hash#dig #5332

Ana06 opened this issue Dec 27, 2017 · 5 comments
Labels
feature request stale Issues that haven't been active in a while

Comments

@Ana06
Copy link
Contributor

Ana06 commented Dec 27, 2017

Add a new cop that enforce the use of

params.dig(:user, :name)

instead of

params[:user] && params[:user][:name]

Concerns

When using symbol as key I would say that the complain and the autocorrect are always safe, but not in the following cases:

Strings

Due to the str[match_str] method, things like this happens:

params = { "user" => "Nicolas Cage" } => {"user"=>"Nicolas Cage"}

params.dig("user","age") => TypeError: String does not have #dig method

params["user"] && params["user"]["age"] => "age"

Integers

The same for integers:

params = { "user" => "Nicolas Cage" } => {"user"=>"Nicolas Cage"}

params.dig("user",1) => TypeError: String does not have #dig method

params["user"] && params["user"][1] => "i"

And that's the reason why I think it may be not safe that this cop check for Array#dig as well, as in this case numbers are always use and we get things like:


array=["hola"] => ["hola"]

array.dig(0,1) => TypeError: String does not have #dig method

array[0] && array[0][1] => "o"

For these cases we could add in the Ruby style guide that dig is preferred. I would say those cases are not the normal ones but still maybe not possible that a cop look for them.

@Ana06
Copy link
Contributor Author

Ana06 commented Jan 7, 2018

@Ana06

When using symbol as key I would say that the complain and the autocorrect are always safe

The method are not exactly equivalente with symbols as well:

params = { "user": { name: "Nicolas Cage", married: false } }
  => {:user=>{:name=>"Nicolas Cage", :married=>false}}

params[:user] && params[:user][:married] && params[:user][:married][:date]
  => false

params.dig(:user, :married, :date)
  => TypeError: FalseClass does not have #dig method
             from (irb):6:in `dig'
             from (irb):6
             from /usr/bin/irb.ruby2.4:11:in `<main>'

I am not sure if this can be safely implemented. 🤔

@tjwp
Copy link
Contributor

tjwp commented Jan 8, 2018

I wrote a custom cop to do this: https://github.com/ezcater/ezcater_rubocop/blob/master/lib/rubocop/cop/ezcater/style_dig.rb

We've found it useful, but it is definitely susceptible to false positives, e.g. ranges also have to be excluded.

I wouldn't enable it by default for rubocop.

@Ana06
Copy link
Contributor Author

Ana06 commented Jan 15, 2018

@tjwp what about sending a PR to Rubocop? 😉

I wouldn't enable it by default for rubocop.

yes, me neither 😉

@stale
Copy link

stale bot commented May 8, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale Issues that haven't been active in a while label May 8, 2019
@stale
Copy link

stale bot commented Jun 8, 2019

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

@stale stale bot closed this as completed Jun 8, 2019
@pocke pocke mentioned this issue Jan 2, 2020
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request stale Issues that haven't been active in a while
Projects
None yet
Development

No branches or pull requests

3 participants