Skip to content

Commit

Permalink
[Fix #562] Add new Rails/CompactBlank cop
Browse files Browse the repository at this point in the history
Fixes #562.

This cop checks for places where custom logic on rejection blanks
from arrays and hashes can be replaced with `compact_blank`.

```ruby
# bad
collection.reject { |e| e.blank? }
collection.reject { |e| e.empty? }

# good
collection.compact_blank

# bad
collection.reject! { |e| e.blank? }
collection.reject! { |e| e.empty? }

# good
collection.compact_blank!
```
  • Loading branch information
koic committed Dec 21, 2021
1 parent 584deee commit c7f39c4
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 0 deletions.
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@ Rails/BulkChangeTable:
Include:
- db/migrate/*.rb

Rails/CompactBlank:
Description: 'Checks if collection can be blank-compacted with `compact_blank`.'
Enabled: pending
VersionAdded: '<<next>>'

Rails/ContentTag:
Description: 'Use `tag.something` instead of `tag(:something)`.'
Reference:
Expand Down
90 changes: 90 additions & 0 deletions lib/rubocop/cop/rails/compact_blank.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# Checks if collection can be blank-compacted with `compact_blank`.
#
# @example
#
# # bad
# collection.reject(&:blank?)
# collection.reject(&:empty?)
# collection.reject { |_k, v| v.blank? }
# collection.reject { |_k, v| v.empty? }
#
# # good
# collection.compact_blank
#
# # bad
# collection.reject!(&:blank?)
# collection.reject!(&:empty?)
# collection.reject! { |_k, v| v.blank? }
# collection.reject! { |_k, v| v.empty? }
#
# # good
# collection.compact_blank!
#
class CompactBlank < Base
include RangeHelp
extend AutoCorrector
extend TargetRailsVersion

MSG = 'Use `%<preferred_method>s` instead.'
RESTRICT_ON_SEND = %i[reject reject!].freeze

minimum_target_rails_version 6.1

def_node_matcher :reject_with_block?, <<~PATTERN
(block
(send _ {:reject :reject!})
$(args ...)
(send
$(lvar _) {:blank? :empty?}))
PATTERN

def_node_matcher :reject_with_block_pass?, <<~PATTERN
(send _ {:reject :reject!}
(block_pass
(sym {:blank? :empty?})))
PATTERN

def on_send(node)
return unless bad_method?(node)

range = offense_range(node)
preferred_method = preferred_method(node)
add_offense(range, message: format(MSG, preferred_method: preferred_method)) do |corrector|
corrector.replace(range, preferred_method)
end
end

private

def bad_method?(node)
return true if reject_with_block_pass?(node)

if (arguments, receiver_in_block = reject_with_block?(node.parent))
return arguments.length == 1 || use_hash_value_block_argument?(arguments, receiver_in_block)
end

false
end

def use_hash_value_block_argument?(arguments, receiver_in_block)
arguments.length == 2 && arguments[1].source == receiver_in_block.source
end

def offense_range(node)
end_pos = node.parent&.block_type? ? node.parent.loc.expression.end_pos : node.loc.expression.end_pos

range_between(node.loc.selector.begin_pos, end_pos)
end

def preferred_method(node)
node.method?(:reject) ? 'compact_blank' : 'compact_blank!'
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
require_relative 'rails/belongs_to'
require_relative 'rails/blank'
require_relative 'rails/bulk_change_table'
require_relative 'rails/compact_blank'
require_relative 'rails/content_tag'
require_relative 'rails/create_table_with_timestamps'
require_relative 'rails/date'
Expand Down
136 changes: 136 additions & 0 deletions spec/rubocop/cop/rails/compact_blank_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::CompactBlank, :config do
context 'Rails >= 6.1', :rails61 do
it 'registers and corrects an offense when using `reject { |e| e.blank? }`' do
expect_offense(<<~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 { |e| e.empty? }`' do
expect_offense(<<~RUBY)
collection.reject { |e| e.empty? }
^^^^^^^^^^^^^^^^^^^^^^^ 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)
collection.reject(&:blank?)
^^^^^^^^^^^^^^^^ Use `compact_blank` instead.
RUBY

expect_correction(<<~RUBY)
collection.compact_blank
RUBY
end

it 'registers and corrects an offense when using `reject(&:empty?)`' do
expect_offense(<<~RUBY)
collection.reject(&:empty?)
^^^^^^^^^^^^^^^^ Use `compact_blank` instead.
RUBY

expect_correction(<<~RUBY)
collection.compact_blank
RUBY
end

it 'registers and corrects an offense when using `reject! { |e| e.blank? }`' do
expect_offense(<<~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! { |e| e.empty? }`' do
expect_offense(<<~RUBY)
collection.reject! { |e| e.empty? }
^^^^^^^^^^^^^^^^^^^^^^^^ 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)
collection.reject!(&:blank?)
^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
RUBY

expect_correction(<<~RUBY)
collection.compact_blank!
RUBY
end

it 'registers and corrects an offense when using `reject!(&:empty?)`' do
expect_offense(<<~RUBY)
collection.reject!(&:empty?)
^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
RUBY

expect_correction(<<~RUBY)
collection.compact_blank!
RUBY
end

it 'registers and corrects an offense when using `reject { |k, v| v.empty? }`' do
expect_offense(<<~RUBY)
collection.reject { |k, v| v.empty? }
^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank` instead.
RUBY

expect_correction(<<~RUBY)
collection.compact_blank
RUBY
end

it 'does not register an offense when using `compact_blank`' do
expect_no_offenses(<<~RUBY)
collection.compact_blank
RUBY
end

it 'does not register an offense when using `compact_blank!`' do
expect_no_offenses(<<~RUBY)
collection.compact_blank!
RUBY
end

it 'does not register an offense when using `reject { |k, v| k.empty? }`' do
expect_no_offenses(<<~RUBY)
collection.reject { |k, v| k.empty? }
RUBY
end
end

context 'Rails <= 6.0', :rails60 do
it 'does not register an offense when using `reject { |e| e.blank? }`' do
expect_no_offenses(<<~RUBY)
collection.reject { |e| e.blank? }
RUBY
end

it 'does not register an offense when using `reject { |e| e.empty? }`' do
expect_no_offenses(<<~RUBY)
collection.reject { |e| e.empty? }
RUBY
end
end
end

0 comments on commit c7f39c4

Please sign in to comment.