Merge pull request #467 from koic/fix_false_positive_for_rails_has_ma…

…ny_or_has_one_dependent

[Fix #147] Fix a false positive for `Rails/HasManyOrHasOneDependent`

CHANGELOG.md

@@ -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
 

docs/modules/ROOT/pages/cops_rails.adoc

@@ -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
 ----

lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb

@@ -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
@@ -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

spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb

@@ -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)
@@ -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')