Skip to content

Commit ea7a891

Browse files
koickoic
authored
Merge pull request #602 from natematykiewicz/duplicate_associations_cop
[Fix #599] Add Rails/DuplicateAssociation cop
2 parents cfa4bb9 + fabae13 commit ea7a891

File tree

5 files changed

+220
-0
lines changed

5 files changed

+220
-0
lines changed

‎changelog/new_add_railsduplicateassociation_cop.md

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

‎config/default.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,11 @@ Rails/DelegateAllowBlank:
251251
Enabled: true
252252
VersionAdded: '0.44'
253253

254+
Rails/DuplicateAssociation:
255+
Description: "Don't repeat associations in a model."
256+
Enabled: pending
257+
VersionAdded: '<<next>>'
258+
254259
Rails/DurationArithmetic:
255260
Description: 'Do not use duration as arithmetic operand with `Time.current`.'
256261
StyleGuide: 'https://rails.rubystyle.guide#duration-arithmetic'

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# This cop looks for associations that have been defined multiple times in the same file.
7+
#
8+
# When an association is defined multiple times on a model, Active Record overrides the
9+
# previously defined association with the new one. Because of this, this cop's autocorrection
10+
# simply keeps the last of any duplicates and discards the rest.
11+
#
12+
# @example
13+
#
14+
# # bad
15+
# belongs_to :foo
16+
# belongs_to :bar
17+
# has_one :foo
18+
#
19+
# # good
20+
# belongs_to :bar
21+
# has_one :foo
22+
#
23+
class DuplicateAssociation < Base
24+
include RangeHelp
25+
extend AutoCorrector
26+
27+
MSG = "Association `%<name>s` is defined multiple times. Don't repeat associations."
28+
29+
def_node_matcher :association, <<~PATTERN
30+
(send nil? {:belongs_to :has_one :has_many :has_and_belongs_to_many} ({sym str} $_) ...)
31+
PATTERN
32+
33+
def on_class(class_node)
34+
offenses(class_node).each do |name, nodes|
35+
nodes.each do |node|
36+
add_offense(node, message: format(MSG, name: name)) do |corrector|
37+
next if nodes.last == node
38+
39+
corrector.remove(range_by_whole_lines(node.source_range, include_final_newline: true))
40+
end
41+
end
42+
end
43+
end
44+
45+
private
46+
47+
def offenses(class_node)
48+
class_send_nodes(class_node).select { |node| association(node) }
49+
.group_by { |node| association(node).to_sym }
50+
.select { |_, nodes| nodes.length > 1 }
51+
end
52+
53+
def class_send_nodes(class_node)
54+
class_def = class_node.body
55+
56+
return [] unless class_def
57+
58+
if class_def.send_type?
59+
[class_def]
60+
else
61+
class_def.each_child_node(:send)
62+
end
63+
end
64+
end
65+
end
66+
end
67+
end

‎lib/rubocop/cop/rails_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
require_relative 'rails/default_scope'
3232
require_relative 'rails/delegate'
3333
require_relative 'rails/delegate_allow_blank'
34+
require_relative 'rails/duplicate_association'
3435
require_relative 'rails/duration_arithmetic'
3536
require_relative 'rails/dynamic_find_by'
3637
require_relative 'rails/eager_evaluation_log_message'

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

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Rails::DuplicateAssociation, :config do
4+
describe 'belongs_to' do
5+
it 'registers an offense' do
6+
expect_offense(<<~RUBY)
7+
class Post < ApplicationRecord
8+
belongs_to :foo
9+
^^^^^^^^^^^^^^^ Association `foo` is defined multiple times. Don't repeat associations.
10+
belongs_to :bar
11+
belongs_to :foo, -> { active }
12+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `foo` is defined multiple times. Don't repeat associations.
13+
belongs_to :blah
14+
end
15+
RUBY
16+
17+
expect_correction(<<~RUBY)
18+
class Post < ApplicationRecord
19+
belongs_to :bar
20+
belongs_to :foo, -> { active }
21+
belongs_to :blah
22+
end
23+
RUBY
24+
end
25+
end
26+
27+
describe 'has_many' do
28+
it 'registers an offense' do
29+
expect_offense(<<~RUBY)
30+
class Post < ApplicationRecord
31+
has_many :foos
32+
^^^^^^^^^^^^^^ Association `foos` is defined multiple times. Don't repeat associations.
33+
has_many :bars
34+
has_many 'foos', -> { active }
35+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `foos` is defined multiple times. Don't repeat associations.
36+
has_many :blahs
37+
end
38+
RUBY
39+
40+
expect_correction(<<~RUBY)
41+
class Post < ApplicationRecord
42+
has_many :bars
43+
has_many 'foos', -> { active }
44+
has_many :blahs
45+
end
46+
RUBY
47+
end
48+
end
49+
50+
describe 'has_one' do
51+
it 'registers an offense' do
52+
expect_offense(<<~RUBY)
53+
class Post < ApplicationRecord
54+
has_one :foo
55+
^^^^^^^^^^^^ Association `foo` is defined multiple times. Don't repeat associations.
56+
has_one :bar
57+
has_one :foo, -> { active }
58+
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `foo` is defined multiple times. Don't repeat associations.
59+
has_one :blah
60+
end
61+
RUBY
62+
63+
expect_correction(<<~RUBY)
64+
class Post < ApplicationRecord
65+
has_one :bar
66+
has_one :foo, -> { active }
67+
has_one :blah
68+
end
69+
RUBY
70+
end
71+
end
72+
73+
describe 'has_and_belongs_to_many' do
74+
it 'registers an offense' do
75+
expect_offense(<<~RUBY)
76+
class Post < ApplicationRecord
77+
has_and_belongs_to_many :foos
78+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `foos` is defined multiple times. Don't repeat associations.
79+
has_and_belongs_to_many :bars
80+
has_and_belongs_to_many :foos, -> { active }
81+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `foos` is defined multiple times. Don't repeat associations.
82+
has_and_belongs_to_many :blahs
83+
end
84+
RUBY
85+
86+
expect_correction(<<~RUBY)
87+
class Post < ApplicationRecord
88+
has_and_belongs_to_many :bars
89+
has_and_belongs_to_many :foos, -> { active }
90+
has_and_belongs_to_many :blahs
91+
end
92+
RUBY
93+
end
94+
end
95+
96+
describe 'all associations' do
97+
it 'marks offenses for duplicate associations of differing types' do
98+
expect_offense(<<~RUBY)
99+
class Post < ApplicationRecord
100+
has_many :foos
101+
^^^^^^^^^^^^^^ Association `foos` is defined multiple times. Don't repeat associations.
102+
has_and_belongs_to_many :foos
103+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `foos` is defined multiple times. Don't repeat associations.
104+
has_and_belongs_to_many :bars
105+
has_and_belongs_to_many :foos, -> { active }
106+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `foos` is defined multiple times. Don't repeat associations.
107+
has_and_belongs_to_many :blahs
108+
109+
has_one :author
110+
^^^^^^^^^^^^^^^ Association `author` is defined multiple times. Don't repeat associations.
111+
has_one :top_comment, -> { order(likes: :desc) }, class_name: 'Comment'
112+
belongs_to :author
113+
^^^^^^^^^^^^^^^^^^ Association `author` is defined multiple times. Don't repeat associations.
114+
end
115+
RUBY
116+
117+
expect_correction(<<~RUBY)
118+
class Post < ApplicationRecord
119+
has_and_belongs_to_many :bars
120+
has_and_belongs_to_many :foos, -> { active }
121+
has_and_belongs_to_many :blahs
122+
123+
has_one :top_comment, -> { order(likes: :desc) }, class_name: 'Comment'
124+
belongs_to :author
125+
end
126+
RUBY
127+
end
128+
129+
it 'does not flag non-duplicate associations' do
130+
expect_no_offenses(<<-RUBY)
131+
class Post < ApplicationRecord
132+
belongs_to :user
133+
134+
has_many :comments
135+
has_many :commenters, through: :comments
136+
137+
has_many :active_comments, -> { active }, class_name: 'Comment'
138+
139+
has_one :top_comment, -> { order(likes: :desc) }, class_name: 'Comment'
140+
141+
has_and_belongs_to_many :related_posts, class_name: 'Post'
142+
end
143+
RUBY
144+
end
145+
end
146+
end

0 commit comments

Comments
 (0)