-
-
Notifications
You must be signed in to change notification settings - Fork 263
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add PrependTrueOptionOnBeforeDestroy cop
Prevents unexpected deletion of associated records
- Loading branch information
1 parent
73fe04a
commit d470791
Showing
5 changed files
with
96 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
* [#922](https://github.com/rubocop/rubocop-rails/pull/922): Add PrependTrueOptionOnBeforeDestroy cop. ([@SHinGo-Koba][]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
57 changes: 57 additions & 0 deletions
57
lib/rubocop/cop/rails/prepend_true_option_on_before_destroy.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
32 changes: 32 additions & 0 deletions
32
spec/rubocop/cop/rails/prepend_true_option_on_before_destroy_spec.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |