Use StrongMemoize and enable/disable cops properly

8 files changed, 33 insertions, 34 deletions

diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb
index 7644f2ea95f..69de1044c2d 100644
--- a/app/models/concerns/mentionable.rb
+++ b/app/models/concerns/mentionable.rb
@@ -43,15 +43,12 @@ module Mentionable
     self
   end
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def all_references(current_user = nil, extractor: nil)
-    @extractors ||= {}
-
     # Use custom extractor if it's passed in the function parameters.
     if extractor
-      @extractors[current_user] = extractor
+      extractors[current_user] = extractor
     else
-      extractor = @extractors[current_user] ||= Gitlab::ReferenceExtractor.new(project, current_user)
+      extractor = extractors[current_user] ||= Gitlab::ReferenceExtractor.new(project, current_user)
 
       extractor.reset_memoized_values
     end
@@ -70,6 +67,10 @@ module Mentionable
     extractor
   end
 
+  def extractors
+    @extractors ||= {}
+  end
+
   def mentioned_users(current_user = nil)
     all_references(current_user).users
   end
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index c22fb01a4ba..fd6703831e4 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -102,11 +102,12 @@ module Milestoneish
     end
   end
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def memoize_per_user(user, method_name)
-    @memoized ||= {}
-    @memoized[method_name] ||= {}
-    @memoized[method_name][user&.id] ||= yield
+    memoized_users[method_name][user&.id] ||= yield
+  end
+
+  def memoized_users
+    @memoized_users ||= Hash.new { |h, k| h[k] = {} }
   end
 
   # override in a class that includes this module to get a faster query
diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb
index b44274f6145..fdea8bcb918 100644
--- a/app/models/concerns/noteable.rb
+++ b/app/models/concerns/noteable.rb
@@ -55,6 +55,7 @@ module Noteable
         discussion_notes.resolvable.discussions(self)
       end
   end
+  # rubocop:enable Cop/ModuleWithInstanceVariables
 
   def discussions_resolvable?
     resolvable_discussions.any?(&:resolvable?)
diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb
index 9a7b9cd12b0..2ecce745ab9 100644
--- a/app/models/concerns/relative_positioning.rb
+++ b/app/models/concerns/relative_positioning.rb
@@ -44,7 +44,6 @@ module RelativePositioning
     next_pos
   end
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def move_between(before, after)
     return move_after(before) unless after
     return move_before(after) unless before
@@ -53,13 +52,12 @@ module RelativePositioning
     # to its predecessor. This process will recursively move all the predecessors until we have a place
     if (after.relative_position - before.relative_position) < 2
       before.move_before
-      @positionable_neighbours = [before]
+      @positionable_neighbours = [before] # rubocop:disable Cop/ModuleWithInstanceVariables
     end
 
     self.relative_position = position_between(before.relative_position, after.relative_position)
   end
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def move_after(before = self)
     pos_before = before.relative_position
     pos_after = before.next_relative_position
@@ -67,7 +65,7 @@ module RelativePositioning
     if before.shift_after?
       issue_to_move = self.class.in_projects(project_ids).find_by!(relative_position: pos_after)
       issue_to_move.move_after
-      @positionable_neighbours = [issue_to_move]
+      @positionable_neighbours = [issue_to_move] # rubocop:disable Cop/ModuleWithInstanceVariables
 
       pos_after = issue_to_move.relative_position
     end
@@ -75,7 +73,6 @@ module RelativePositioning
     self.relative_position = position_between(pos_before, pos_after)
   end
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def move_before(after = self)
     pos_after = after.relative_position
     pos_before = after.prev_relative_position
@@ -83,7 +80,7 @@ module RelativePositioning
     if after.shift_before?
       issue_to_move = self.class.in_projects(project_ids).find_by!(relative_position: pos_before)
       issue_to_move.move_before
-      @positionable_neighbours = [issue_to_move]
+      @positionable_neighbours = [issue_to_move] # rubocop:disable Cop/ModuleWithInstanceVariables
 
       pos_before = issue_to_move.relative_position
     end
@@ -144,4 +141,5 @@ module RelativePositioning
 
     status
   end
+  # rubocop:enable Cop/ModuleWithInstanceVariables
 end
diff --git a/app/models/concerns/resolvable_discussion.rb b/app/models/concerns/resolvable_discussion.rb
index 43041a707d3..fc54a8bbca0 100644
--- a/app/models/concerns/resolvable_discussion.rb
+++ b/app/models/concerns/resolvable_discussion.rb
@@ -84,7 +84,6 @@ module ResolvableDiscussion
 
   private
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def update
     # Do not select `Note.resolvable`, so that system notes remain in the collection
     notes_relation = Note.where(id: notes.map(&:id))
diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb
index 05ddae42d2d..efec55d7376 100644
--- a/app/models/concerns/routable.rb
+++ b/app/models/concerns/routable.rb
@@ -86,10 +86,9 @@ module Routable
     end
   end
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def full_name
     if route && route.name.present?
-      @full_name ||= route.name
+      @full_name ||= route.name # rubocop:disable Cop/ModuleWithInstanceVariables
     else
       update_route if persisted?
 
@@ -113,7 +112,7 @@ module Routable
 
   def expires_full_path_cache
     RequestStore.delete(full_path_key) if RequestStore.active?
-    @full_path = nil
+    @full_path = nil # rubocop:disable Cop/ModuleWithInstanceVariables
   end
 
   def build_full_path
@@ -126,10 +125,9 @@ module Routable
 
   private
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def uncached_full_path
     if route && route.path.present?
-      @full_path ||= route.path
+      @full_path ||= route.path # rubocop:disable Cop/ModuleWithInstanceVariables
     else
       update_route if persisted?
 
@@ -164,12 +162,11 @@ module Routable
     route.save
   end
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def prepare_route
     route || build_route(source: self)
     route.path = build_full_path
     route.name = build_full_name
-    @full_path = nil
-    @full_name = nil
+    @full_path = nil # rubocop:disable Cop/ModuleWithInstanceVariables
+    @full_name = nil # rubocop:disable Cop/ModuleWithInstanceVariables
   end
 end
diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb
index a73de49b9bb..378a3ede2aa 100644
--- a/app/models/concerns/taskable.rb
+++ b/app/models/concerns/taskable.rb
@@ -36,11 +36,10 @@ module Taskable
   end
 
   # Called by `TaskList::Summary`
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def task_list_items
     return [] if description.blank?
 
-    @task_list_items ||= Taskable.get_tasks(description)
+    @task_list_items ||= Taskable.get_tasks(description) # rubocop:disable Cop/ModuleWithInstanceVariables
   end
 
   def tasks
diff --git a/app/models/concerns/time_trackable.rb b/app/models/concerns/time_trackable.rb
index 49438908c36..f1b43cb38e1 100644
--- a/app/models/concerns/time_trackable.rb
+++ b/app/models/concerns/time_trackable.rb
@@ -36,6 +36,7 @@ module TimeTrackable
     end
   end
   alias_method :spend_time=, :spend_time
+  # rubocop:enable Cop/ModuleWithInstanceVariables
 
   def total_time_spent
     timelogs.sum(:time_spent)
@@ -51,11 +52,11 @@ module TimeTrackable
 
   private
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def reset_spent_time
-    timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user)
+    timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) # rubocop:disable Cop/ModuleWithInstanceVariables
   end
 
+  # rubocop:disable Cop/ModuleWithInstanceVariables
   def add_or_subtract_spent_time
     timelogs.new(
       time_spent: time_spent,
@@ -63,17 +64,19 @@ module TimeTrackable
       spent_at: @spent_at
     )
   end
+  # rubocop:enable Cop/ModuleWithInstanceVariables
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def check_negative_time_spent
     return if time_spent.nil? || time_spent == :reset
 
-    # we need to cache the total time spent so multiple calls to #valid?
-    # doesn't give a false error
-    @original_total_time_spent ||= total_time_spent
-
-    if time_spent < 0 && (time_spent.abs > @original_total_time_spent)
+    if time_spent < 0 && (time_spent.abs > original_total_time_spent)
       errors.add(:time_spent, 'Time to subtract exceeds the total time spent')
     end
   end
+
+  # we need to cache the total time spent so multiple calls to #valid?
+  # doesn't give a false error
+  def original_total_time_spent
+    @original_total_time_spent ||= total_time_spent
+  end
 end