Skip to content

Commit 4a16e3a

Browse files
masato-bknmasato-bkn
committed
[Fix #1016] Add Rails/RedundantAll cop
1 parent 6f1a411 commit 4a16e3a

File tree

5 files changed

+341
-0
lines changed

5 files changed

+341
-0
lines changed

‎changelog/fix_add_rails_redundant_all_cop.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1016](https://github.com/rubocop/rubocop-rails/issues/1016): Add `Rails/RedundantAll` cop. ([@masato-bkn][])

‎config/default.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,12 @@ Rails/ReadWriteAttribute:
774774
Include:
775775
- app/models/**/*.rb
776776

777+
Rails/RedundantAll:
778+
Description: Detect redundant `all` used as a receiver of Active Record query method.
779+
Enabled: true
780+
Safe: false
781+
VersionAdded: "<<next>>"
782+
777783
Rails/RedundantAllowNil:
778784
Description: >-
779785
Finds redundant use of `allow_nil` when `allow_blank` is set to

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

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# Detect redundant `all` used as a receiver of Active Record query method.
7+
#
8+
# @safety
9+
# This cop is unsafe if the receiver object is not an Active Record object.
10+
#
11+
# @example
12+
# # bad
13+
# User.all.order(:created_at)
14+
# User.all.find(id)
15+
#
16+
# # good
17+
# User.order(:created_at)
18+
# User.find(id)
19+
class RedundantAll < Base
20+
include ActiveRecordHelper
21+
extend AutoCorrector
22+
23+
MSG = 'Redundant `all` detected'
24+
25+
RESTRICT_ON_SEND = [:all].freeze
26+
27+
# Defined methods in `ActiveRecord::Querying::QUERYING_METHODS` on activerecord 7.0.5.
28+
QUERYING_METHODS = %i[
29+
and
30+
annotate
31+
any?
32+
average
33+
calculate
34+
count
35+
create_or_find_by
36+
create_or_find_by!
37+
create_with
38+
delete_all
39+
delete_by
40+
destroy_all
41+
destroy_by
42+
distinct
43+
eager_load
44+
except
45+
excluding
46+
exists?
47+
extending
48+
extract_associated
49+
fifth
50+
fifth!
51+
find
52+
find_by
53+
find_by!
54+
find_each
55+
find_in_batches
56+
find_or_create_by
57+
find_or_create_by!
58+
find_or_initialize_by
59+
find_sole_by
60+
first
61+
first!
62+
first_or_create
63+
first_or_create!
64+
first_or_initialize
65+
forty_two
66+
forty_two!
67+
fourth
68+
fourth!
69+
from
70+
group
71+
having
72+
ids
73+
in_batches
74+
in_order_of
75+
includes
76+
invert_where
77+
joins
78+
last
79+
last!
80+
left_joins
81+
left_outer_joins
82+
limit
83+
lock
84+
many?
85+
maximum
86+
merge
87+
minimum
88+
none
89+
none?
90+
offset
91+
one?
92+
only
93+
optimizer_hints
94+
or
95+
order
96+
pick
97+
pluck
98+
preload
99+
readonly
100+
references
101+
reorder
102+
reselect
103+
rewhere
104+
second
105+
second!
106+
second_to_last
107+
second_to_last!
108+
select
109+
sole
110+
strict_loading
111+
sum
112+
take
113+
take!
114+
third
115+
third!
116+
third_to_last
117+
third_to_last!
118+
touch_all
119+
unscope
120+
update_all
121+
where
122+
without
123+
].freeze
124+
125+
def on_send(node)
126+
return if node.receiver.nil? && !inherit_active_record_base?(node)
127+
128+
query_node = node.parent
129+
return unless query_node&.send_type?
130+
return unless QUERYING_METHODS.include?(query_node.method_name)
131+
132+
all_range = node.loc.selector
133+
add_offense(all_range) do |collector|
134+
collector.remove(all_range)
135+
collector.remove(query_node.loc.dot)
136+
end
137+
end
138+
end
139+
end
140+
end
141+
end

‎lib/rubocop/cop/rails_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@
8787
require_relative 'rails/present'
8888
require_relative 'rails/rake_environment'
8989
require_relative 'rails/read_write_attribute'
90+
require_relative 'rails/redundant_all'
9091
require_relative 'rails/redundant_allow_nil'
9192
require_relative 'rails/redundant_foreign_key'
9293
require_relative 'rails/redundant_presence_validation_on_belongs_to'

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

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Rails::RedundantAll, :config do
4+
sources = [
5+
'and(User.where(age: 30))',
6+
'annotate("selecting id")',
7+
'any?',
8+
'average(:age)',
9+
'calculate(:average, :age)',
10+
'count',
11+
'create_or_find_by(name: name)',
12+
'create_or_find_by!(name: name)',
13+
'create_with(name: name)',
14+
'delete_all',
15+
'delete_by(id: id)',
16+
'destroy_all',
17+
'destroy_by(id: id)',
18+
'distinct',
19+
'eager_load(:posts)',
20+
'except(:order)',
21+
'excluding(user)',
22+
'exists?',
23+
'extending(Pagination)',
24+
'extract_associated(:posts)',
25+
'fifth',
26+
'fifth!',
27+
'find(id)',
28+
'find_by(name: name)',
29+
'find_by!(name: name)',
30+
'find_each(&:do_something)',
31+
'find_in_batches(&:do_something)',
32+
'find_or_create_by(name: name)',
33+
'find_or_create_by!(name: name)',
34+
'find_or_initialize_by(name: name)',
35+
'find_sole_by(name: name)',
36+
'first',
37+
'first!',
38+
'first_or_create(name: name)',
39+
'first_or_create!(name: name)',
40+
'first_or_initialize(name: name)',
41+
'forty_two',
42+
'forty_two!',
43+
'fourth',
44+
'fourth!',
45+
'from("users")',
46+
'group(:age)',
47+
'having("AVG(age) > 30")',
48+
'ids',
49+
'in_batches(&:do_something)',
50+
'in_order_of(:id, ids)',
51+
'includes(:posts)',
52+
'invert_where',
53+
'joins(:posts)',
54+
'last',
55+
'last!',
56+
'left_joins(:posts)',
57+
'left_outer_joins(:posts)',
58+
'limit(n)',
59+
'lock',
60+
'many?',
61+
'maximum(:age)',
62+
'merge(users)',
63+
'minimum(:age)',
64+
'none',
65+
'none?',
66+
'offset(n)',
67+
'one?',
68+
'only(:order)',
69+
'optimizer_hints("SeqScan(users)", "Parallel(users 8)")',
70+
'or(User.where(age: 30))',
71+
'order(:created_at)',
72+
'pick(:id)',
73+
'pluck(:age)',
74+
'preload(:posts)',
75+
'readonly',
76+
'references(:posts)',
77+
'reorder(:created_at)',
78+
'reselect(:age)',
79+
'rewhere(id: ids)',
80+
'second',
81+
'second!',
82+
'second_to_last',
83+
'second_to_last!',
84+
'select(:age)',
85+
'sole',
86+
'strict_loading',
87+
'sum(:age)',
88+
'take(n)',
89+
'take!',
90+
'third',
91+
'third!',
92+
'third_to_last',
93+
'third_to_last!',
94+
'touch_all',
95+
'unscope(:order)',
96+
'update_all(name: name)',
97+
'where(id: ids)',
98+
'without(user)'
99+
]
100+
101+
describe '::QUERYING_METHODS' do
102+
it 'contains all of the querying methods' do
103+
methods = sources.map { |s| s.split('(')[0].to_sym }
104+
expect(described_class::QUERYING_METHODS).to eq(methods)
105+
end
106+
end
107+
108+
context 'with receiver' do
109+
sources.each do |source|
110+
it "registers an offense and corrects when `all.#{source}`" do
111+
expect_offense(<<~RUBY)
112+
User.all.#{source}
113+
^^^ Redundant `all` detected
114+
RUBY
115+
116+
expect_correction(<<~RUBY)
117+
User.#{source}
118+
RUBY
119+
end
120+
end
121+
122+
it 'does not register an offense when not using any method' do
123+
expect_no_offenses(<<~RUBY)
124+
User.all
125+
RUBY
126+
end
127+
128+
it 'does not register an offense when not using defined methods in ::QUERYING_METHODS' do
129+
expect_no_offenses(<<~RUBY)
130+
User.all.map(&:do_something)
131+
RUBY
132+
end
133+
end
134+
135+
context 'with no receiver' do
136+
it 'does not register an offense when not inheriting any class' do
137+
expect_no_offenses(<<~RUBY)
138+
class User
139+
def do_something
140+
all.where(id: ids)
141+
end
142+
end
143+
RUBY
144+
end
145+
146+
it 'does not register an offense when not inheriting `ApplicationRecord`' do
147+
expect_no_offenses(<<~RUBY)
148+
class User < Foo
149+
def do_something
150+
all.where(id: ids)
151+
end
152+
end
153+
RUBY
154+
end
155+
156+
it 'registers an offense when inheriting `ApplicationRecord`' do
157+
expect_offense(<<~RUBY)
158+
class User < ApplicationRecord
159+
scope :admins, -> { all.where(admin: true) }
160+
^^^ Redundant `all` detected
161+
end
162+
RUBY
163+
end
164+
165+
it 'registers an offense when inheriting `::ApplicationRecord`' do
166+
expect_offense(<<~RUBY)
167+
class User < ::ApplicationRecord
168+
scope :admins, -> { all.where(admin: true) }
169+
^^^ Redundant `all` detected
170+
end
171+
RUBY
172+
end
173+
174+
it 'registers an offense when inheriting `ActiveRecord::Base`' do
175+
expect_offense(<<~RUBY)
176+
class User < ActiveRecord::Base
177+
scope :admins, -> { all.where(admin: true) }
178+
^^^ Redundant `all` detected
179+
end
180+
RUBY
181+
end
182+
183+
it 'registers an offense when inheriting `::ActiveRecord::Base`' do
184+
expect_offense(<<~RUBY)
185+
class User < ::ActiveRecord::Base
186+
scope :admins, -> { all.where(admin: true) }
187+
^^^ Redundant `all` detected
188+
end
189+
RUBY
190+
end
191+
end
192+
end

0 commit comments

Comments
 (0)