Merge pull request #651 from koic/make_rails_compact_blank_aware_of_reject_delete_if

[Fix #650] Make `Rails/CompactBlank` aware of `delete_if(&:blank)`

Author: koic

changelog/change_make_rails_compact_blank_aware_of_reject_delete_if.md

@@ -0,0 +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

@@ -13,6 +13,12 @@ module Rails
       #   `[[1, 2], [3, nil]].compact_blank` are not compatible. The same is true for `blank?`.
       #   This will work fine when the receiver is a hash object.
       #
+      #   And `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.
+      #
       # @example
       #
       #   # bad
@@ -23,8 +29,10 @@ module Rails
       #   collection.compact_blank
       #
       #   # bad
-      #   collection.reject!(&:blank?)
-      #   collection.reject! { |_k, v| v.blank? }
+      #   collection.delete_if(&:blank?)            # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
+      #   collection.delete_if { |_k, v| v.blank? } # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
+      #   collection.reject!(&:blank?)              # Same behavior as `ActionController::Parameters#compact_blank!`
+      #   collection.reject! { |_k, v| v.blank? }   # Same behavior as `ActionController::Parameters#compact_blank!`
       #
       #   # good
       #   collection.compact_blank!
@@ -35,20 +43,20 @@ class CompactBlank < Base
         extend TargetRailsVersion
 
         MSG = 'Use `%<preferred_method>s` instead.'
-        RESTRICT_ON_SEND = %i[reject reject!].freeze
+        RESTRICT_ON_SEND = %i[reject delete_if reject!].freeze
 
         minimum_target_rails_version 6.1
 
         def_node_matcher :reject_with_block?, <<~PATTERN
           (block
-            (send _ {:reject :reject!})
+            (send _ {:reject :delete_if :reject!})
             $(args ...)
             (send
               $(lvar _) :blank?))
         PATTERN
 
         def_node_matcher :reject_with_block_pass?, <<~PATTERN
-          (send _ {:reject :reject!}
+          (send _ {:reject :delete_if :reject!}
             (block_pass
               (sym :blank?)))
         PATTERN

spec/rubocop/cop/rails/compact_blank_spec.rb

@@ -24,6 +24,28 @@
       RUBY
     end
 
+    it 'registers and corrects an offense when using `delete_if { |e| e.blank? }`' do
+      expect_offense(<<~RUBY)
+        collection.delete_if { |e| e.blank? }
+                   ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection.compact_blank!
+      RUBY
+    end
+
+    it 'registers and corrects an offense when using `delete_if(&:blank?)`' do
+      expect_offense(<<~RUBY)
+        collection.delete_if(&:blank?)
+                   ^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection.compact_blank!
+      RUBY
+    end
+
     it 'registers and corrects an offense when using `reject! { |e| e.blank? }`' do
       expect_offense(<<~RUBY)
         collection.reject! { |e| e.blank? }