From 78d3eee2bdde967d67fe834686db812c9696b283 Mon Sep 17 00:00:00 2001
From: Koichi ITO <koic.ito@gmail.com>
Date: Sat, 13 Jul 2024 23:57:26 +0900
Subject: [PATCH] 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.
---
 ...x_false_positive_for_rails_compact_blank.md |  1 +
 lib/rubocop/cop/rails/compact_blank.rb         |  9 +++------
 spec/rubocop/cop/rails/compact_blank_spec.rb   | 18 ++++--------------
 3 files changed, 8 insertions(+), 20 deletions(-)
 create mode 100644 changelog/fix_false_positive_for_rails_compact_blank.md

diff --git a/changelog/fix_false_positive_for_rails_compact_blank.md b/changelog/fix_false_positive_for_rails_compact_blank.md
new file mode 100644
index 0000000000..9718f82438
--- /dev/null
+++ b/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][])
diff --git a/lib/rubocop/cop/rails/compact_blank.rb b/lib/rubocop/cop/rails/compact_blank.rb
index 24753bb96b..a6f0f0860a 100644
--- a/lib/rubocop/cop/rails/compact_blank.rb
+++ b/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
diff --git a/spec/rubocop/cop/rails/compact_blank_spec.rb b/spec/rubocop/cop/rails/compact_blank_spec.rb
index 3c0390a150..93d1ca2bd7 100644
--- a/spec/rubocop/cop/rails/compact_blank_spec.rb
+++ b/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