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

False Rails/HasManyOrHasOneDependent warning #528

Closed
leopirolo opened this issue Aug 27, 2021 · 3 comments · Fixed by #530
Closed

False Rails/HasManyOrHasOneDependent warning #528

leopirolo opened this issue Aug 27, 2021 · 3 comments · Fixed by #530
Labels
bug Something isn't working

Comments

@leopirolo
Copy link

Hello,

Not sure if I'm doing this right or not, but I'm having some issues with has_many associations.

Example

Here's an example:

class Character < ::ApplicationRecord
  has_many(:items, **{ dependent: :destroy })
end

Actual behaviour

Here's the warning message I get:

Specify a `:dependent` option. (convention:Rails/HasManyOrHasOneDependent)

But if I write this line like this, without keywords brackets, I get no warning:

class Character < ::ApplicationRecord
  has_many(:items, dependent: :destroy)
end

Expected behaviour

No warning messages?

Specs

  • Ruby 3.0.2
  • Rails 6.1.4.1
  • Rubocop 1.20.0
@leopirolo leopirolo changed the title False Rails/HasManyOrHasOneDependent warning False Rails/HasManyOrHasOneDependent warning Aug 27, 2021
@dvandersluis dvandersluis transferred this issue from rubocop/rubocop Aug 27, 2021
@andyw8
Copy link
Contributor

andyw8 commented Aug 27, 2021

There are no warnings when it's written as:

has_many(:items, { dependent: :destroy })

I don't think there's any reason to use the double splat (**) here.

koic added a commit to koic/rubocop-rails that referenced this issue Aug 31, 2021
…ndent`

Fixes rubocop#528.

This PR fixes a false positive for `Rails/HasManyOrHasOneDependent`
when specifying `:dependent` strategy with double splat.
@koic
Copy link
Member

koic commented Aug 31, 2021

This is affected by changes in the keyword arguments in Ruby 3.0.
https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/

One of the following changes is required:

has_many(:items, dependent: :destroy) # I strongly recommend this.
has_many(:items, **{dependent: :destroy})

I've opened #530, the latter will be accepted, but I strongly recommend the former (It's simple and clear) .

@koic koic added the bug Something isn't working label Sep 1, 2021
@koic koic closed this as completed in #530 Sep 2, 2021
koic added a commit that referenced this issue Sep 2, 2021
…many_or_has_one_dependent

[Fix #528] Fix a false positive for `Rails/HasManyOrHasOneDependent`
@erickm32
Copy link

erickm32 commented Mar 9, 2022

Hi @koic, I have a situation that I believe it could fit on this issue, but I'm on ruby 2.7 yet.
We have a class that has several has_many relations and they share some options like this

class MyClass < ApplicationRecord

  DEFAULT_OPERATIONABLE_OPTIONS = { source: :operationable, dependent: :restrict_with_error }.freeze

  has_many :items, DEFAULT_OPERATIONABLE_OPTIONS.merge(source_type: 'Item', through: :some_column)
  has_many :things,  DEFAULT_OPERATIONABLE_OPTIONS.merge(source_type: 'Thing',  through: :some_column)
  ...

  has_many :other_items, DEFAULT_OPERATIONABLE_OPTIONS.merge(source_type: 'OtherItem', through: :some_other_column)
  has_many :other_things, DEFAULT_OPERATIONABLE_OPTIONS.merge(source_type: 'OtherThing', through: :some_other_column)
  ...
end

I'm getting the same error when running rubocop, could this be a bug or this is not acceptable?

I'm running on rubocop 1.25.1 (rubocop-rails 2.13.2), Rails 6.1.4.6 and ruby 2.7.2p137 [x86_64-linux], on the process of upgrading from rails 5.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants