Skip to content

Commit

Permalink
[Fix #1128] Make Rails/DuplicateAssociation aware of duplicate `cla…
Browse files Browse the repository at this point in the history
…ss_name`

Fixes #1128.

This PR makes `Rails/DuplicateAssociation` aware of duplicate `class_name`.
  • Loading branch information
koic committed Oct 6, 2023
1 parent e773322 commit c2efc00
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1128](https://github.com/rubocop/rubocop-rails/issues/1128): Make `Rails/DuplicateAssociation` aware of duplicate `class_name`. ([@koic][])
77 changes: 65 additions & 12 deletions lib/rubocop/cop/rails/duplicate_association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,91 @@ module Rails
# belongs_to :bar
# has_one :foo
#
# # bad
# belongs_to :foo, class_name: 'Foo'
# belongs_to :bar, class_name: 'Foo'
# has_one :baz
#
# # good
# belongs_to :bar, class_name: 'Foo'
# has_one :foo
#
class DuplicateAssociation < Base
include RangeHelp
extend AutoCorrector
include ClassSendNodeHelper
include ActiveRecordHelper

MSG = "Association `%<name>s` is defined multiple times. Don't repeat associations."
MSG_CLASS_NAME = "Association `class_name: %<name>s` is defined multiple times. Don't repeat associations."

def_node_matcher :association, <<~PATTERN
(send nil? {:belongs_to :has_one :has_many :has_and_belongs_to_many} ({sym str} $_) ...)
(send nil? {:belongs_to :has_one :has_many :has_and_belongs_to_many} ({sym str} $_) $...)
PATTERN

def_node_matcher :class_name, <<~PATTERN
(hash (pair (sym :class_name) $_))
PATTERN

def on_class(class_node)
return unless active_record?(class_node.parent_class)

offenses(class_node).each do |name, nodes|
nodes.each do |node|
add_offense(node, message: format(MSG, name: name)) do |corrector|
next if same_line?(nodes.last, node)
association_nodes = association_nodes(class_node)

corrector.remove(range_by_whole_lines(node.source_range, include_final_newline: true))
end
end
duplicated_association_name_nodes(association_nodes).each do |name, nodes|
register_offense(name, nodes, MSG)
end

duplicated_class_name_nodes(association_nodes).each do |class_name, nodes|
register_offense(class_name, nodes, MSG_CLASS_NAME)
end
end

private

def offenses(class_node)
class_send_nodes(class_node).select { |node| association(node) }
.group_by { |node| association(node).to_sym }
.select { |_, nodes| nodes.length > 1 }
def register_offense(name, nodes, message_template)
nodes.each do |node|
add_offense(node, message: format(message_template, name: name)) do |corrector|
next if same_line?(nodes.last, node)

corrector.remove(range_by_whole_lines(node.source_range, include_final_newline: true))
end
end
end

def association_nodes(class_node)
class_send_nodes(class_node).select do |node|
association(node)&.first
end
end

def duplicated_association_name_nodes(association_nodes)
grouped_associations = association_nodes.group_by do |node|
association(node).first.to_sym
end

leave_duplicated_association(grouped_associations)
end

def duplicated_class_name_nodes(association_nodes)
grouped_associations = association_nodes.group_by do |node|
arguments = association(node).last
next unless arguments.count == 1

if (class_name = class_name(arguments.first))
class_name.source
end
end

grouped_associations.delete(nil)

leave_duplicated_association(grouped_associations)
end

def leave_duplicated_association(grouped_associations)
grouped_associations.select do |_, nodes|
nodes.length > 1
end
end
end
end
Expand Down
46 changes: 46 additions & 0 deletions spec/rubocop/cop/rails/duplicate_association_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,52 @@ class Post < ApplicationRecord
RUBY
end

it 'registers offenses when using duplicate associations of same class without other arguments' do
expect_offense(<<~RUBY)
class Post < ApplicationRecord
has_many :foos, class_name: 'Foo'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `class_name: 'Foo'` is defined multiple times. Don't repeat associations.
has_many :bars, class_name: 'Foo'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `class_name: 'Foo'` is defined multiple times. Don't repeat associations.
has_one :baz, class_name: 'Bar'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `class_name: 'Bar'` is defined multiple times. Don't repeat associations.
has_one :qux, class_name: 'Bar'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `class_name: 'Bar'` is defined multiple times. Don't repeat associations.
end
RUBY

expect_correction(<<~RUBY)
class Post < ApplicationRecord
has_many :bars, class_name: 'Foo'
has_one :qux, class_name: 'Bar'
end
RUBY
end

it 'does not register an offense when using duplicate associations of same class with other arguments' do
expect_no_offenses(<<~RUBY)
class Post < ApplicationRecord
has_many :foos, if: condition, class_name: 'Foo'
has_many :bars, if: some_condition, class_name: 'Foo'
has_one :baz, -> { condition }, class_name: 'Bar'
has_one :qux, -> { some_condition }, class_name: 'Bar'
belongs_to :group, class_name: 'IndustryGroup', foreign_key: :industry_group_id
end
RUBY
end

it 'does not register an offense when not using association method' do
expect_no_offenses(<<~RUBY)
class Post < ApplicationRecord
validates_presence_of :name
end
RUBY
end

it 'does not flag non-duplicate associations' do
expect_no_offenses(<<-RUBY)
class Post < ApplicationRecord
Expand Down

0 comments on commit c2efc00

Please sign in to comment.