From b5b5dbbe22442ad726e0590b74f528f5fdd2bb90 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 28 Apr 2021 00:18:26 +0900 Subject: [PATCH] [Fix #147] Fix a false positive for `Rails/HasManyOrHasOneDependent` Fixes #147. Closes #176. This PR fixes a false positive for `Rails/HasManyOrHasOneDependent` when specifying default `dependent: nil` strategy. --- CHANGELOG.md | 1 + docs/modules/ROOT/pages/cops_rails.adoc | 1 + .../cop/rails/has_many_or_has_one_dependent.rb | 3 ++- .../rails/has_many_or_has_one_dependent_spec.rb | 16 ++++++++++++++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ed2d12aab..2090278142 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/docs/modules/ROOT/pages/cops_rails.adoc b/docs/modules/ROOT/pages/cops_rails.adoc index 75429617b3..8ccf2e4170 100644 --- a/docs/modules/ROOT/pages/cops_rails.adoc +++ b/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 ---- 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 index 4c831035d8..881e4340e1 100644 --- a/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb +++ b/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 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 index 0043dd8ee5..32fc1001e2 100644 --- 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 @@ -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')