From 3554550f9a262c8fc8f9c44a6453c8ed8f530b2a Mon Sep 17 00:00:00 2001 From: Ryo Nakamura Date: Sat, 1 Oct 2022 11:57:09 +0900 Subject: [PATCH] Support autocorrection even if `reject` is used on `Performance/Count` --- ...t_autocorrection_even_if_reject_is_used.md | 1 + lib/rubocop/cop/performance/count.rb | 40 +++- spec/rubocop/cop/performance/count_spec.rb | 225 +++++++++++------- 3 files changed, 184 insertions(+), 82 deletions(-) create mode 100644 changelog/change_support_autocorrection_even_if_reject_is_used.md diff --git a/changelog/change_support_autocorrection_even_if_reject_is_used.md b/changelog/change_support_autocorrection_even_if_reject_is_used.md new file mode 100644 index 0000000000..ff20e75fdd --- /dev/null +++ b/changelog/change_support_autocorrection_even_if_reject_is_used.md @@ -0,0 +1 @@ +* [#307](https://github.com/rubocop/rubocop-performance/pull/307): Support autocorrection even if `reject` is used on `Performance/Count`. ([@r7kamura][]) diff --git a/lib/rubocop/cop/performance/count.rb b/lib/rubocop/cop/performance/count.rb index d3d3233570..97b7debaea 100644 --- a/lib/rubocop/cop/performance/count.rb +++ b/lib/rubocop/cop/performance/count.rb @@ -79,12 +79,11 @@ def on_send(node) def autocorrect(corrector, node, selector_node, selector) selector_loc = selector_node.loc.selector - return if selector == :reject - range = source_starting_at(node) { |n| n.loc.dot.begin_pos } corrector.remove(range) corrector.replace(selector_loc, 'count') + negate_reject(corrector, node) if selector == :reject end def eligible_node?(node) @@ -100,6 +99,43 @@ def source_starting_at(node) range_between(begin_pos, node.source_range.end_pos) end + + def negate_reject(corrector, node) + if node.receiver.send_type? + negate_block_pass_reject(corrector, node) + else + negate_block_reject(corrector, node) + end + end + + def negate_block_pass_reject(corrector, node) + corrector.replace( + node.receiver.loc.expression.with(begin_pos: node.receiver.loc.begin.begin_pos), + negate_block_pass_as_inline_block(node.receiver) + ) + end + + def negate_block_reject(corrector, node) + target = + if node.receiver.body.begin_type? + node.receiver.body.children.last + else + node.receiver.body + end + corrector.replace(target, negate_expression(target)) + end + + def negate_expression(node) + "!(#{node.source})" + end + + def negate_block_pass_as_inline_block(node) + if node.last_argument.children.first.sym_type? + " { |element| !element.#{node.last_argument.children.first.value} }" + else + " { !#{node.last_argument.children.first.source}.call }" + end + end end end end diff --git a/spec/rubocop/cop/performance/count_spec.rb b/spec/rubocop/cop/performance/count_spec.rb index bcf0d71373..af2167cba5 100644 --- a/spec/rubocop/cop/performance/count_spec.rb +++ b/spec/rubocop/cop/performance/count_spec.rb @@ -177,101 +177,166 @@ def count(&block) end end - context 'autocorrect' do - context 'will correct' do - it 'select..size to count' do - expect_offense(<<~RUBY) - [1, 2].select { |e| e > 2 }.size - ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `select...size`. - RUBY - - expect_correction(<<~RUBY) - [1, 2].count { |e| e > 2 } - RUBY - end + context 'with `select` and `size`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + array.select { |e| e > 2 }.size + ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `select...size`. + RUBY - it 'select..count without a block to count' do - expect_offense(<<~RUBY) - [1, 2].select { |e| e > 2 }.count - ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `select...count`. - RUBY + expect_correction(<<~RUBY) + array.count { |e| e > 2 } + RUBY + end + end - expect_correction(<<~RUBY) - [1, 2].count { |e| e > 2 } - RUBY - end + context 'with `select` and `count`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + array.select { |e| e > 2 }.count + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `select...count`. + RUBY - it 'select..length to count' do - expect_offense(<<~RUBY) - [1, 2].select { |e| e > 2 }.length - ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `select...length`. - RUBY + expect_correction(<<~RUBY) + array.count { |e| e > 2 } + RUBY + end + end - expect_correction(<<~RUBY) - [1, 2].count { |e| e > 2 } - RUBY - end + context 'with `select` and `length`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + array.select { |e| e > 2 }.length + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `select...length`. + RUBY - it 'select...size when select has parameters' do - expect_offense(<<~RUBY) - Data = Struct.new(:value) - array = [Data.new(2), Data.new(3), Data.new(2)] - puts array.select(&:value).size - ^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `select...size`. - RUBY - - expect_correction(<<~RUBY) - Data = Struct.new(:value) - array = [Data.new(2), Data.new(3), Data.new(2)] - puts array.count(&:value) - RUBY - end + expect_correction(<<~RUBY) + array.count { |e| e > 2 } + RUBY end + end - describe 'will not correct' do - it 'reject...size' do - expect_offense(<<~RUBY) - [1, 2].reject { |e| e > 2 }.size - ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `reject...size`. - RUBY + context 'with `select` with symbol block argument and `size`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + array.select(&:value).size + ^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `select...size`. + RUBY - expect_no_corrections - end + expect_correction(<<~RUBY) + array.count(&:value) + RUBY + end + end - it 'reject...count' do - expect_offense(<<~RUBY) - [1, 2].reject { |e| e > 2 }.count - ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `reject...count`. - RUBY + context 'with `reject` and `size`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + array.reject { |e| e > 2 }.size + ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `reject...size`. + RUBY - expect_no_corrections - end + expect_correction(<<~RUBY) + array.count { |e| !(e > 2) } + RUBY + end + end - it 'reject...length' do - expect_offense(<<~RUBY) - [1, 2].reject { |e| e > 2 }.length - ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `reject...length`. - RUBY + context 'with `reject` and `count`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + array.reject { |e| e > 2 }.count + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `reject...count`. + RUBY - expect_no_corrections - end + expect_correction(<<~RUBY) + array.count { |e| !(e > 2) } + RUBY + end + end - it 'select...count when count has a block' do - expect_no_offenses(<<~RUBY) - [1, 2].select { |e| e > 2 }.count { |e| e.even? } - RUBY - end + context 'with `reject` and `length`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + array.reject { |e| e > 2 }.length + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `reject...length`. + RUBY - it 'reject...size when select has parameters' do - expect_offense(<<~RUBY) - Data = Struct.new(:value) - array = [Data.new(2), Data.new(3), Data.new(2)] - puts array.reject(&:value).size - ^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `reject...size`. - RUBY + expect_correction(<<~RUBY) + array.count { |e| !(e > 2) } + RUBY + end + end - expect_no_corrections - end + context 'with `reject` with symbol block argument and `size`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + array.reject(&:value).size + ^^^^^^^^^^^^^^^^^^^^ Use `count` instead of `reject...size`. + RUBY + + expect_correction(<<~RUBY) + array.count { |element| !element.value } + RUBY + end + end + + context 'with `reject` with variable block argument and `size`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + array.reject(&block).size + ^^^^^^^^^^^^^^^^^^^ Use `count` instead of `reject...size`. + RUBY + + expect_correction(<<~RUBY) + array.count { !block.call } + RUBY + end + end + + context 'with `reject` with some statements and `length`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + array.reject { + ^^^^^^^^ Use `count` instead of `reject...length`. + foo + bar + }.length + RUBY + + expect_correction(<<~RUBY) + array.count { + foo + !(bar) + } + RUBY + end + end + + context 'with `reject` with some conditional statement and `length`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + array.reject { + ^^^^^^^^ Use `count` instead of `reject...length`. + foo + if bar + baz + else + qux + end + }.length + RUBY + + expect_correction(<<~RUBY) + array.count { + foo + !(if bar + baz + else + qux + end) + } + RUBY end end end