Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails/HasManyOrHasOneDependent should allow combination of association extension and with_options #415

Closed
ohbarye opened this issue Jan 6, 2021 · 0 comments · Fixed by #416

Comments

@ohbarye
Copy link
Contributor

ohbarye commented Jan 6, 2021

Rails/HasManyOrHasOneDependent should allow combination of association extension and with_options.

As for association extensions, please see the official document.


Expected behavior

The following code does not offense Rails/HasManyOrHasOneDependent cop rule since it actually gives dependent option.

class Person < ApplicationRecord
  with_options dependent: :destroy do
    has_many :foo do
      def bar
      end
    end
  end
end

Actual behavior

rubocop-rails treats the code as offensed.

        class Person < ApplicationRecord
          with_options dependent: :destroy do
            has_many :foo do
       +    ^^^^^^^^ Specify a `:dependent` option.
              def bar
              end
            end

Steps to reproduce the problem

  1. git clone https://github.com/rubocop-hq/rubocop-rails.git && cd rubocop-rails && bundle install
  2. Save the following patch as patch.diff
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 2d9ab5e60..a99b4a9a9 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
@@ -132,6 +132,19 @@ RSpec.describe RuboCop::Cop::Rails::HasManyOrHasOneDependent do
         RUBY
       end
 
+      it "doesn't register an offense for `with_options dependent: :destroy` and for using association extension" do
+        expect_no_offenses(<<~RUBY)
+          class Person < ApplicationRecord
+            with_options dependent: :destroy do
+              has_many :foo do
+                def bar
+                end
+              end
+            end
+          end
+        RUBY
+      end
+
       context 'Multiple associations' do
         it "doesn't register an offense for " \
            '`with_options dependent: :destroy`' do
  1. git apply patch.diff
  2. bundle exec rspec spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb:135

RuboCop version

$ [bundle exec] rubocop -V
1.7.0 (using Parser 3.0.0.0, rubocop-ast 1.4.0, running on ruby 2.7.1 x86_64-darwin19)
ohbarye added a commit to ohbarye/rubocop-rails that referenced this issue Jan 6, 2021
ohbarye added a commit to ohbarye/rubocop-rails that referenced this issue Jan 6, 2021
@koic koic closed this as completed in #416 Jan 12, 2021
koic added a commit that referenced this issue Jan 12, 2021
…extension-and-with_options

[Fix #415] Make `Rails/HasManyOrHasOneDependent` accept association extension
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant