Skip to content

Commit

Permalink
[Fix #147] Fix a false positive for Rails/HasManyOrHasOneDependent
Browse files Browse the repository at this point in the history
Fixes #147.
Closes #176.

This PR fixes a false positive for `Rails/HasManyOrHasOneDependent`
when specifying default `dependent: nil` strategy.
  • Loading branch information
koic committed Apr 27, 2021
1 parent c63330e commit b5b5dbb
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* [#450](https://github.com/rubocop/rubocop-rails/issues/450): Fix a crash for `Rails/ContentTag` with nested content tags. ([@tejasbubane][])
* [#103](https://github.com/rubocop/rubocop-rails/issues/103): Fix a false positive for `Rails/FindEach` when not inheriting `ActiveRecord::Base` and using `all.each`. ([@koic][])
* [#466](https://github.com/rubocop/rubocop-rails/pull/466): Fix a false positive for `Rails/DynamicFindBy` when not inheriting `ApplicationRecord` and without no receiver. ([@koic][])
* [#147](https://github.com/rubocop/rubocop-rails/issues/147): Fix a false positive for `Rails/HasManyOrHasOneDependent` when specifying default `dependent: nil` strategy. ([@koic][])

### Changes

Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1586,6 +1586,7 @@ end
class User < ActiveRecord::Base
has_many :comments, dependent: :restrict_with_exception
has_one :avatar, dependent: :destroy
has_many :articles, dependent: nil
has_many :patients, through: :appointments
end
----
Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ module Rails
# class User < ActiveRecord::Base
# has_many :comments, dependent: :restrict_with_exception
# has_one :avatar, dependent: :destroy
# has_many :articles, dependent: nil
# has_many :patients, through: :appointments
# end
class HasManyOrHasOneDependent < Base
Expand All @@ -37,7 +38,7 @@ class HasManyOrHasOneDependent < Base
PATTERN

def_node_matcher :dependent_option?, <<~PATTERN
(pair (sym :dependent) !nil)
(pair (sym :dependent) {!nil (nil)})
PATTERN

def_node_matcher :present_option?, <<~PATTERN
Expand Down
16 changes: 16 additions & 0 deletions spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ class Person < ApplicationRecord
RUBY
end

it 'does not register an offense when specifying default `dependent: nil` strategy' do
expect_no_offenses(<<~RUBY)
class Person < ApplicationRecord
has_one :foo, dependent: nil
end
RUBY
end

context 'with :through option' do
it 'does not register an offense for non-nil value' do
expect_no_offenses(<<~RUBY)
Expand Down Expand Up @@ -93,6 +101,14 @@ class Person < ApplicationRecord
expect_no_offenses('has_many :foo, dependent: :bar')
end

it 'does not register an offense when specifying default `dependent: nil` strategy' do
expect_no_offenses(<<~RUBY)
class Person < ApplicationRecord
has_many :foo, dependent: nil
end
RUBY
end

context 'with :through option' do
it 'does not register an offense for non-nil value' do
expect_no_offenses('has_many :foo, through: :bars')
Expand Down

0 comments on commit b5b5dbb

Please sign in to comment.