Skip to content

Commit a80ac3b

Browse files
koickoic
authored
Merge pull request #598 from koic/add_new_rails_compact_blank_cop
[Fix #562] Add new `Rails/CompactBlank` cop
2 parents 37e882f + c7f39c4 commit a80ac3b

File tree

4 files changed

+232
-0
lines changed

4 files changed

+232
-0
lines changed

‎config/default.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,11 @@ Rails/BulkChangeTable:
180180
Include:
181181
- db/migrate/*.rb
182182

183+
Rails/CompactBlank:
184+
Description: 'Checks if collection can be blank-compacted with `compact_blank`.'
185+
Enabled: pending
186+
VersionAdded: '<<next>>'
187+
183188
Rails/ContentTag:
184189
Description: 'Use `tag.something` instead of `tag(:something)`.'
185190
Reference:

‎lib/rubocop/cop/rails/compact_blank.rb

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# Checks if collection can be blank-compacted with `compact_blank`.
7+
#
8+
# @example
9+
#
10+
# # bad
11+
# collection.reject(&:blank?)
12+
# collection.reject(&:empty?)
13+
# collection.reject { |_k, v| v.blank? }
14+
# collection.reject { |_k, v| v.empty? }
15+
#
16+
# # good
17+
# collection.compact_blank
18+
#
19+
# # bad
20+
# collection.reject!(&:blank?)
21+
# collection.reject!(&:empty?)
22+
# collection.reject! { |_k, v| v.blank? }
23+
# collection.reject! { |_k, v| v.empty? }
24+
#
25+
# # good
26+
# collection.compact_blank!
27+
#
28+
class CompactBlank < Base
29+
include RangeHelp
30+
extend AutoCorrector
31+
extend TargetRailsVersion
32+
33+
MSG = 'Use `%<preferred_method>s` instead.'
34+
RESTRICT_ON_SEND = %i[reject reject!].freeze
35+
36+
minimum_target_rails_version 6.1
37+
38+
def_node_matcher :reject_with_block?, <<~PATTERN
39+
(block
40+
(send _ {:reject :reject!})
41+
$(args ...)
42+
(send
43+
$(lvar _) {:blank? :empty?}))
44+
PATTERN
45+
46+
def_node_matcher :reject_with_block_pass?, <<~PATTERN
47+
(send _ {:reject :reject!}
48+
(block_pass
49+
(sym {:blank? :empty?})))
50+
PATTERN
51+
52+
def on_send(node)
53+
return unless bad_method?(node)
54+
55+
range = offense_range(node)
56+
preferred_method = preferred_method(node)
57+
add_offense(range, message: format(MSG, preferred_method: preferred_method)) do |corrector|
58+
corrector.replace(range, preferred_method)
59+
end
60+
end
61+
62+
private
63+
64+
def bad_method?(node)
65+
return true if reject_with_block_pass?(node)
66+
67+
if (arguments, receiver_in_block = reject_with_block?(node.parent))
68+
return arguments.length == 1 || use_hash_value_block_argument?(arguments, receiver_in_block)
69+
end
70+
71+
false
72+
end
73+
74+
def use_hash_value_block_argument?(arguments, receiver_in_block)
75+
arguments.length == 2 && arguments[1].source == receiver_in_block.source
76+
end
77+
78+
def offense_range(node)
79+
end_pos = node.parent&.block_type? ? node.parent.loc.expression.end_pos : node.loc.expression.end_pos
80+
81+
range_between(node.loc.selector.begin_pos, end_pos)
82+
end
83+
84+
def preferred_method(node)
85+
node.method?(:reject) ? 'compact_blank' : 'compact_blank!'
86+
end
87+
end
88+
end
89+
end
90+
end

‎lib/rubocop/cop/rails_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
require_relative 'rails/belongs_to'
2424
require_relative 'rails/blank'
2525
require_relative 'rails/bulk_change_table'
26+
require_relative 'rails/compact_blank'
2627
require_relative 'rails/content_tag'
2728
require_relative 'rails/create_table_with_timestamps'
2829
require_relative 'rails/date'

‎spec/rubocop/cop/rails/compact_blank_spec.rb

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Rails::CompactBlank, :config do
4+
context 'Rails >= 6.1', :rails61 do
5+
it 'registers and corrects an offense when using `reject { |e| e.blank? }`' do
6+
expect_offense(<<~RUBY)
7+
collection.reject { |e| e.blank? }
8+
^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank` instead.
9+
RUBY
10+
11+
expect_correction(<<~RUBY)
12+
collection.compact_blank
13+
RUBY
14+
end
15+
16+
it 'registers and corrects an offense when using `reject { |e| e.empty? }`' do
17+
expect_offense(<<~RUBY)
18+
collection.reject { |e| e.empty? }
19+
^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank` instead.
20+
RUBY
21+
22+
expect_correction(<<~RUBY)
23+
collection.compact_blank
24+
RUBY
25+
end
26+
27+
it 'registers and corrects an offense when using `reject(&:blank?)`' do
28+
expect_offense(<<~RUBY)
29+
collection.reject(&:blank?)
30+
^^^^^^^^^^^^^^^^ Use `compact_blank` instead.
31+
RUBY
32+
33+
expect_correction(<<~RUBY)
34+
collection.compact_blank
35+
RUBY
36+
end
37+
38+
it 'registers and corrects an offense when using `reject(&:empty?)`' do
39+
expect_offense(<<~RUBY)
40+
collection.reject(&:empty?)
41+
^^^^^^^^^^^^^^^^ Use `compact_blank` instead.
42+
RUBY
43+
44+
expect_correction(<<~RUBY)
45+
collection.compact_blank
46+
RUBY
47+
end
48+
49+
it 'registers and corrects an offense when using `reject! { |e| e.blank? }`' do
50+
expect_offense(<<~RUBY)
51+
collection.reject! { |e| e.blank? }
52+
^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
53+
RUBY
54+
55+
expect_correction(<<~RUBY)
56+
collection.compact_blank!
57+
RUBY
58+
end
59+
60+
it 'registers and corrects an offense when using `reject! { |e| e.empty? }`' do
61+
expect_offense(<<~RUBY)
62+
collection.reject! { |e| e.empty? }
63+
^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
64+
RUBY
65+
66+
expect_correction(<<~RUBY)
67+
collection.compact_blank!
68+
RUBY
69+
end
70+
71+
it 'registers and corrects an offense when using `reject!(&:blank?)`' do
72+
expect_offense(<<~RUBY)
73+
collection.reject!(&:blank?)
74+
^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
75+
RUBY
76+
77+
expect_correction(<<~RUBY)
78+
collection.compact_blank!
79+
RUBY
80+
end
81+
82+
it 'registers and corrects an offense when using `reject!(&:empty?)`' do
83+
expect_offense(<<~RUBY)
84+
collection.reject!(&:empty?)
85+
^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead.
86+
RUBY
87+
88+
expect_correction(<<~RUBY)
89+
collection.compact_blank!
90+
RUBY
91+
end
92+
93+
it 'registers and corrects an offense when using `reject { |k, v| v.empty? }`' do
94+
expect_offense(<<~RUBY)
95+
collection.reject { |k, v| v.empty? }
96+
^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank` instead.
97+
RUBY
98+
99+
expect_correction(<<~RUBY)
100+
collection.compact_blank
101+
RUBY
102+
end
103+
104+
it 'does not register an offense when using `compact_blank`' do
105+
expect_no_offenses(<<~RUBY)
106+
collection.compact_blank
107+
RUBY
108+
end
109+
110+
it 'does not register an offense when using `compact_blank!`' do
111+
expect_no_offenses(<<~RUBY)
112+
collection.compact_blank!
113+
RUBY
114+
end
115+
116+
it 'does not register an offense when using `reject { |k, v| k.empty? }`' do
117+
expect_no_offenses(<<~RUBY)
118+
collection.reject { |k, v| k.empty? }
119+
RUBY
120+
end
121+
end
122+
123+
context 'Rails <= 6.0', :rails60 do
124+
it 'does not register an offense when using `reject { |e| e.blank? }`' do
125+
expect_no_offenses(<<~RUBY)
126+
collection.reject { |e| e.blank? }
127+
RUBY
128+
end
129+
130+
it 'does not register an offense when using `reject { |e| e.empty? }`' do
131+
expect_no_offenses(<<~RUBY)
132+
collection.reject { |e| e.empty? }
133+
RUBY
134+
end
135+
end
136+
end

0 commit comments

Comments
 (0)