[Fix #12801] Fix incorrect autocorrect for Style/CollectionCompact

Fixes #12801. This PR fixes incorrect autocorrect for `Style/CollectionCompact` when using `delete_if`.
rubocop · Mar 21, 2024 · b28842e · b28842e
1 parent 3cc5ead
commit b28842e
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 17 deletions.
diff --git a/changelog/fix_incorrect_autocorrect_for_style_collection_compact.md b/changelog/fix_incorrect_autocorrect_for_style_collection_compact.md
@@ -0,0 +1 @@
+* [#12801](https://github.com/rubocop/rubocop/issues/12801): Fix incorrect autocorrect for `Style/CollectionCompact` when using `delete_if`. ([@koic][])
diff --git a/lib/rubocop/cop/style/collection_compact.rb b/lib/rubocop/cop/style/collection_compact.rb
@@ -19,9 +19,7 @@ module Style
       # @example
       #   # bad
       #   array.reject(&:nil?)
-      #   array.delete_if(&:nil?)
       #   array.reject { |e| e.nil? }
-      #   array.delete_if { |e| e.nil? }
       #   array.select { |e| !e.nil? }
       #   array.grep_v(nil)
       #   array.grep_v(NilClass)
@@ -31,7 +29,9 @@ module Style
       #
       #   # bad
       #   hash.reject!(&:nil?)
+      #   array.delete_if(&:nil?)
       #   hash.reject! { |k, v| v.nil? }
+      #   array.delete_if { |e| e.nil? }
       #   hash.select! { |k, v| !v.nil? }
       #
       #   # good
@@ -127,7 +127,7 @@ def to_enum_method?(node)
         end
 
         def good_method_name(node)
-          if node.bang_method?
+          if node.bang_method? || node.method?(:delete_if)
             'compact!'
           else
             'compact'

diff --git a/spec/rubocop/cop/style/collection_compact_spec.rb b/spec/rubocop/cop/style/collection_compact_spec.rb
@@ -6,14 +6,14 @@
       array.reject { |e| e.nil? }
             ^^^^^^^^^^^^^^^^^^^^^ Use `compact` instead of `reject { |e| e.nil? }`.
       array.delete_if { |e| e.nil? }
-            ^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact` instead of `delete_if { |e| e.nil? }`.
+            ^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact!` instead of `delete_if { |e| e.nil? }`.
       array.reject! { |e| e.nil? }
             ^^^^^^^^^^^^^^^^^^^^^^ Use `compact!` instead of `reject! { |e| e.nil? }`.
     RUBY
 
     expect_correction(<<~RUBY)
       array.compact
-      array.compact
+      array.compact!
       array.compact!
     RUBY
   end
@@ -23,14 +23,14 @@
       array&.reject { |e| e&.nil? }
              ^^^^^^^^^^^^^^^^^^^^^^ Use `compact` instead of `reject { |e| e&.nil? }`.
       array&.delete_if { |e| e&.nil? }
-             ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact` instead of `delete_if { |e| e&.nil? }`.
+             ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact!` instead of `delete_if { |e| e&.nil? }`.
       array&.reject! { |e| e&.nil? }
              ^^^^^^^^^^^^^^^^^^^^^^^ Use `compact!` instead of `reject! { |e| e&.nil? }`.
     RUBY
 
     expect_correction(<<~RUBY)
       array&.compact
-      array&.compact
+      array&.compact!
       array&.compact!
     RUBY
   end
@@ -40,14 +40,14 @@
       array.reject(&:nil?)
             ^^^^^^^^^^^^^^ Use `compact` instead of `reject(&:nil?)`.
       array.delete_if(&:nil?)
-            ^^^^^^^^^^^^^^^^^ Use `compact` instead of `delete_if(&:nil?)`.
+            ^^^^^^^^^^^^^^^^^ Use `compact!` instead of `delete_if(&:nil?)`.
       array.reject!(&:nil?)
             ^^^^^^^^^^^^^^^ Use `compact!` instead of `reject!(&:nil?)`.
     RUBY
 
     expect_correction(<<~RUBY)
       array.compact
-      array.compact
+      array.compact!
       array.compact!
     RUBY
   end
@@ -57,15 +57,15 @@
       array&.reject(&:nil?)
              ^^^^^^^^^^^^^^ Use `compact` instead of `reject(&:nil?)`.
       array&.delete_if(&:nil?)
-             ^^^^^^^^^^^^^^^^^ Use `compact` instead of `delete_if(&:nil?)`.
+             ^^^^^^^^^^^^^^^^^ Use `compact!` instead of `delete_if(&:nil?)`.
       array&.reject!(&:nil?)
              ^^^^^^^^^^^^^^^ Use `compact!` instead of `reject!(&:nil?)`.
     RUBY
 
     expect_correction(<<~RUBY)
-      array&.compact
       array&.compact
       array&.compact!
+      array&.compact!
     RUBY
   end
 
@@ -74,12 +74,12 @@
       array.reject &:nil?
             ^^^^^^^^^^^^^ Use `compact` instead of `reject &:nil?`.
       array.delete_if &:nil?
-            ^^^^^^^^^^^^^^^^ Use `compact` instead of `delete_if &:nil?`.
+            ^^^^^^^^^^^^^^^^ Use `compact!` instead of `delete_if &:nil?`.
     RUBY
 
     expect_correction(<<~RUBY)
       array.compact
-      array.compact
+      array.compact!
     RUBY
   end
 
@@ -88,14 +88,14 @@
       hash.reject { |k, v| v.nil? }
            ^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact` instead of `reject { |k, v| v.nil? }`.
       hash.delete_if { |k, v| v.nil? }
-           ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact` instead of `delete_if { |k, v| v.nil? }`.
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact!` instead of `delete_if { |k, v| v.nil? }`.
       hash.reject! { |k, v| v.nil? }
            ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact!` instead of `reject! { |k, v| v.nil? }`.
     RUBY
 
     expect_correction(<<~RUBY)
       hash.compact
-      hash.compact
+      hash.compact!
       hash.compact!
     RUBY
   end
@@ -134,14 +134,14 @@ def foo(params)
         params.reject { |_k, v| v.nil? }
                ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact` instead of `reject { |_k, v| v.nil? }`.
         params.delete_if { |_k, v| v.nil? }
-               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact` instead of `delete_if { |_k, v| v.nil? }`.
+               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact!` instead of `delete_if { |_k, v| v.nil? }`.
       end
     RUBY
 
     expect_correction(<<~RUBY)
       def foo(params)
         params.compact
-        params.compact
+        params.compact!
       end
     RUBY
   end
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		* [#12801](https://github.com/rubocop/rubocop/issues/12801): Fix incorrect autocorrect for `Style/CollectionCompact` when using `delete_if`. ([@koic][])