Merge pull request #1310 from fatkodima/compact_blank-select_present

Change `Rails/CompactBlank` to handle `select(&:present?)`

Author: koic

changelog/change_compact_blank_to_handle_select_present.md

@@ -0,0 +1 @@
+* [#1308](https://github.com/rubocop/rubocop-rails/issues/1308): Change `Rails/CompactBlank` to handle `select(&:present?)`. ([@fatkodima][])

lib/rubocop/cop/rails/compact_blank.rb

@@ -24,6 +24,8 @@ module Rails
       #   # bad
       #   collection.reject(&:blank?)
       #   collection.reject { |_k, v| v.blank? }
+      #   collection.select(&:present?)
+      #   collection.select { |_k, v| v.present? }
       #
       #   # good
       #   collection.compact_blank
@@ -33,6 +35,8 @@ module Rails
       #   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!`
       #
       #   # good
       #   collection.compact_blank!
@@ -43,7 +47,7 @@ class CompactBlank < Base
         extend TargetRailsVersion
 
         MSG = 'Use `%<preferred_method>s` instead.'
-        RESTRICT_ON_SEND = %i[reject delete_if reject!].freeze
+        RESTRICT_ON_SEND = %i[reject delete_if reject! select keep_if].freeze
 
         minimum_target_rails_version 6.1
 
@@ -61,6 +65,20 @@ class CompactBlank < Base
               (sym :blank?)))
         PATTERN
 
+        def_node_matcher :select_with_block?, <<~PATTERN
+          (block
+            (send _ {:select :keep_if})
+            $(args ...)
+            (send
+              $(lvar _) :present?))
+        PATTERN
+
+        def_node_matcher :select_with_block_pass?, <<~PATTERN
+          (send _ {:select :keep_if}
+            (block-pass
+              (sym :present?)))
+        PATTERN
+
         def on_send(node)
           return unless bad_method?(node)
 
@@ -75,8 +93,10 @@ def on_send(node)
 
         def bad_method?(node)
           return true if reject_with_block_pass?(node)
+          return true if select_with_block_pass?(node)
 
-          if (arguments, receiver_in_block = reject_with_block?(node.parent))
+          arguments, receiver_in_block = reject_with_block?(node.parent) || select_with_block?(node.parent)
+          if arguments
             return use_single_value_block_argument?(arguments, receiver_in_block) ||
                    use_hash_value_block_argument?(arguments, receiver_in_block)
           end
@@ -103,7 +123,7 @@ def offense_range(node)
         end
 
         def preferred_method(node)
-          node.method?(:reject) ? 'compact_blank' : 'compact_blank!'
+          node.method?(:reject) || node.method?(:select) ? 'compact_blank' : 'compact_blank!'
         end
       end
     end

spec/rubocop/cop/rails/compact_blank_spec.rb

@@ -79,6 +79,62 @@
       RUBY
     end
 
+    it 'registers and corrects an offense when using `select { |e| e.present? }`' do
+      expect_offense(<<~RUBY)
+        collection.select { |e| e.present? }
+                   ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank` instead.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection.compact_blank
+      RUBY
+    end
+
+    it 'registers and corrects an offense when using `select(&:present?)`' do
+      expect_offense(<<~RUBY)
+        collection.select(&:present?)
+                   ^^^^^^^^^^^^^^^^^^ Use `compact_blank` instead.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection.compact_blank
+      RUBY
+    end
+
+    it 'registers and corrects an offense when using `keep_if { |e| e.present? }`' do
+      expect_offense(<<~RUBY)
+        collection.keep_if { |e| e.present? }
+                   ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection.compact_blank!
+      RUBY
+    end
+
+    it 'registers and corrects an offense when using `keep_if(&:present?)`' do
+      expect_offense(<<~RUBY)
+        collection.keep_if(&:present?)
+                   ^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        collection.compact_blank!
+      RUBY
+    end
+
+    it 'does not register an offense when using `select! { |e| e.present? }`' do
+      expect_no_offenses(<<~RUBY)
+        collection.select! { |e| e.present? }
+      RUBY
+    end
+
+    it 'does not register an offense when using `select!(&:present?)`' do
+      expect_no_offenses(<<~RUBY)
+        collection.select!(&:present?)
+      RUBY
+    end
+
     it 'does not register an offense when using `compact_blank`' do
       expect_no_offenses(<<~RUBY)
         collection.compact_blank
@@ -110,6 +166,12 @@ def foo(arg)
         collection.reject { |e| e.empty? }
       RUBY
     end
+
+    it 'does not register an offense when using `select { |e| e.blank? }`' do
+      expect_no_offenses(<<~RUBY)
+        collection.select { |e| e.blank? }
+      RUBY
+    end
   end
 
   context 'Rails <= 6.0', :rails60 do
@@ -124,5 +186,11 @@ def foo(arg)
         collection.reject { |e| e.empty? }
       RUBY
     end
+
+    it 'does not register an offense when using `select { |e| e.present? }`' do
+      expect_no_offenses(<<~RUBY)
+        collection.select { |e| e.present? }
+      RUBY
+    end
   end
 end