diff --git a/changelog/new_rails_before_destroy_cop.md b/changelog/new_rails_before_destroy_cop.md new file mode 100644 index 0000000000..eeb7038fad --- /dev/null +++ b/changelog/new_rails_before_destroy_cop.md @@ -0,0 +1 @@ +* [#1083](https://github.com/rubocop/rubocop-rails/issues/1083): Add `Rails/BeforeDestroy` cop. ([@ecbrodie][], [@ydakuka][]) diff --git a/config/default.yml b/config/default.yml index 6b2a8b09e4..c3c4610a9b 100644 --- a/config/default.yml +++ b/config/default.yml @@ -240,6 +240,12 @@ Rails/AttributeDefaultBlockValue: Include: - 'app/models/**/*' +Rails/BeforeDestroy: + Description: 'Ensure the correct usage of `prepend: true` in `before_destroy` callbacks.' + Enabled: pending + Reference: 'https://guides.rubyonrails.org/active_record_callbacks.html#destroying-an-object' + VersionAdded: '<>' + Rails/BelongsTo: Description: >- Use `optional: true` instead of `required: false` for diff --git a/lib/rubocop/cop/rails/before_destroy.rb b/lib/rubocop/cop/rails/before_destroy.rb new file mode 100644 index 0000000000..aa93aa7251 --- /dev/null +++ b/lib/rubocop/cop/rails/before_destroy.rb @@ -0,0 +1,169 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Ensures that `before_destroy` callbacks are executed + # before `dependent: :destroy` associations by requiring + # the `prepend: true` option. + # + # Without `prepend: true`, `before_destroy` callbacks may run + # after associated records are already deleted, leading to + # unintended behavior. + # + # @example + # # bad + # has_many :entities, dependent: :destroy + # before_destroy { do_something } + # + # # good + # has_many :entities, dependent: :destroy + # before_destroy(prepend: true) { do_something } + # + # @example + # # bad + # belongs_to :entity, dependent: :destroy + # before_destroy :some_method + # + # # good + # belongs_to :entity, dependent: :destroy + # before_destroy :some_method, prepend: true + # + # @example + # # bad + # has_one :entity, dependent: :destroy + # before_destroy MyClass.new + # + # # good + # has_one :entity, dependent: :destroy + # before_destroy MyClass.new, prepend: true + # + # @example + # # bad + # has_many :entities, dependent: :destroy + # before_destroy -> { do_something } + # + # # good + # has_many :entities, dependent: :destroy + # before_destroy -> { do_something }, prepend: true + # + class BeforeDestroy < Base + extend AutoCorrector + + MSG = '"before_destroy" callbacks must be declared before "dependent: :destroy" associations ' \ + 'or use `prepend: true`.' + RESTRICT_ON_SEND = %i[before_destroy].freeze + + def_node_search :association_nodes, <<~PATTERN + (send nil? {:belongs_to :has_one :has_many} _ (hash ...)) + PATTERN + + def_node_matcher :hash_options, <<~PATTERN + `(hash $...) + PATTERN + + def_node_matcher :dependent_destroy?, <<~PATTERN + (pair (sym :dependent) (sym :destroy)) + PATTERN + + def_node_matcher :prepend_true?, <<~PATTERN + (pair (sym :prepend) true) + PATTERN + + def on_send(node) + check_add_prepend_true(node) + check_remove_prepend_true(node) + end + + private + + def check_add_prepend_true(node) + return if contains_prepend_true?(node) + return unless before_association_with_dependent_destroy?(node) + + add_offense(node) { |corrector| autocorrect_add_prepend(corrector, node) } + end + + def check_remove_prepend_true(node) + return unless contains_prepend_true?(node) + return if before_any_association?(node) + + add_offense(node) { |corrector| autocorrect_remove_prepend(corrector, node) } + end + + def autocorrect_remove_prepend(corrector, node) + prepend_pair = hash_options(node).find { |pair| prepend_true?(pair) } + prepend_range = prepend_pair.source_range + start_pos, end_pos = adjust_removal_range(prepend_range) + + corrector.remove(prepend_range.with(begin_pos: start_pos, end_pos: end_pos)) + remove_block_delimiters(corrector, node) + end + + def autocorrect_add_prepend(corrector, node) + hash_node = node.arguments.find(&:hash_type?) + + if hash_node + corrector.insert_before(hash_node.children.first, 'prepend: true, ') + elsif node.arguments.empty? + corrector.insert_after(node.loc.selector, '(prepend: true)') + else + corrector.insert_after(node.last_argument, ', prepend: true') + end + end + + def adjust_removal_range(prepend_range) + start_pos = prepend_range.begin_pos + end_pos = prepend_range.end_pos + + source = processed_source.buffer.source + + prev_match = source[0...start_pos].match(/,\s*$/) + next_match = source[end_pos..].match(/^\s*,?/) + + if prev_match + start_pos = prev_match.begin(0) + elsif next_match + end_pos += next_match.end(0) + end + + [start_pos, end_pos] + end + + def remove_block_delimiters(corrector, node) + return unless node.block_literal? + return unless node.loc.begin + + corrector.remove(node.loc.begin) + corrector.remove(node.loc.end) + end + + def before_association_with_dependent_destroy?(node) + root_class_node = find_root_class(node) + association_nodes(root_class_node).any? do |assoc| + contains_dependent_destroy?(assoc) && assoc.first_line < node.first_line + end + end + + def before_any_association?(node) + root_class_node = find_root_class(node) + association_nodes(root_class_node).any? do |assoc| + assoc.first_line < node.first_line + end + end + + def find_root_class(node) + node.each_ancestor(:class, :module).first + end + + def contains_prepend_true?(node) + hash_options(node)&.any? { |pair| prepend_true?(pair) } + end + + def contains_dependent_destroy?(node) + hash_options(node).any? { |pair| dependent_destroy?(pair) } + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index d3b24ec9e9..4e62e60b24 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -29,6 +29,7 @@ require_relative 'rails/arel_star' require_relative 'rails/assert_not' require_relative 'rails/attribute_default_block_value' +require_relative 'rails/before_destroy' require_relative 'rails/belongs_to' require_relative 'rails/blank' require_relative 'rails/bulk_change_table' diff --git a/spec/rubocop/cop/rails/before_destroy_spec.rb b/spec/rubocop/cop/rails/before_destroy_spec.rb new file mode 100644 index 0000000000..371f024b63 --- /dev/null +++ b/spec/rubocop/cop/rails/before_destroy_spec.rb @@ -0,0 +1,1153 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::BeforeDestroy, :config do + ['class MyRecord < ApplicationRecord', 'module MyMixin'].each do |container| + %w[belongs_to has_one has_many].freeze.each do |association_type| + context "when inside a #{container.split.first}" do + context "and #{association_type} is declared before before_destroy" do + context 'and before_destroy uses a block' do + it 'registers an offense' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy { do_something } + ^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy(prepend: true) { do_something } + end + RUBY + end + + it 'does not register an offense if before_destroy with `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy(prepend: true) { do_something } + end + RUBY + end + + it "registers an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy with `prepend: true`' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy(prepend: true) { do_something } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy { do_something } + end + RUBY + end + + it "does not register an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy { do_something } + end + RUBY + end + + it 'does not register an offense if dependent has an option other than :destroy' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :nullify + before_destroy { do_something } + end + RUBY + end + + it 'registers an offense if before_destroy uses a block with a condition' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy unless: :condition? do + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + do_something + end + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy prepend: true, unless: :condition? do + do_something + end + end + RUBY + end + + it 'registers an offense ' \ + "if a #{association_type} has both associations with and without `dependent: :destroy`" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + before_destroy { do_something } + ^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + before_destroy(prepend: true) { do_something } + end + RUBY + end + end + + context 'and before_destroy references a method' do + it 'registers an offense' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy :some_method + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy :some_method, prepend: true + end + RUBY + end + + it 'does not register an offense if before_destroy with `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy :some_method, prepend: true + end + RUBY + end + + it "registers an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy with `prepend: true`' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy :some_method, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy :some_method + end + RUBY + end + + it "does not register an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy :some_method + end + RUBY + end + + it 'does not register an offense if dependent has an option other than :destroy' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :nullify + before_destroy :some_method + end + RUBY + end + + it 'registers an offense if before_destroy passes a method with a condition' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy :some_method, unless: :condition? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy :some_method, prepend: true, unless: :condition? + end + RUBY + end + + it 'registers an offense if before_destroy passes multiple methods' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy :some_method, :another_method + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy :some_method, :another_method, prepend: true + end + RUBY + end + + it 'registers an offense ' \ + "if a #{association_type} has both associations with and without `dependent: :destroy`" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + before_destroy :some_method + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + before_destroy :some_method, prepend: true + end + RUBY + end + end + + context 'and before_destroy is called with an instance of a class' do + it 'registers an offense' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy MyClass.new + ^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy MyClass.new, prepend: true + end + RUBY + end + + it 'does not register an offense if before_destroy with `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy MyClass.new, prepend: true + end + RUBY + end + + it "registers an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy with `prepend: true`' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy MyClass.new, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy MyClass.new + end + RUBY + end + + it "does not register an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy MyClass.new + end + RUBY + end + + it 'does not register an offense if dependent has an option other than :destroy' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :nullify + before_destroy MyClass.new + end + RUBY + end + + it 'registers an offense if before_destroy includes a condition' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy MyClass.new, unless: :condition? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy MyClass.new, prepend: true, unless: :condition? + end + RUBY + end + + it 'registers an offense if before_destroy passes multiple instances of classes' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy MyClass.new, AnotherClass.new + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy MyClass.new, AnotherClass.new, prepend: true + end + RUBY + end + + it 'registers an offense ' \ + "if a #{association_type} has both associations with and without `dependent: :destroy`" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + before_destroy MyClass.new + ^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + before_destroy MyClass.new, prepend: true + end + RUBY + end + end + + context 'and before_destroy references a lambda expression' do + it 'registers an offense' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy -> { do_something } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy -> { do_something }, prepend: true + end + RUBY + end + + it 'does not register an offense if before_destroy with `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy -> { do_something }, prepend: true + end + RUBY + end + + it "registers an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy with `prepend: true`' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy -> { do_something }, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy -> { do_something } + end + RUBY + end + + it "does not register an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy -> { do_something } + end + RUBY + end + + it 'does not register an offense if dependent has an option other than :destroy' do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :nullify + before_destroy -> { do_something } + end + RUBY + end + + it 'registers an offense if before_destroy includes a condition' do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy -> { do_something }, unless: :condition? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy -> { do_something }, prepend: true, unless: :condition? + end + RUBY + end + + it 'registers an offense ' \ + "if a #{association_type} has both associations with and without `dependent: :destroy`" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + before_destroy -> { do_something } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + before_destroy -> { do_something }, prepend: true + end + RUBY + end + end + end + + context "and before_destroy is declared before #{association_type} with `dependent: :destroy`" do + context 'and before_destroy uses a block' do + it 'registers an offense' do + expect_offense(<<~RUBY) + #{container} + before_destroy(prepend: true) { do_something } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy { do_something } + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it 'does not register an offense if before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy { do_something } + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it "registers an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy with `prepend: true`' do + expect_offense(<<~RUBY) + #{container} + before_destroy(prepend: true) { do_something } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy { do_something } + #{association_type} :entities + end + RUBY + end + + it "does not register an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy { do_something } + #{association_type} :entities + end + RUBY + end + + it 'registers an offense if dependent has an option other than :destroy' do + expect_offense(<<~RUBY) + #{container} + before_destroy(prepend: true) { do_something } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :nullify + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy { do_something } + #{association_type} :entities, dependent: :nullify + end + RUBY + end + + it 'registers an offense if before_destroy uses a block with a condition' do + expect_offense(<<~RUBY) + #{container} + before_destroy prepend: true, unless: :condition? do + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + do_something + end + #{association_type} :entities, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy unless: :condition? do + do_something + end + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it 'does not register an offense ' \ + "if a #{association_type} has both associations with and without `dependent: :destroy`" do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy do + do_something + end + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + end + RUBY + end + end + + context 'and before_destroy references a method' do + it 'registers an offense' do + expect_offense(<<~RUBY) + #{container} + before_destroy :some_method, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy :some_method + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it 'does not register an offense if before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy :some_method + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it "registers an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy with `prepend: true`' do + expect_offense(<<~RUBY) + #{container} + before_destroy :some_method, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy :some_method + #{association_type} :entities + end + RUBY + end + + it "does not register an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy :some_method + #{association_type} :entities + end + RUBY + end + + it 'registers an offense if dependent has an option other than :destroy' do + expect_offense(<<~RUBY) + #{container} + before_destroy :some_method, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :nullify + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy :some_method + #{association_type} :entities, dependent: :nullify + end + RUBY + end + + it 'registers an offense if before_destroy passes a method with a condition' do + expect_offense(<<~RUBY) + #{container} + before_destroy :some_method, prepend: true, unless: :condition? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy :some_method, unless: :condition? + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it 'registers an offense if before_destroy passes multiple methods' do + expect_offense(<<~RUBY) + #{container} + before_destroy :some_method, :another_method, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy :some_method, :another_method + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it 'does not register an offense ' \ + "if a #{association_type} has both associations with and without `dependent: :destroy`" do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy :some_method + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + end + RUBY + end + end + + context 'and before_destroy is called with an instance of a class' do + it 'registers an offense' do + expect_offense(<<~RUBY) + #{container} + before_destroy MyClass.new, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy MyClass.new + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it 'does not register an offense if before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy MyClass.new + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it "registers an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy with `prepend: true`' do + expect_offense(<<~RUBY) + #{container} + before_destroy MyClass.new, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy MyClass.new + #{association_type} :entities + end + RUBY + end + + it "does not register an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy MyClass.new + #{association_type} :entities + end + RUBY + end + + it 'registers an offense if dependent has an option other than :destroy' do + expect_offense(<<~RUBY) + #{container} + before_destroy MyClass.new, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :nullify + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy MyClass.new + #{association_type} :entities, dependent: :nullify + end + RUBY + end + + it 'registers an offense if before_destroy includes a condition' do + expect_offense(<<~RUBY) + #{container} + before_destroy MyClass.new, prepend: true, unless: :condition? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy MyClass.new, unless: :condition? + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it 'registers an offense if before_destroy passes multiple instances of classes' do + expect_offense(<<~RUBY) + #{container} + before_destroy MyClass.new, AnotherClass.new, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy MyClass.new, AnotherClass.new + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it 'does not register an offense ' \ + "if a #{association_type} has both associations with and without `dependent: :destroy`" do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy MyClass.new + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + end + RUBY + end + end + + context 'and before_destroy references a lambda expression' do + it 'registers an offense' do + expect_offense(<<~RUBY) + #{container} + before_destroy -> { do_something }, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy -> { do_something } + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it 'does not register an offense if before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy -> { do_something } + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it "registers an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy with `prepend: true`' do + expect_offense(<<~RUBY) + #{container} + before_destroy -> { do_something }, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy -> { do_something } + #{association_type} :entities + end + RUBY + end + + it "does not register an offense if a #{association_type} does not use `dependent: :destroy` " \ + 'and before_destroy without `prepend: true`' do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy -> { do_something } + #{association_type} :entities + end + RUBY + end + + it 'registers an offense if dependent has an option other than :destroy' do + expect_offense(<<~RUBY) + #{container} + before_destroy -> { do_something }, prepend: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :nullify + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy -> { do_something } + #{association_type} :entities, dependent: :nullify + end + RUBY + end + + it 'registers an offense if before_destroy includes a condition' do + expect_offense(<<~RUBY) + #{container} + before_destroy -> { do_something }, prepend: true, unless: :condition? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :entities, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + before_destroy -> { do_something }, unless: :condition? + #{association_type} :entities, dependent: :destroy + end + RUBY + end + + it 'does not register an offense ' \ + "if a #{association_type} has both associations with and without `dependent: :destroy`" do + expect_no_offenses(<<~RUBY) + #{container} + before_destroy -> { do_something } + #{association_type} :entities, dependent: :destroy + #{association_type} :accounts + end + RUBY + end + end + end + + context "and #{association_type} is declared around before_destroy" do + context 'and before_destroy uses a block' do + it "registers an offense when all #{association_type}'s have `dependent: :destroy`" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy { do_something } + ^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :accounts, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy(prepend: true) { do_something } + #{association_type} :accounts, dependent: :destroy + end + RUBY + end + + it "does not register an offense when no #{association_type}'s have `dependent: :destroy`" do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy { do_something } + #{association_type} :accounts + end + RUBY + end + + it 'registers an offense ' \ + "when the first #{association_type} has `dependent: :destroy` and the second does not" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy { do_something } + ^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :accounts + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy(prepend: true) { do_something } + #{association_type} :accounts + end + RUBY + end + + it 'does not register an offense ' \ + "when the first #{association_type} does not have `dependent: :destroy` and the second does" do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy { do_something } + #{association_type} :accounts, dependent: :destroy + end + RUBY + end + end + + context 'and before_destroy references a method' do + it "registers an offense when all #{association_type}'s have `dependent: :destroy`" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy :some_method + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :accounts, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy :some_method, prepend: true + #{association_type} :accounts, dependent: :destroy + end + RUBY + end + + it "does not register an offense when no #{association_type}'s have `dependent: :destroy`" do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy :some_method + #{association_type} :accounts + end + RUBY + end + + it 'registers an offense ' \ + "when the first #{association_type} has `dependent: :destroy` and the second does not" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy :some_method + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :accounts + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy :some_method, prepend: true + #{association_type} :accounts + end + RUBY + end + + it 'does not register an offense ' \ + "when the first #{association_type} does not have `dependent: :destroy` and the second does" do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy :some_method + #{association_type} :accounts, dependent: :destroy + end + RUBY + end + end + + context 'and before_destroy is called with an instance of a class' do + it "registers an offense when all #{association_type}'s have `dependent: :destroy`" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy MyClass.new + ^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :accounts, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy MyClass.new, prepend: true + #{association_type} :accounts, dependent: :destroy + end + RUBY + end + + it "does not register an offense when no #{association_type}'s have `dependent: :destroy`" do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy MyClass.new + #{association_type} :accounts + end + RUBY + end + + it 'registers an offense ' \ + "when the first #{association_type} has `dependent: :destroy` and the second does not" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy MyClass.new + ^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :accounts + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy MyClass.new, prepend: true + #{association_type} :accounts + end + RUBY + end + + it 'does not register an offense ' \ + "when the first #{association_type} does not have `dependent: :destroy` and the second does" do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy MyClass.new + #{association_type} :accounts, dependent: :destroy + end + RUBY + end + end + + context 'and before_destroy references a lambda expression' do + it "registers an offense when all #{association_type}'s have `dependent: :destroy`" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy -> { do_something } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :accounts, dependent: :destroy + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy -> { do_something }, prepend: true + #{association_type} :accounts, dependent: :destroy + end + RUBY + end + + it "does not register an offense when no #{association_type}'s have `dependent: :destroy`" do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy -> { do_something } + #{association_type} :accounts + end + RUBY + end + + it 'registers an offense ' \ + "when the first #{association_type} has `dependent: :destroy` and the second does not" do + expect_offense(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy -> { do_something } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "before_destroy" callbacks must be declared before "dependent: :destroy" associations or use `prepend: true`. + #{association_type} :accounts + end + RUBY + + expect_correction(<<~RUBY) + #{container} + #{association_type} :entities, dependent: :destroy + before_destroy -> { do_something }, prepend: true + #{association_type} :accounts + end + RUBY + end + + it 'does not register an offense ' \ + "when the first #{association_type} does not have `dependent: :destroy` and the second does" do + expect_no_offenses(<<~RUBY) + #{container} + #{association_type} :entities + before_destroy -> { do_something } + #{association_type} :accounts, dependent: :destroy + end + RUBY + end + end + end + end + end + end +end