Skip to content

Commit 6fb055b

Browse files
Bhacazkoic
authored andcommitted
Added ReflectionClassName cop
This PR add a cop to check if the value of the option `class_name` of a [ActiveRecord::Reflection](https://apidock.com/rails/ActiveRecord/Reflection/AbstractReflection/class_name) is a string. _Why this cop:_ I was working in a Rails project where often there was a constant used for `class_name` value. That caused a issue which was to preload unnecessary models use in the definition of a reflection.
1 parent 50212ed commit 6fb055b

File tree

5 files changed

+108
-0
lines changed

5 files changed

+108
-0
lines changed

config/enabled.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ Rails/RedundantReceiverInWithOptions:
172172
Description: 'Checks for redundant receiver in `with_options`.'
173173
Enabled: true
174174

175+
Rails/ReflectionClassName:
176+
Description: 'Use a string for `class_name` option value in the definition of a reflection.'
177+
Enabled: true
178+
175179
Rails/RefuteMethods:
176180
Description: 'Use `assert_not` methods instead of `refute` methods.'
177181
Enabled: true
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# This cop checks if the value of the option `class_name`, in
7+
# the definition of a reflection is a string.
8+
#
9+
# @example
10+
# # bad
11+
# has_many :accounts, class_name: Account
12+
# has_many :accounts, class_name: Account.name
13+
#
14+
# # good
15+
# has_many :accounts, class_name: 'Account'
16+
class ReflectionClassName < Cop
17+
MSG = 'Use a string has value for a class_name.'.freeze
18+
19+
def_node_matcher :association_with_options?, <<-PATTERN
20+
(send nil? {:has_many :has_one :belongs_to} _ (hash $...))
21+
PATTERN
22+
23+
def_node_search :reflection_class_name, <<-PATTERN
24+
(pair (sym :class_name) !str)
25+
PATTERN
26+
27+
def on_send(node)
28+
return unless association_with_options?(node)
29+
30+
reflection_class_name = reflection_class_name(node).first
31+
return unless reflection_class_name
32+
33+
add_offense(node, location: reflection_class_name.loc.expression)
34+
end
35+
end
36+
end
37+
end
38+
end

lib/rubocop/cop/rails_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ module Cop
4545
require_relative 'rails/present'
4646
require_relative 'rails/read_write_attribute'
4747
require_relative 'rails/redundant_receiver_in_with_options'
48+
require_relative 'rails/reflection_class_name'
4849
require_relative 'rails/refute_methods'
4950
require_relative 'rails/relative_date_constant'
5051
require_relative 'rails/request_referer'

manual/cops_rails.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1608,6 +1608,26 @@ with_options options: false do |merger|
16081608
end
16091609
```
16101610

1611+
## Rails/ReflectionClassName
1612+
1613+
Enabled by default | Supports autocorrection
1614+
--- | ---
1615+
Enabled | No
1616+
1617+
This cop checks if the value of the option `class_name`, in
1618+
the definition of a reflection is a string.
1619+
1620+
### Examples
1621+
1622+
```ruby
1623+
# bad
1624+
has_many :accounts, class_name: Account
1625+
has_many :accounts, class_name: Account.name
1626+
1627+
# good
1628+
has_many :accounts, class_name: 'Account'
1629+
```
1630+
16111631
## Rails/RefuteMethods
16121632

16131633
Enabled by default | Supports autocorrection
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Rails::ReflectionClassName do
4+
subject(:cop) { described_class.new(config) }
5+
6+
let(:config) { RuboCop::Config.new }
7+
8+
context "registers an offense when using `foreign_key: 'account_id'`" do
9+
it 'has_many' do
10+
expect_offense(<<-RUBY.strip_indent)
11+
has_many :accounts, class_name: Account, foreign_key: :account_id
12+
^^^^^^^^^^^^^^^^^^^ Use a string has value for a class_name.
13+
RUBY
14+
end
15+
16+
it '.name' do
17+
expect_offense(<<-RUBY.strip_indent)
18+
has_many :accounts, class_name: Account.name
19+
^^^^^^^^^^^^^^^^^^^^^^^^ Use a string has value for a class_name.
20+
RUBY
21+
end
22+
23+
it 'has_one' do
24+
expect_offense(<<-RUBY.strip_indent)
25+
has_one :account, class_name: Account
26+
^^^^^^^^^^^^^^^^^^^ Use a string has value for a class_name.
27+
RUBY
28+
end
29+
30+
it 'belongs_to' do
31+
expect_offense(<<-RUBY.strip_indent)
32+
belongs_to :account, class_name: Account
33+
^^^^^^^^^^^^^^^^^^^ Use a string has value for a class_name.
34+
RUBY
35+
end
36+
end
37+
38+
it 'does not register an offense when using `foreign_key :account_id`' do
39+
expect_no_offenses(<<-RUBY.strip_indent)
40+
has_many :accounts, class_name: 'Account', foreign_key: :account_id
41+
has_one :account, class_name: 'Account'
42+
belongs_to :account, class_name: 'Account'
43+
RUBY
44+
end
45+
end

0 commit comments

Comments
 (0)