diff --git a/changelog/new_add_prepend_true_option_on_before_destroy_cop.md b/changelog/new_add_prepend_true_option_on_before_destroy_cop.md new file mode 100644 index 0000000000..94995a7dbe --- /dev/null +++ b/changelog/new_add_prepend_true_option_on_before_destroy_cop.md @@ -0,0 +1 @@ +* [#922](https://github.com/rubocop/rubocop-rails/pull/922): Add PrependTrueOptionOnBeforeDestroy cop. ([@SHinGo-Koba][]) diff --git a/config/default.yml b/config/default.yml index 92b8b3ddd2..374514e5f1 100644 --- a/config/default.yml +++ b/config/default.yml @@ -736,6 +736,11 @@ Rails/PluralizationGrammar: Enabled: true VersionAdded: '0.35' +Rails/PrependTrueOptionOnBeforeDestroy: + Description: 'Add `prepend: true` option on `before_destroy`.' + Enabled: pending + VersionAdded: '<>' + Rails/Presence: Description: 'Checks code that can be written more easily using `Object#presence` defined by Active Support.' Enabled: true diff --git a/lib/rubocop/cop/rails/prepend_true_option_on_before_destroy.rb b/lib/rubocop/cop/rails/prepend_true_option_on_before_destroy.rb new file mode 100644 index 0000000000..12965f649b --- /dev/null +++ b/lib/rubocop/cop/rails/prepend_true_option_on_before_destroy.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Looks for `before_destroy` without `prepend: true` option. + # + # `dependent: :destroy` might cause unexpected deletion of associated records + # unless `before_destroy` callback had `prepend: true` option. + # + # This could cause because `dependent: :destroy` is one of callbacks as well as `before_destroy`, + # therefore they are run in the order they are defined. + # + # Adding `prepend: true` option on `before_destroy` is one of solutions of this happening. + # + # @example + # # bad + # class User < ActiveRecord::Base + # has_many :comments, dependent: :destroy + # + # before_destroy :prevent_deletion_if_comments_exists + # end + # + # # good + # class User < ActiveRecord::Base + # has_many :comments, dependent: :destroy + # + # before_destroy :prevent_deletion_if_comments_exists, prepend: true + # end + class PrependTrueOptionOnBeforeDestroy < Base + extend AutoCorrector + + RESTRICT_ON_SEND = %i[before_destroy].freeze + + def_node_matcher :match_before_destroy_with_prepend_true_option, <<~PATTERN + (send _ :before_destroy ... + (hash <(pair (sym :prepend) true) ...>) + ) + PATTERN + + MSG = 'Add `prepend: true` option on `before_destroy` to prevent unexpected deletion of associated records.' + + def on_send(node) + return if node.receiver + + matched_node = match_before_destroy_with_prepend_true_option(node) + + return if matched_node + + add_offense(node) do |corrector| + corrector.insert_after(node, ', prepend: true') + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 24908f0516..a7c2a7861a 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -83,6 +83,7 @@ require_relative 'rails/pluck_id' require_relative 'rails/pluck_in_where' require_relative 'rails/pluralization_grammar' +require_relative 'rails/prepend_true_option_on_before_destroy' require_relative 'rails/presence' require_relative 'rails/present' require_relative 'rails/rake_environment' diff --git a/spec/rubocop/cop/rails/prepend_true_option_on_before_destroy_spec.rb b/spec/rubocop/cop/rails/prepend_true_option_on_before_destroy_spec.rb new file mode 100644 index 0000000000..0272692047 --- /dev/null +++ b/spec/rubocop/cop/rails/prepend_true_option_on_before_destroy_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::PrependTrueOptionOnBeforeDestroy, :config do + it 'registers an offense when `before_destroy` without `prepend: true` option.' do + expect_offense(<<~RUBY) + class User < ActiveRecord::Base + has_many :comments, dependent: :destroy + + before_destroy :prevent_deletion_if_comments_exists + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Add `prepend: true` option on `before_destroy` to prevent unexpected deletion of associated records. + end + RUBY + + expect_correction(<<~RUBY) + class User < ActiveRecord::Base + has_many :comments, dependent: :destroy + + before_destroy :prevent_deletion_if_comments_exists, prepend: true + end + RUBY + end + + it 'registers no offense when `before_destroy` with `prepend: true` option' do + expect_no_offenses(<<~RUBY) + class User < ActiveRecord::Base + has_many :comments, dependent: :destroy + + before_destroy :prevent_deletion_if_comments_exists, prepend: true + end + RUBY + end +end