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 to detect unnecessary hash merge and then splat #12257

Closed
fatkodima opened this issue Oct 10, 2023 · 0 comments · Fixed by #12258
Closed

New cop to detect unnecessary hash merge and then splat #12257

fatkodima opened this issue Oct 10, 2023 · 0 comments · Fixed by #12258

Comments

@fatkodima
Copy link
Contributor

Just found an example code from rails that can be improved https://github.com/rails/rails/blob/8ab5e58272b1da9548bb607542525002b44799ed/activesupport/lib/active_support/number_helper/number_converter.rb#L166-L172

Duplicating first method here

# Bad
def translate_number_value_with_default(key, **i18n_options)
  I18n.translate(key, **{ default: default_value(key), scope: :number }.merge!(i18n_options))
end

There is unnecessary merge! and hash splat. Can be also written as:

# Good
def translate_number_value_with_default(key, **i18n_options)
  I18n.translate(key, default: default_value(key), scope: :number, **i18n_options)
end

I have also seen such examples before in the wild, so this is quite a popular pattern.

koic added a commit to koic/rubocop that referenced this issue Oct 11, 2023
… of `merge`

Resolves rubocop#12257.

This PR makes `Style/RedundantDoubleSplatHashBraces` aware of `merge` methods.
Instead of introducing a new cop, this `Style/RedundantDoubleSplatHashBraces` cop can be extended.
bbatsov pushed a commit that referenced this issue Oct 11, 2023
…rge`

Resolves #12257.

This PR makes `Style/RedundantDoubleSplatHashBraces` aware of `merge` methods.
Instead of introducing a new cop, this `Style/RedundantDoubleSplatHashBraces` cop can be extended.
koic added a commit to koic/rubocop that referenced this issue Oct 11, 2023
… of `merge`

Resolves rubocop#12257.

This PR makes `Style/RedundantDoubleSplatHashBraces` aware of `merge` methods.
Instead of introducing a new cop, this `Style/RedundantDoubleSplatHashBraces` cop can be extended.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants