Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #1128] Make Rails/DuplicateAssociation aware of duplicate class_name #1145

Conversation

koic
Copy link
Member

@koic koic commented Oct 6, 2023

Fixes #1128.

This PR makes Rails/DuplicateAssociation aware of duplicate class_name.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.
  • If this is a new cop, consider making a corresponding update to the Rails Style Guide.

…te `class_name`

Fixes rubocop#1128.

This PR makes `Rails/DuplicateAssociation` aware of duplicate `class_name`.
@koic koic merged commit d59999a into rubocop:master Oct 10, 2023
4 checks passed
@koic koic deleted the make_rails_duplicate_association_aware_of_duplicate_class_name branch October 10, 2023 15:25
Comment on lines +23 to +26
# # bad
# belongs_to :foo, class_name: 'Foo'
# belongs_to :bar, class_name: 'Foo'
# has_one :baz

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@koic This example (with belongs_to) is a false positive. Here's an example that should make this clear:

class GraphEdge
  belongs_to :source_node, class_name: "GraphNode"
  belongs_to :target_node, class_name: "GraphNode"
end

Contrary to has_many, the names of belongs_to associations are not arbitrary, as they implicitly reference the foreign key.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed the same problem after upgrading to v2.22.0 today.

I think the “identical class_name value” is a valid offense for both :has_one, :has_many and :has_and_belongs_to_many – but not for :belongs_to.

koic added a commit to koic/rubocop-rails that referenced this pull request Oct 28, 2023
Fixes rubocop#1145.

This PR fixes a false positive for `Rails/DuplicateAssociation`
when using duplicate `belongs_to` associations of same class without other arguments.
koic added a commit to koic/rubocop-rails that referenced this pull request Oct 28, 2023
Fixes rubocop#1145.

This PR fixes a false positive for `Rails/DuplicateAssociation`
when using duplicate `belongs_to` associations of same class without other arguments.
koic added a commit that referenced this pull request Oct 28, 2023
…licate_association

[Fix #1145] Fix a false positive for `Rails/DuplicateAssociation`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rubocop Missing Offence for Identical has_many Definitions
3 participants