-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This PR adds new `Style/HashExcept` cop. This cop checks for usages of `Hash#reject`, `Hash#select`, and `Hash#filter` methods that can be replaced with `Hash#except` method. This cop should only be enabled on Ruby version 3.0 or higher. (`Hash#except` was added in Ruby 3.0.) ```ruby # bad {foo: 1, bar: 2, baz: 3}.reject {|k, v| k == :bar } {foo: 1, bar: 2, baz: 3}.select {|k, v| k != :bar } # good {foo: 1, bar: 2, baz: 3}.except(:bar) ``` cf. https://bugs.ruby-lang.org/issues/15822
- Loading branch information
Showing
5 changed files
with
272 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
* [#9283](https://github.com/rubocop-hq/rubocop/pull/9283): Add new `Style/HashExcept` cop. ([@koic][]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Style | ||
# This cop checks for usages of `Hash#reject`, `Hash#select`, and `Hash#filter` methods | ||
# that can be replaced with `Hash#except` method. | ||
# | ||
# This cop should only be enabled on Ruby version 3.0 or higher. | ||
# (`Hash#except` was added in Ruby 3.0.) | ||
# | ||
# For safe detection, it is limited to commonly used string and symbol comparisons | ||
# when used `==`. | ||
# And do not check `Hash#delete_if` and `Hash#keep_if` to change receiver object. | ||
# | ||
# @example | ||
# | ||
# # bad | ||
# {foo: 1, bar: 2, baz: 3}.reject {|k, v| k == :bar } | ||
# {foo: 1, bar: 2, baz: 3}.select {|k, v| k != :bar } | ||
# {foo: 1, bar: 2, baz: 3}.filter {|k, v| k != :bar } | ||
# | ||
# # good | ||
# {foo: 1, bar: 2, baz: 3}.except(:bar) | ||
# | ||
class HashExcept < Base | ||
include RangeHelp | ||
extend TargetRubyVersion | ||
extend AutoCorrector | ||
|
||
minimum_target_ruby_version 3.0 | ||
|
||
MSG = 'Use `%<prefer>s` instead.' | ||
RESTRICT_ON_SEND = %i[reject select filter].freeze | ||
|
||
def_node_matcher :bad_method?, <<~PATTERN | ||
(block | ||
(send _ _) | ||
(args | ||
(arg _) | ||
(arg _)) | ||
(send | ||
_ {:== :!= :eql?} _)) | ||
PATTERN | ||
|
||
def on_send(node) | ||
block = node.parent | ||
return unless bad_method?(block) && semantically_except_method?(node, block) | ||
|
||
except_key = except_key(block) | ||
return unless safe_to_register_offense?(block, except_key) | ||
|
||
range = offense_range(node) | ||
preferred_method = "except(#{except_key.source})" | ||
|
||
add_offense(range, message: format(MSG, prefer: preferred_method)) do |corrector| | ||
corrector.replace(range, preferred_method) | ||
end | ||
end | ||
|
||
private | ||
|
||
def semantically_except_method?(send, block) | ||
body = block.body | ||
|
||
case send.method_name | ||
when :reject | ||
body.method?('==') || body.method?('eql?') | ||
when :select, :filter | ||
body.method?('!=') | ||
else | ||
false | ||
end | ||
end | ||
|
||
def safe_to_register_offense?(block, except_key) | ||
return true if block.body.method?('eql?') | ||
|
||
except_key.sym_type? || except_key.str_type? | ||
end | ||
|
||
def except_key(node) | ||
key_argument = node.argument_list.first | ||
lhs, _method_name, rhs = *node.body | ||
|
||
[lhs, rhs].find { |operand| operand.source != key_argument.source } | ||
end | ||
|
||
def offense_range(node) | ||
range_between(node.loc.selector.begin_pos, node.parent.loc.end.end_pos) | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,168 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe RuboCop::Cop::Style::HashExcept, :config do | ||
context 'Ruby 3.0 or higher', :ruby30 do | ||
it 'registers and corrects an offense when using `reject` and comparing with `lvar == :sym`' do | ||
expect_offense(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.reject { |k, v| k == :bar } | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `except(:bar)` instead. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.except(:bar) | ||
RUBY | ||
end | ||
|
||
it 'registers and corrects an offense when using `reject` and comparing with `:sym == lvar`' do | ||
expect_offense(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.reject { |k, v| :bar == k } | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `except(:bar)` instead. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.except(:bar) | ||
RUBY | ||
end | ||
|
||
it 'registers and corrects an offense when using `select` and comparing with `lvar != :sym`' do | ||
expect_offense(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.select { |k, v| k != :bar } | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `except(:bar)` instead. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.except(:bar) | ||
RUBY | ||
end | ||
|
||
it 'registers and corrects an offense when using `select` and comparing with `:sym != lvar`' do | ||
expect_offense(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.select { |k, v| :bar != k } | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `except(:bar)` instead. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.except(:bar) | ||
RUBY | ||
end | ||
|
||
it "registers and corrects an offense when using `reject` and comparing with `lvar == 'str'`" do | ||
expect_offense(<<~RUBY) | ||
hash.reject { |k, v| k == 'str' } | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `except('str')` instead. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
hash.except('str') | ||
RUBY | ||
end | ||
|
||
it 'registers and corrects an offense when using `reject` and other than comparison by string and symbol using `eql?`' do | ||
expect_offense(<<~RUBY) | ||
hash.reject { |k, v| k.eql?(0.0) } | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `except(0.0)` instead. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
hash.except(0.0) | ||
RUBY | ||
end | ||
|
||
it 'registers and corrects an offense when using `filter` and comparing with `lvar != :sym`' do | ||
expect_offense(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.filter { |k, v| k != :bar } | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `except(:bar)` instead. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.except(:bar) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `reject` and other than comparison by string and symbol using `==`' do | ||
expect_no_offenses(<<~RUBY) | ||
hash.reject { |k, v| k == 0.0 } | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `delete_if` and comparing with `lvar == :sym`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.delete_if { |k, v| k == :bar } | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `keep_if` and comparing with `lvar != :sym`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.keep_if { |k, v| k != :bar } | ||
RUBY | ||
end | ||
end | ||
|
||
context 'Ruby 2.7 or lower', :ruby27 do | ||
it 'does not register an offense when using `reject` and comparing with `lvar == :key`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.reject { |k, v| k == :bar } | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `reject` and comparing with `:key == lvar`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.reject { |k, v| :bar == k } | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `select` and comparing with `lvar != :key`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.select { |k, v| k != :bar } | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `select` and comparing with `:key != lvar`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.select { |k, v| :bar != k } | ||
RUBY | ||
end | ||
end | ||
|
||
it 'does not register an offense when using `reject` and comparing with `lvar != :key`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.reject { |k, v| k != :bar } | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `reject` and comparing with `:key != lvar`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.reject { |k, v| :bar != key } | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `select` and comparing with `lvar == :key`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.select { |k, v| k == :bar } | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `select` and comparing with `:key == lvar`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.select { |k, v| :bar == key } | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when not using key block argument`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.reject { |k, v| do_something != :bar } | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when not using block`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.reject | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `Hash#except`' do | ||
expect_no_offenses(<<~RUBY) | ||
{foo: 1, bar: 2, baz: 3}.except(:bar) | ||
RUBY | ||
end | ||
end |