-
-
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
Rails/FindByOrAssignmentMemoization
cop
- Loading branch information
Showing
5 changed files
with
106 additions
and
0 deletions.
There are no files selected for viewing
1 change: 1 addition & 0 deletions
1
changelog/new_add_rails_find_by_or_assignment_memoization_cop.md
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 @@ | ||
* [#x](https://github.com/rubocop/rubocop-rails/pull/x): Add `Rails/FindByOrAssignmentMemoization` cop. ([@r7kamura][]) |
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
61 changes: 61 additions & 0 deletions
61
lib/rubocop/cop/rails/find_by_or_assignment_memoization.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,61 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Rails | ||
# Avoid memoization by `find_by` with `||=`. | ||
# | ||
# It is common to see code that attempts to memoize `find_by` result by `||=`, | ||
# but `find_by` may return `nil`, in which case it is not memoized as intended. | ||
# | ||
# @safety | ||
# This cop is unsafe because detected `find_by` may not be activerecord's method, | ||
# or the code may have a different purpose than memoization. | ||
# | ||
# @example | ||
# # bad | ||
# @current_user ||= User.find_by(id: session[:user_id]) | ||
# | ||
# # good | ||
# @current_user = | ||
# if instance_variable_defined?(:@current_user) | ||
# @current_user | ||
# else | ||
# User.find_by(id: session[:user_id]) | ||
# end | ||
class FindByOrAssignmentMemoization < Base | ||
extend AutoCorrector | ||
|
||
MSG = 'Avoid memoization by `find_by` with `||=`.' | ||
|
||
RESTRICT_ON_SEND = %i[find_by].freeze | ||
|
||
def_node_matcher :find_by_or_assignment_memoization, <<~PATTERN | ||
(or_asgn | ||
(ivasgn $_) | ||
$(send _ :find_by ...) | ||
) | ||
PATTERN | ||
|
||
def on_send(node) | ||
assignment_node = node.parent | ||
find_by_or_assignment_memoization(assignment_node) do |varible_name, find_by| | ||
add_offense(assignment_node) do |corrector| | ||
corrector.replace( | ||
assignment_node, | ||
<<~RUBY.rstrip | ||
#{varible_name} = | ||
if instance_variable_defined?(:#{varible_name}) | ||
#{varible_name} | ||
else | ||
#{find_by.source} | ||
end | ||
RUBY | ||
) | ||
end | ||
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
37 changes: 37 additions & 0 deletions
37
spec/rubocop/cop/rails/find_by_or_assignment_memoization_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,37 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe RuboCop::Cop::Rails::FindByOrAssignmentMemoization, :config do | ||
context 'when using `find_by` with `||=`' do | ||
it 'registers an offense' do | ||
expect_offense(<<~RUBY) | ||
@current_user ||= User.find_by(id: session[:user_id]) | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid memoization by `find_by` with `||=`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
@current_user = | ||
if instance_variable_defined?(:@current_user) | ||
@current_user | ||
else | ||
User.find_by(id: session[:user_id]) | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'with `find_by!`' do | ||
it 'does not register an offense' do | ||
expect_no_offenses(<<~RUBY) | ||
@current_user ||= User.find_by!(id: session[:user_id]) | ||
RUBY | ||
end | ||
end | ||
|
||
context 'with local variable' do | ||
it 'does not register an offense' do | ||
expect_no_offenses(<<~RUBY) | ||
current_user ||= User.find_by(id: session[:user_id]) | ||
RUBY | ||
end | ||
end | ||
end |