From df47686261fd4b9b4312b5b57365a1f80979977e Mon Sep 17 00:00:00 2001 From: fatkodima Date: Sat, 6 Jul 2024 16:47:46 +0300 Subject: [PATCH] Change `Rails/CompactBlank` to handle `select(&:present?)` --- ..._compact_blank_to_handle_select_present.md | 1 + lib/rubocop/cop/rails/compact_blank.rb | 26 ++++++- spec/rubocop/cop/rails/compact_blank_spec.rb | 68 +++++++++++++++++++ 3 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 changelog/change_compact_blank_to_handle_select_present.md diff --git a/changelog/change_compact_blank_to_handle_select_present.md b/changelog/change_compact_blank_to_handle_select_present.md new file mode 100644 index 0000000000..cec5b9b4f2 --- /dev/null +++ b/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][]) diff --git a/lib/rubocop/cop/rails/compact_blank.rb b/lib/rubocop/cop/rails/compact_blank.rb index 5ba028d55c..24753bb96b 100644 --- a/lib/rubocop/cop/rails/compact_blank.rb +++ b/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 `%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 diff --git a/spec/rubocop/cop/rails/compact_blank_spec.rb b/spec/rubocop/cop/rails/compact_blank_spec.rb index a1f459c892..3c0390a150 100644 --- a/spec/rubocop/cop/rails/compact_blank_spec.rb +++ b/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