Skip to content

Commit 604e429

Browse files
committed
[Fix #650] Make Rails/CompactBlank aware of delete_if(&:blank)
Fixes #650. This PR makes `Rails/CompactBlank` aware of `delete_if(&:blank)`. It is unsafe because `compact_blank!` has different implementations for `Array`, `Hash`, and `ActionController::Parameters`. `Array#compact_blank!`, `Hash#compact_blank!` are equivalent to `delete_if(&:blank?)`. `ActionController::Parameters#compact_blank!` is equivalent to `reject!(&:blank?)`. If the cop makes a mistake, auto-corrected code may get unexpected behavior. This PR also updates unsafe document about it.
1 parent 3331b65 commit 604e429

File tree

3 files changed

+36
-5
lines changed

3 files changed

+36
-5
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#650](https://github.com/rubocop/rubocop-rails/issues/650): Make `Rails/CompactBlank` aware of `delete_if(&:blank)`. ([@koic][])

lib/rubocop/cop/rails/compact_blank.rb

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ module Rails
1313
# `[[1, 2], [3, nil]].compact_blank` are not compatible. The same is true for `blank?`.
1414
# This will work fine when the receiver is a hash object.
1515
#
16+
# And `compact_blank!` has different implementations for `Array`, `Hash`, and
17+
# `ActionController::Parameters`.
18+
# `Array#compact_blank!`, `Hash#compact_blank!` are equivalent to `delete_if(&:blank?)`.
19+
# `ActionController::Parameters#compact_blank!` is equivalent to `reject!(&:blank?)`.
20+
# If the cop makes a mistake, auto-corrected code may get unexpected behavior.
21+
#
1622
# @example
1723
#
1824
# # bad
@@ -23,8 +29,10 @@ module Rails
2329
# collection.compact_blank
2430
#
2531
# # bad
26-
# collection.reject!(&:blank?)
27-
# collection.reject! { |_k, v| v.blank? }
32+
# collection.delete_if(&:blank?) # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
33+
# collection.delete_if { |_k, v| v.blank? } # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
34+
# collection.reject!(&:blank?) # Same behavior as `ActionController::Parameters#compact_blank!`
35+
# collection.reject! { |_k, v| v.blank? } # Same behavior as `ActionController::Parameters#compact_blank!`
2836
#
2937
# # good
3038
# collection.compact_blank!
@@ -35,20 +43,20 @@ class CompactBlank < Base
3543
extend TargetRailsVersion
3644

3745
MSG = 'Use `%<preferred_method>s` instead.'
38-
RESTRICT_ON_SEND = %i[reject reject!].freeze
46+
RESTRICT_ON_SEND = %i[reject delete_if reject!].freeze
3947

4048
minimum_target_rails_version 6.1
4149

4250
def_node_matcher :reject_with_block?, <<~PATTERN
4351
(block
44-
(send _ {:reject :reject!})
52+
(send _ {:reject :delete_if :reject!})
4553
$(args ...)
4654
(send
4755
$(lvar _) :blank?))
4856
PATTERN
4957

5058
def_node_matcher :reject_with_block_pass?, <<~PATTERN
51-
(send _ {:reject :reject!}
59+
(send _ {:reject :delete_if :reject!}
5260
(block_pass
5361
(sym :blank?)))
5462
PATTERN

spec/rubocop/cop/rails/compact_blank_spec.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,28 @@
2424
RUBY
2525
end
2626

27+
it 'registers and corrects an offense when using `delete_if { |e| e.blank? }`' do
28+
expect_offense(<<~RUBY)
29+
collection.delete_if { |e| e.blank? }
30+
^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
31+
RUBY
32+
33+
expect_correction(<<~RUBY)
34+
collection.compact_blank!
35+
RUBY
36+
end
37+
38+
it 'registers and corrects an offense when using `delete_if(&:blank?)`' do
39+
expect_offense(<<~RUBY)
40+
collection.delete_if(&:blank?)
41+
^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
42+
RUBY
43+
44+
expect_correction(<<~RUBY)
45+
collection.compact_blank!
46+
RUBY
47+
end
48+
2749
it 'registers and corrects an offense when using `reject! { |e| e.blank? }`' do
2850
expect_offense(<<~RUBY)
2951
collection.reject! { |e| e.blank? }

0 commit comments

Comments
 (0)