Merge pull request #542 from koic/fix_a_false_positive_for_rails_has_…

…many_or_has_one_dependent [Fix #541] Fix a false positive for `Rails/HasManyOrHasOneDependent`
rubocop · Sep 11, 2021 · ad0c558 · ad0c558
2 parents 7607908 + 245bbbc
commit ad0c558
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### Bug fixes
+
+* [#541](https://github.com/rubocop/rubocop-rails/issues/541): Fix an error for `Rails/HasManyOrHasOneDependent` when using lambda argument and specifying `:dependent` strategy. ([@koic][])
+
 ## 2.12.1 (2021-09-10)
 
 ### Bug fixes

diff --git a/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb b/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb
@@ -45,7 +45,7 @@ class HasManyOrHasOneDependent < Base
         PATTERN
 
         def_node_matcher :association_with_options?, <<~PATTERN
-          (send nil? {:has_many :has_one} _ (hash $...))
+          (send nil? {:has_many :has_one} ... (hash $...))
         PATTERN
 
         def_node_matcher :dependent_option?, <<~PATTERN

diff --git a/spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb b/spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb
@@ -20,15 +20,6 @@ class Person < ApplicationRecord
       RUBY
     end
 
-    it 'registers an offense when using lambda argument and not specifying any options' do
-      expect_offense(<<~RUBY)
-        class User < ApplicationRecord
-          has_one :articles, -> { where(active: true) }
-          ^^^^^^^ Specify a `:dependent` option.
-        end
-      RUBY
-    end
-
     it 'does not register an offense when specifying `:dependent` strategy' do
       expect_no_offenses(<<~RUBY)
         class Person < ApplicationRecord
@@ -114,10 +105,27 @@ class Person < ApplicationRecord
       RUBY
     end
 
+    it 'registers an offense when using lambda argument and not specifying any options' do
+      expect_offense(<<~RUBY)
+        class User < ApplicationRecord
+          has_many :articles, -> { where(active: true) }
+          ^^^^^^^^ Specify a `:dependent` option.
+        end
+      RUBY
+    end
+
     it 'does not register an offense when specifying `:dependent` strategy' do
       expect_no_offenses('has_many :foo, dependent: :bar')
     end
 
+    it 'does not register an offense when using lambda argument and specifying `:dependent` strategy' do
+      expect_no_offenses(<<~RUBY)
+        class User < ApplicationRecord
+          has_many :articles, -> { where(active: true) }, dependent: :destroy
+        end
+      RUBY
+    end
+
     it 'does not register an offense when specifying default `dependent: nil` strategy' do
       expect_no_offenses(<<~RUBY)
         class Person < ApplicationRecord
@@ -131,6 +139,14 @@ class Person < ApplicationRecord
         expect_no_offenses('has_many :foo, through: :bars')
       end
 
+      it 'does not register an offense when using lambda argument and specifying non-nil `:through` option' do
+        expect_no_offenses(<<~RUBY)
+          class User < ApplicationRecord
+            has_many :activities, -> { order(created_at: :desc) }, through: :notes, source: :activities
+          end
+        RUBY
+      end
+
       it 'registers an offense for nil value' do
         expect_offense(<<~RUBY)
           class Person < ApplicationRecord