Skip to content

Commit 78d3eee

Browse files
committed
Fix false positives for Rails/CompactBlank
This PR fixes false positives for `Rails/CompactBlank` when using `collection.reject!`. `reject!(&:blank?)` is not equivalent to `compact_blank!`: ```ruby {}.reject!(&:blank?) # => nil {}.reject! { |_k, v| v.blank? } # => nil {}.compact_blank! # => {} ``` This PR prioritizes safer behavior with plain hashes and arrays.
1 parent e012af8 commit 78d3eee

File tree

3 files changed

+8
-20
lines changed

3 files changed

+8
-20
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1313](https://github.com/rubocop/rubocop-rails/pull/1313): Fix false positives for `Rails/CompactBlank` when using `collection.reject!`. ([@koic][])

lib/rubocop/cop/rails/compact_blank.rb

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ module Rails
1616
# And `compact_blank!` has different implementations for `Array`, `Hash`, and
1717
# `ActionController::Parameters`.
1818
# `Array#compact_blank!`, `Hash#compact_blank!` are equivalent to `delete_if(&:blank?)`.
19-
# `ActionController::Parameters#compact_blank!` is equivalent to `reject!(&:blank?)`.
2019
# If the cop makes a mistake, autocorrected code may get unexpected behavior.
2120
#
2221
# @example
@@ -33,8 +32,6 @@ module Rails
3332
# # bad
3433
# collection.delete_if(&:blank?) # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
3534
# collection.delete_if { |_k, v| v.blank? } # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
36-
# collection.reject!(&:blank?) # Same behavior as `ActionController::Parameters#compact_blank!`
37-
# collection.reject! { |_k, v| v.blank? } # Same behavior as `ActionController::Parameters#compact_blank!`
3835
# collection.keep_if(&:present?) # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
3936
# collection.keep_if { |_k, v| v.present? } # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
4037
#
@@ -47,20 +44,20 @@ class CompactBlank < Base
4744
extend TargetRailsVersion
4845

4946
MSG = 'Use `%<preferred_method>s` instead.'
50-
RESTRICT_ON_SEND = %i[reject delete_if reject! select keep_if].freeze
47+
RESTRICT_ON_SEND = %i[reject delete_if select keep_if].freeze
5148

5249
minimum_target_rails_version 6.1
5350

5451
def_node_matcher :reject_with_block?, <<~PATTERN
5552
(block
56-
(send _ {:reject :delete_if :reject!})
53+
(send _ {:reject :delete_if})
5754
$(args ...)
5855
(send
5956
$(lvar _) :blank?))
6057
PATTERN
6158

6259
def_node_matcher :reject_with_block_pass?, <<~PATTERN
63-
(send _ {:reject :delete_if :reject!}
60+
(send _ {:reject :delete_if}
6461
(block_pass
6562
(sym :blank?)))
6663
PATTERN

spec/rubocop/cop/rails/compact_blank_spec.rb

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,25 +46,15 @@
4646
RUBY
4747
end
4848

49-
it 'registers and corrects an offense when using `reject! { |e| e.blank? }`' do
50-
expect_offense(<<~RUBY)
49+
it 'does not registers an offense when using `reject! { |e| e.blank? }`' do
50+
expect_no_offenses(<<~RUBY)
5151
collection.reject! { |e| e.blank? }
52-
^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
53-
RUBY
54-
55-
expect_correction(<<~RUBY)
56-
collection.compact_blank!
5752
RUBY
5853
end
5954

60-
it 'registers and corrects an offense when using `reject!(&:blank?)`' do
61-
expect_offense(<<~RUBY)
55+
it 'does not register an offense when using `reject!(&:blank?)`' do
56+
expect_no_offenses(<<~RUBY)
6257
collection.reject!(&:blank?)
63-
^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
64-
RUBY
65-
66-
expect_correction(<<~RUBY)
67-
collection.compact_blank!
6858
RUBY
6959
end
7060

0 commit comments

Comments
 (0)