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

Is there a way to force Style/HashExcept on Ruby 2.x such as in Rails app? #790

Closed
r7kamura opened this issue Sep 11, 2022 · 1 comment · Fixed by #791
Closed

Is there a way to force Style/HashExcept on Ruby 2.x such as in Rails app? #790

r7kamura opened this issue Sep 11, 2022 · 1 comment · Fixed by #791

Comments

@r7kamura
Copy link
Contributor

r7kamura commented Sep 11, 2022

Is your feature request related to a problem? Please describe.

I'm using Ruby 2.7 in my Rails application and I know there is Style/HashExcept cop which is useful where we can use Hash#except.

Hash#except was originally provided as a core extension to activesupport, and then it was ported to Ruby, right? Since Hash#except is already available for environments like Rails apps, I would really like to take advantage of this cop, regardless of the Ruby version.

So the question is, is there any good way to enable RuboCop to enable this cop in such case?

Describe the solution you'd like

I think the best way for users is that this cop is disabled by default, but can be enabled by writing the following setting:

Style/HashExcept:
  Enabled: true

though I understand that from an internal implementation point of view, that would be a bit of a challenge.

Describe alternatives you've considered

Monkey-patching minimum_target_ruby_version will work, so I'm doing this for now 😇

require:
  - ./monkey-patch-style-hash-except.rb
#  ./monkey-patch-style-hash-except.rb
RuboCop::Cop::Style::HashExcept.minimum_target_ruby_version(2.7)

Additional context

In this issue, I am targeting Style/HashExcept for example, but I think this kind of problem can actually occur in other cops. I hope some good way is provided in such cases.

The real problem we are having is that we are trying to upgrade Ruby from 2.7 to 3.0, but when we try to upgrade it to Ruby 3.0, Style/HashExcept starts reporting offenses. Well, we can temporarily disable this cop, but ideally we don't want to do that, right? Also we don't want to make changes about Hash#except at the same time with Ruby upgrade, if possible, because the diff would be larger and then the risk of conflict would be greater.

Besides Hash#except, I am sure there are other cases where, for example, you want to backport a new feature from new Ruby or Rails version and enable cop for this feature in advance in order to make upgrading easier. Another example is the deprecation of to_fs in Rails 7. That required a large change to our codebase, so we backported that feature in Rails 6.1, and then we had to monkey-patch Rails/ToSWithArgument cop on minimum_target_rails_version 7.0. I wish we had the ability to force enable it in this situation as well.

@koic koic transferred this issue from rubocop/rubocop Sep 22, 2022
koic added a commit to koic/rubocop-rails that referenced this issue Sep 22, 2022
… 2.x

Fixes rubocop#790.

This PR makes `Style/HashExcept` aware of TargetRubyVersion: 2.x
because Rails has `Hash#except`.
So Ruby version is irrelevant when used in Rails.
@koic
Copy link
Member

koic commented Sep 22, 2022

Since Rails has Hash#except, I think it makes sense for Rails to always support it and opened #791. On the other hand, the Rails/ToSWithArgument backpoting is about monkey patching. I think application monkey patching should be solved on the application side, not framework and follower side.

@koic koic closed this as completed in #791 Oct 4, 2022
koic added a commit that referenced this issue Oct 4, 2022
…get_ruby_version_2_x

[Fix #790] Make `Style/HashExcept` aware of TargetRubyVersion: 2.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants