Skip to content

Commit

Permalink
[FIX rubocop#4751] Rails/HasManyOrHasOneDependent shouldn't report of…
Browse files Browse the repository at this point in the history
…fense if :through option was specified
  • Loading branch information
smakagon committed Sep 23, 2017
1 parent 3f60bab commit 1b56de5
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* [#4745](https://github.com/bbatsov/rubocop/issues/4745): Make `Style/SafeNavigation` ignore negated continuations. ([@drenmi][])
* [#4732](https://github.com/bbatsov/rubocop/issues/4732): Prevent `Performance/HashEachMethods` from registering an offense when `#each` follows `#to_a`. ([@drenmi][])
* [#4730](https://github.com/bbatsov/rubocop/issues/4730): False positive on Lint/InterpolationCheck. ([@koic][])
* [#4751](https://github.com/bbatsov/rubocop/issues/4751): Prevent `Rails/HasManyOrHasOneDependent` cop from registering offense if `:through` option was specified. ([@smakagon][])

## 0.50.0 (2017-09-14)

Expand Down
10 changes: 9 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 @@ -5,6 +5,7 @@ module Cop
module Rails
# This cop looks for `has_many` or `has_one` associations that don't
# specify a `:dependent` option.
# It doesn't register an offense if `:through` option was specified.
#
# @example
# # bad
Expand All @@ -17,6 +18,7 @@ module Rails
# class User < ActiveRecord::Base
# has_many :comments, dependent: :restrict_with_exception
# has_one :avatar, dependent: :destroy
# has_many :patients, through: :appointments
# end
class HasManyOrHasOneDependent < Cop
MSG = 'Specify a `:dependent` option.'.freeze
Expand All @@ -33,11 +35,17 @@ class HasManyOrHasOneDependent < Cop
(pair (sym :dependent) !(:nil))
PATTERN

def_node_matcher :has_through?, <<-PATTERN
(pair (sym :through) !(:nil))
PATTERN

def on_send(node)
unless is_has_many_or_has_one_without_options?(node)
pairs = is_has_many_or_has_one_with_options?(node)
return unless pairs
return if pairs.any? { |pair| has_dependent?(pair) }
return if pairs.any? do |pair|
has_dependent?(pair) || has_through?(pair)
end
end

add_offense(node, :selector)
Expand Down
2 changes: 2 additions & 0 deletions manual/cops_rails.md
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ Enabled | No

This cop looks for `has_many` or `has_one` associations that don't
specify a `:dependent` option.
It doesn't register an offense if `:through` option was specified.

### Example

Expand All @@ -496,6 +497,7 @@ end
class User < ActiveRecord::Base
has_many :comments, dependent: :restrict_with_exception
has_one :avatar, dependent: :destroy
has_many :patients, through: :appointments
end
```

Expand Down
30 changes: 30 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 @@ -25,6 +25,21 @@ class Person
it 'does not register an offense when specifying `:dependent` strategy' do
expect_no_offenses('has_one :foo, dependent: :bar')
end

context 'with :through option' do
it 'does not register an offense for non-nil value' do
expect_no_offenses('has_one :foo, through: :bar')
end

it 'registers an offense for nil value' do
expect_offense(<<-RUBY.strip_indent)
class Person
has_one :foo, through: nil
^^^^^^^ Specify a `:dependent` option.
end
RUBY
end
end
end

context 'has_many' do
Expand All @@ -49,5 +64,20 @@ class Person
it 'does not register an offense when specifying `:dependent` strategy' do
expect_no_offenses('has_many :foo, dependent: :bar')
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')
end

it 'registers an offense for nil value' do
expect_offense(<<-RUBY.strip_indent)
class Person
has_many :foo, through: nil
^^^^^^^^ Specify a `:dependent` option.
end
RUBY
end
end
end
end

0 comments on commit 1b56de5

Please sign in to comment.