Add Rails/FindByOrAssignmentMemoization cop

Author: r7kamura

changelog/new_add_rails_find_by_or_assignment_memoization_cop.md

@@ -0,0 +1 @@
+* [#1324](https://github.com/rubocop/rubocop-rails/pull/1324): Add `Rails/FindByOrAssignmentMemoization` cop. ([@r7kamura][])

config/default.yml

@@ -502,6 +502,12 @@ Rails/FindById:
   Enabled: 'pending'
   VersionAdded: '2.7'
 
+Rails/FindByOrAssignmentMemoization:
+  Description: 'Avoid memoization by `find_by` with `||=`.'
+  Enabled: pending
+  Safe: false
+  VersionAdded: '<<next>>'
+
 Rails/FindEach:
   Description: 'Prefer all.find_each over all.each.'
   StyleGuide: 'https://rails.rubystyle.guide#find-each'

lib/rubocop/cop/rails/find_by_or_assignment_memoization.rb

@@ -0,0 +1,71 @@
+# 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
+      #   def current_user
+      #     @current_user ||= User.find_by(id: session[:user_id])
+      #   end
+      #
+      #   # good
+      #   def current_user
+      #     if instance_variable_defined?(:@current_user)
+      #       @current_user
+      #     else
+      #       @current_user = User.find_by(id: session[:user_id])
+      #     end
+      #   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|
+            next if in_condition?(assignment_node)
+
+            add_offense(assignment_node) do |corrector|
+              corrector.replace(
+                assignment_node,
+                <<~RUBY.rstrip
+                  if instance_variable_defined?(:#{varible_name})
+                    #{varible_name}
+                  else
+                    #{varible_name} = #{find_by.source}
+                  end
+                RUBY
+              )
+            end
+          end
+        end
+
+        private
+
+        def in_condition?(node)
+          node.each_ancestor(:if, :unless).any?
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -55,6 +55,7 @@
 require_relative 'rails/file_path'
 require_relative 'rails/find_by'
 require_relative 'rails/find_by_id'
+require_relative 'rails/find_by_or_assignment_memoization'
 require_relative 'rails/find_each'
 require_relative 'rails/freeze_time'
 require_relative 'rails/has_and_belongs_to_many'

spec/rubocop/cop/rails/find_by_or_assignment_memoization_spec.rb

@@ -0,0 +1,68 @@
+# 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)
+        if instance_variable_defined?(:@current_user)
+          @current_user
+        else
+          @current_user = 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
+
+  context 'with `||`' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        @current_user ||= User.find_by(id: session[:user_id]) || User.anonymous
+      RUBY
+    end
+  end
+
+  context 'with ternary operator' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        @current_user ||= session[:user_id] ? User.find_by(id: session[:user_id]) : nil
+      RUBY
+    end
+  end
+
+  context 'with `if`' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        @current_user ||= User.find_by(id: session[:user_id]) if session[:user_id]
+      RUBY
+    end
+  end
+
+  context 'with `unless`' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        @current_user ||= User.find_by(id: session[:user_id]) unless session[:user_id].nil?
+      RUBY
+    end
+  end
+end