Merge pull request #1313 from koic/fix_false_positive_for_rails_compact_blank

Fix false positives for `Rails/CompactBlank`

Author: koic

changelog/fix_false_positive_for_rails_compact_blank.md

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

@@ -16,7 +16,6 @@ module Rails
       #   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, autocorrected code may get unexpected behavior.
       #
       # @example
@@ -33,8 +32,6 @@ module Rails
       #   # bad
       #   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!`
       #   collection.keep_if(&:present?)            # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
       #   collection.keep_if { |_k, v| v.present? } # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
       #
@@ -47,20 +44,20 @@ class CompactBlank < Base
         extend TargetRailsVersion
 
         MSG = 'Use `%<preferred_method>s` instead.'
-        RESTRICT_ON_SEND = %i[reject delete_if reject! select keep_if].freeze
+        RESTRICT_ON_SEND = %i[reject delete_if select keep_if].freeze
 
         minimum_target_rails_version 6.1
 
         def_node_matcher :reject_with_block?, <<~PATTERN
           (block
-            (send _ {:reject :delete_if :reject!})
+            (send _ {:reject :delete_if})
             $(args ...)
             (send
               $(lvar _) :blank?))
         PATTERN
 
         def_node_matcher :reject_with_block_pass?, <<~PATTERN
-          (send _ {:reject :delete_if :reject!}
+          (send _ {:reject :delete_if}
             (block_pass
               (sym :blank?)))
         PATTERN

spec/rubocop/cop/rails/compact_blank_spec.rb

@@ -46,25 +46,15 @@
       RUBY
     end
 
-    it 'registers and corrects an offense when using `reject! { |e| e.blank? }`' do
-      expect_offense(<<~RUBY)
+    it 'does not registers an offense when using `reject! { |e| e.blank? }`' do
+      expect_no_offenses(<<~RUBY)
         collection.reject! { |e| e.blank? }
-                   ^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
-      RUBY
-
-      expect_correction(<<~RUBY)
-        collection.compact_blank!
       RUBY
     end
 
-    it 'registers and corrects an offense when using `reject!(&:blank?)`' do
-      expect_offense(<<~RUBY)
+    it 'does not register an offense when using `reject!(&:blank?)`' do
+      expect_no_offenses(<<~RUBY)
         collection.reject!(&:blank?)
-                   ^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
-      RUBY
-
-      expect_correction(<<~RUBY)
-        collection.compact_blank!
       RUBY
     end