Make Style/CollectionCompact aware of delete_if

This PR makes `Style/CollectionCompact` aware of `delete_if`. Note, `Enumerator` does not have `delete_if`, the following use case are allowed: ```ruby % ruby -ve '[1, 2, 3].to_enum.delete_if { |e| e.nil? }' ruby 3.3.0dev (2023-03-17T00:50:41Z master f29c9d6d36) [x86_64-darwin19] -e:1:in `<main>': undefined method `delete_if' for an instance of Enumerator (NoMethodError) [1, 2, 3].to_enum.delete_if { |e| e.nil? } ^^^^^^^^^^ ``` It doesn't register an offense as well as `to_enum` when `lazy` is used.
rubocop · Apr 24, 2023 · dcb7d4a · dcb7d4a
1 parent 95426a2
commit dcb7d4a
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 6 deletions.
diff --git a/changelog/new_make_collection_compact_aware_of_delete_if.md b/changelog/new_make_collection_compact_aware_of_delete_if.md
@@ -0,0 +1 @@
+* [#11814](https://github.com/rubocop/rubocop/pull/11814): Make `Style/CollectionCompact` aware of `delete_if`. ([@koic][])
diff --git a/lib/rubocop/cop/style/collection_compact.rb b/lib/rubocop/cop/style/collection_compact.rb
@@ -9,8 +9,8 @@ module Style
       # @safety
       #   It is unsafe by default because false positives may occur in the
       #   `nil` check of block arguments to the receiver object. Additionally,
-      #    we can't know the type of the receiver object for sure, which may
-      #    result in false positives as well.
+      #   we can't know the type of the receiver object for sure, which may
+      #   result in false positives as well.
       #
       #   For example, `[[1, 2], [3, nil]].reject { |first, second| second.nil? }`
       #   and `[[1, 2], [3, nil]].compact` are not compatible. This will work fine
@@ -19,7 +19,9 @@ 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? }
       #
       #   # good
@@ -39,14 +41,14 @@ class CollectionCompact < Base
         extend TargetRubyVersion
 
         MSG = 'Use `%<good>s` instead of `%<bad>s`.'
-        RESTRICT_ON_SEND = %i[reject reject! select select!].freeze
+        RESTRICT_ON_SEND = %i[reject delete_if reject! select select!].freeze
         TO_ENUM_METHODS = %i[to_enum lazy].freeze
 
         minimum_target_ruby_version 2.4
 
         # @!method reject_method_with_block_pass?(node)
         def_node_matcher :reject_method_with_block_pass?, <<~PATTERN
-          (send !nil? {:reject :reject!}
+          (send !nil? {:reject :delete_if :reject!}
             (block_pass
               (sym :nil?)))
         PATTERN
@@ -55,7 +57,7 @@ class CollectionCompact < Base
         def_node_matcher :reject_method?, <<~PATTERN
           (block
             (send
-              !nil? {:reject :reject!})
+              !nil? {:reject :delete_if :reject!})
             $(args ...)
             (send
               $(lvar _) :nil?))
@@ -74,7 +76,9 @@ class CollectionCompact < Base
 
         def on_send(node)
           return unless (range = offense_range(node))
-          return if target_ruby_version <= 3.0 && to_enum_method?(node)
+          if (target_ruby_version <= 3.0 || node.method?(:delete_if)) && to_enum_method?(node)
+            return
+          end
 
           good = good_method_name(node)
           message = format(MSG, good: good, bad: range.source)

diff --git a/spec/rubocop/cop/style/collection_compact_spec.rb b/spec/rubocop/cop/style/collection_compact_spec.rb
@@ -5,11 +5,14 @@
     expect_offense(<<~RUBY)
       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? }`.
       array.reject! { |e| e.nil? }
             ^^^^^^^^^^^^^^^^^^^^^^ Use `compact!` instead of `reject! { |e| e.nil? }`.
     RUBY
 
     expect_correction(<<~RUBY)
+      array.compact
       array.compact
       array.compact!
     RUBY
@@ -19,11 +22,14 @@
     expect_offense(<<~RUBY)
       array.reject(&:nil?)
             ^^^^^^^^^^^^^^ Use `compact` instead of `reject(&:nil?)`.
+      array.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!
     RUBY
@@ -33,22 +39,28 @@
     expect_offense(<<~RUBY)
       array.reject &:nil?
             ^^^^^^^^^^^^^ Use `compact` instead of `reject &:nil?`.
+      array.delete_if &:nil?
+            ^^^^^^^^^^^^^^^^ Use `compact` instead of `delete_if &:nil?`.
     RUBY
 
     expect_correction(<<~RUBY)
       array.compact
+      array.compact
     RUBY
   end
 
   it 'registers an offense and corrects when using `reject` on hash to reject nils' do
     expect_offense(<<~RUBY)
       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? }`.
       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!
     RUBY
@@ -73,19 +85,23 @@
       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? }`.
       end
     RUBY
 
     expect_correction(<<~RUBY)
       def foo(params)
         params.compact
+        params.compact
       end
     RUBY
   end
 
   it 'does not register an offense when using `reject` to not to rejecting nils' do
     expect_no_offenses(<<~RUBY)
       array.reject { |e| e.odd? }
+      array.delete_if { |e| e.odd? }
     RUBY
   end
 
@@ -100,6 +116,7 @@ def foo(params)
     it 'does not register an offense and corrects when using `reject` on array to reject nils' do
       expect_no_offenses(<<~RUBY)
         reject { |e| e.nil? }
+        delete_if { |e| e.nil? }
         reject! { |e| e.nil? }
       RUBY
     end
@@ -127,6 +144,12 @@ def foo(params)
       RUBY
     end
 
+    it 'does not register an offense and corrects when using `to_enum.delete_if` on array to reject nils' do
+      expect_no_offenses(<<~RUBY)
+        array.to_enum.delete_if { |e| e.nil? }
+      RUBY
+    end
+
     it 'registers an offense and corrects when using `lazy.reject` on array to reject nils' do
       expect_offense(<<~RUBY)
         array.lazy.reject { |e| e.nil? }
@@ -140,19 +163,27 @@ def foo(params)
         array.lazy.compact!
       RUBY
     end
+
+    it 'does not register an offense and corrects when using `lazy.delete_if` on array to reject nils' do
+      expect_no_offenses(<<~RUBY)
+        array.lazy.delete_if { |e| e.nil? }
+      RUBY
+    end
   end
 
   context 'Ruby <= 3.0', :ruby30 do
     it 'does not register an offense and corrects when using `to_enum.reject` on array to reject nils' do
       expect_no_offenses(<<~RUBY)
         array.to_enum.reject { |e| e.nil? }
+        array.to_enum.delete_if { |e| e.nil? }
         array.to_enum.reject! { |e| e.nil? }
       RUBY
     end
 
     it 'does not register an offense and corrects when using `lazy.reject` on array to reject nils' do
       expect_no_offenses(<<~RUBY)
         array.lazy.reject { |e| e.nil? }
+        array.lazy.delete_if { |e| e.nil? }
         array.lazy.reject! { |e| e.nil? }
       RUBY
     end