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

Open
Ana06 opened this Issue Dec 27, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

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 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment