Use StrongMemoize and enable/disable cops properly

20 files changed, 80 insertions, 75 deletions

diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 7f436b69091..c4f81443282 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -33,6 +33,10 @@ module API
     end
 
     # rubocop:disable Cop/ModuleWithInstanceVariables
+    # We can't rewrite this with StrongMemoize because `sudo!` would
+    # actually write to `@current_user`, and `sudo?` would immediately
+    # call `current_user` again which reads from `@current_user`.
+    # We should rewrite this in a way that using StrongMemoize is possible
     def current_user
       return @current_user if defined?(@current_user)
 
@@ -46,6 +50,7 @@ module API
 
       @current_user
     end
+    # rubocop:enable Cop/ModuleWithInstanceVariables
 
     def sudo?
       initial_current_user != current_user
@@ -394,6 +399,7 @@ module API
 
     private
 
+    # rubocop:disable Cop/ModuleWithInstanceVariables
     def initial_current_user
       return @initial_current_user if defined?(@initial_current_user)
 
@@ -403,8 +409,8 @@ module API
         unauthorized!
       end
     end
+    # rubocop:enable Cop/ModuleWithInstanceVariables
 
-    # rubocop:disable Cop/ModuleWithInstanceVariables
     def sudo!
       return unless sudo_identifier
 
@@ -423,7 +429,7 @@ module API
       sudoed_user = find_user(sudo_identifier)
       not_found!("User with ID or username '#{sudo_identifier}'") unless sudoed_user
 
-      @current_user = sudoed_user
+      @current_user = sudoed_user # rubocop:disable Cop/ModuleWithInstanceVariables
     end
 
     def sudo_identifier
diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb
index 0d57c822578..e2077d33b9f 100644
--- a/lib/api/helpers/internal_helpers.rb
+++ b/lib/api/helpers/internal_helpers.rb
@@ -8,16 +8,14 @@ module API
 
       attr_reader :redirected_path
 
-      # rubocop:disable Cop/ModuleWithInstanceVariables
       def wiki?
-        set_project unless defined?(@wiki)
-        @wiki
+        set_project unless defined?(@wiki) # rubocop:disable Cop/ModuleWithInstanceVariables
+        @wiki # rubocop:disable Cop/ModuleWithInstanceVariables
       end
 
-      # rubocop:disable Cop/ModuleWithInstanceVariables
       def project
-        set_project unless defined?(@project)
-        @project
+        set_project unless defined?(@project) # rubocop:disable Cop/ModuleWithInstanceVariables
+        @project # rubocop:disable Cop/ModuleWithInstanceVariables
       end
 
       def ssh_authentication_abilities
@@ -78,6 +76,7 @@ module API
           @project, @wiki, @redirected_path = Gitlab::RepoPath.parse(params[:project])
         end
       end
+      # rubocop:enable Cop/ModuleWithInstanceVariables
 
       # Project id to pass between components that don't share/don't have
       # access to the same filesystem mounts
diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb
index 9e01eed06f3..40a65aad631 100644
--- a/lib/extracts_path.rb
+++ b/lib/extracts_path.rb
@@ -37,11 +37,10 @@ module ExtractsPath
   #
   # Returns an Array where the first value is the tree-ish and the second is the
   # path
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def extract_ref(id)
     pair = ['', '']
 
-    return pair unless @project
+    return pair unless @project # rubocop:disable Cop/ModuleWithInstanceVariables
 
     if id =~ /^(\h{40})(.+)/
       # If the ref appears to be a SHA, we're done, just split the string
@@ -133,10 +132,10 @@ module ExtractsPath
   rescue RuntimeError, NoMethodError, InvalidPathError
     render_404
   end
+  # rubocop:enable Cop/ModuleWithInstanceVariables
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def tree
-    @tree ||= @repo.tree(@commit.id, @path)
+    @tree ||= @repo.tree(@commit.id, @path) # rubocop:disable Cop/ModuleWithInstanceVariables
   end
 
   private
@@ -148,10 +147,9 @@ module ExtractsPath
     id
   end
 
-  # rubocop:disable Cop/ModuleWithInstanceVariables
   def ref_names
-    return [] unless @project
+    return [] unless @project # rubocop:disable Cop/ModuleWithInstanceVariables
 
-    @ref_names ||= @project.repository.ref_names
+    @ref_names ||= @project.repository.ref_names # rubocop:disable Cop/ModuleWithInstanceVariables
   end
 end
diff --git a/lib/gitlab/cache/request_cache.rb b/lib/gitlab/cache/request_cache.rb
index 65e3e672894..ecc85f847d4 100644
--- a/lib/gitlab/cache/request_cache.rb
+++ b/lib/gitlab/cache/request_cache.rb
@@ -45,12 +45,13 @@ module Gitlab
         klass.prepend(extension)
       end
 
-      # rubocop:disable Cop/ModuleWithInstanceVariables
+      attr_accessor :request_cache_key_block
+
       def request_cache_key(&block)
         if block_given?
-          @request_cache_key = block
+          self.request_cache_key_block = block
         else
-          @request_cache_key
+          request_cache_key_block
         end
       end
 
diff --git a/lib/gitlab/ci/charts.rb b/lib/gitlab/ci/charts.rb
index fe2fd08a67a..e94166aee0c 100644
--- a/lib/gitlab/ci/charts.rb
+++ b/lib/gitlab/ci/charts.rb
@@ -2,12 +2,11 @@ module Gitlab
   module Ci
     module Charts
       module DailyInterval
-        # rubocop:disable Cop/ModuleWithInstanceVariables
         def grouped_count(query)
           query
             .group("DATE(#{::Ci::Pipeline.table_name}.created_at)")
             .count(:created_at)
-            .transform_keys { |date| date.strftime(@format) }
+            .transform_keys { |date| date.strftime(@format) } # rubocop:disable Cop/ModuleWithInstanceVariables
         end
 
         def interval_step
diff --git a/lib/gitlab/ci/config/entry/configurable.rb b/lib/gitlab/ci/config/entry/configurable.rb
index 2c96e5f65d7..63b803c15af 100644
--- a/lib/gitlab/ci/config/entry/configurable.rb
+++ b/lib/gitlab/ci/config/entry/configurable.rb
@@ -24,21 +24,20 @@ module Gitlab
             end
           end
 
-          # rubocop:disable Cop/ModuleWithInstanceVariables
           def compose!(deps = nil)
             return unless valid?
 
             self.class.nodes.each do |key, factory|
               factory
-                .value(@config[key])
+                .value(config[key])
                 .with(key: key, parent: self)
 
-              @entries[key] = factory.create!
+              entries[key] = factory.create!
             end
 
             yield if block_given?
 
-            @entries.each_value do |entry|
+            entries.each_value do |entry|
               entry.compose!(deps)
             end
           end
diff --git a/lib/gitlab/ci/config/entry/node.rb b/lib/gitlab/ci/config/entry/node.rb
index c868943c42e..1fba0b2db0b 100644
--- a/lib/gitlab/ci/config/entry/node.rb
+++ b/lib/gitlab/ci/config/entry/node.rb
@@ -90,6 +90,12 @@ module Gitlab
           def self.aspects
             @aspects ||= []
           end
+
+          private
+
+          def entries
+            @entries
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/entry/validatable.rb b/lib/gitlab/ci/config/entry/validatable.rb
index 1850c652c09..0dc359a86c3 100644
--- a/lib/gitlab/ci/config/entry/validatable.rb
+++ b/lib/gitlab/ci/config/entry/validatable.rb
@@ -12,9 +12,8 @@ module Gitlab
             end
           end
 
-          # rubocop:disable Cop/ModuleWithInstanceVariables
           def errors
-            @validator.messages + descendants.flat_map(&:errors)
+            @validator.messages + descendants.flat_map(&:errors) # rubocop:disable Cop/ModuleWithInstanceVariables
           end
 
           class_methods do
diff --git a/lib/gitlab/ci/pipeline/chain/helpers.rb b/lib/gitlab/ci/pipeline/chain/helpers.rb
index 36ed87dbd32..7ab7a64c7e3 100644
--- a/lib/gitlab/ci/pipeline/chain/helpers.rb
+++ b/lib/gitlab/ci/pipeline/chain/helpers.rb
@@ -3,21 +3,19 @@ module Gitlab
     module Pipeline
       module Chain
         module Helpers
-          # rubocop:disable Cop/ModuleWithInstanceVariables
-          def branch_exists?
-            return @is_branch if defined?(@is_branch)
+          include Gitlab::Utils::StrongMemoize
 
-            @is_branch = project.repository.branch_exists?(pipeline.ref)
+          def branch_exists?
+            strong_memoize(:is_branch) do
+              project.repository.branch_exists?(pipeline.ref)
+            end
           end
-          # rubocop:enable Cop/ModuleWithInstanceVariables
 
-          # rubocop:disable Cop/ModuleWithInstanceVariables
           def tag_exists?
-            return @is_tag if defined?(@is_tag)
-
-            @is_tag = project.repository.tag_exists?(pipeline.ref)
+            strong_memoize(:is_tag) do
+              project.repository.tag_exists?(pipeline.ref)
+            end
           end
-          # rubocop:enable Cop/ModuleWithInstanceVariables
 
           def error(message)
             pipeline.errors.add(:base, message)
diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
index 4e0dadcc3c7..dfb2cfe42b7 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -52,9 +52,8 @@ module Gitlab
       ::ApplicationSetting.create_from_defaults || in_memory_application_settings
     end
 
-    # rubocop:disable Cop/ModuleWithInstanceVariables
     def in_memory_application_settings
-      @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults)
+      @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) # rubocop:disable Cop/ModuleWithInstanceVariables
     rescue ActiveRecord::StatementInvalid, ActiveRecord::UnknownAttributeError
       # In case migrations the application_settings table is not created yet,
       # we fallback to a simple OpenStruct
diff --git a/lib/gitlab/cycle_analytics/base_query.rb b/lib/gitlab/cycle_analytics/base_query.rb
index 52fdae84c58..05b4928c3b9 100644
--- a/lib/gitlab/cycle_analytics/base_query.rb
+++ b/lib/gitlab/cycle_analytics/base_query.rb
@@ -11,13 +11,12 @@ module Gitlab
         @base_query ||= stage_query
       end
 
-      # rubocop:disable Cop/ModuleWithInstanceVariables
       def stage_query
         query = mr_closing_issues_table.join(issue_table).on(issue_table[:id].eq(mr_closing_issues_table[:issue_id]))
           .join(issue_metrics_table).on(issue_table[:id].eq(issue_metrics_table[:issue_id]))
-          .where(issue_table[:project_id].eq(@project.id))
+          .where(issue_table[:project_id].eq(@project.id)) # rubocop:disable Cop/ModuleWithInstanceVariables
           .where(issue_table[:deleted_at].eq(nil))
-          .where(issue_table[:created_at].gteq(@options[:from]))
+          .where(issue_table[:created_at].gteq(@options[:from])) # rubocop:disable Cop/ModuleWithInstanceVariables
 
         # Load merge_requests
         query = query.join(mr_table, Arel::Nodes::OuterJoin)
diff --git a/lib/gitlab/cycle_analytics/production_helper.rb b/lib/gitlab/cycle_analytics/production_helper.rb
index 9a05c910ba0..ebbff1b2f33 100644
--- a/lib/gitlab/cycle_analytics/production_helper.rb
+++ b/lib/gitlab/cycle_analytics/production_helper.rb
@@ -1,9 +1,10 @@
 module Gitlab
   module CycleAnalytics
     module ProductionHelper
-      # rubocop:disable Cop/ModuleWithInstanceVariables
       def stage_query
-        super.where(mr_metrics_table[:first_deployed_to_production_at].gteq(@options[:from]))
+        super
+          .where(mr_metrics_table[:first_deployed_to_production_at]
+          .gteq(@options[:from])) # rubocop:disable Cop/ModuleWithInstanceVariables
       end
     end
   end
diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb
index 6959fa74293..9ae1f66a182 100644
--- a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb
+++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb
@@ -3,11 +3,10 @@ module Gitlab
     module RenameReservedPathsMigration
       module V1
         module MigrationClasses
-          # rubocop:disable Cop/ModuleWithInstanceVariables
           module Routable
             def full_path
               if route && route.path.present?
-                @full_path ||= route.path
+                @full_path ||= route.path # rubocop:disable Cop/ModuleWithInstanceVariables
               else
                 update_route if persisted?
 
@@ -31,7 +30,7 @@ module Gitlab
             def prepare_route
               route || build_route(source: self)
               route.path = build_full_path
-              @full_path = nil
+              @full_path = nil # rubocop:disable Cop/ModuleWithInstanceVariables
             end
           end
 
diff --git a/lib/gitlab/import_export/command_line_util.rb b/lib/gitlab/import_export/command_line_util.rb
index 5ac57c5775a..19dab0037bb 100644
--- a/lib/gitlab/import_export/command_line_util.rb
+++ b/lib/gitlab/import_export/command_line_util.rb
@@ -30,10 +30,9 @@ module Gitlab
         execute(%W(tar -#{options} #{archive} -C #{dir}))
       end
 
-      # rubocop:disable Cop/ModuleWithInstanceVariables
       def execute(cmd)
         output, status = Gitlab::Popen.popen(cmd)
-        @shared.error(Gitlab::ImportExport::Error.new(output.to_s)) unless status.zero?
+        @shared.error(Gitlab::ImportExport::Error.new(output.to_s)) unless status.zero? # rubocop:disable Cop/ModuleWithInstanceVariables
         status.zero?
       end
 
diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb
index 3c5f9099584..fa88d41be73 100644
--- a/lib/gitlab/metrics/influx_db.rb
+++ b/lib/gitlab/metrics/influx_db.rb
@@ -171,6 +171,7 @@ module Gitlab
           @pool
         end
       end
+      # rubocop:enable Cop/ModuleWithInstanceVariables
     end
   end
 end
diff --git a/lib/gitlab/metrics/prometheus.rb b/lib/gitlab/metrics/prometheus.rb
index 75461b45005..b0b8e8436db 100644
--- a/lib/gitlab/metrics/prometheus.rb
+++ b/lib/gitlab/metrics/prometheus.rb
@@ -4,6 +4,7 @@ module Gitlab
   module Metrics
     module Prometheus
       include Gitlab::CurrentSettings
+      include Gitlab::Utils::StrongMemoize
 
       REGISTRY_MUTEX = Mutex.new
       PROVIDER_MUTEX = Mutex.new
@@ -16,18 +17,19 @@ module Gitlab
           ::File.writable?(multiprocess_files_dir)
       end
 
-      # rubocop:disable Cop/ModuleWithInstanceVariables
       def prometheus_metrics_enabled?
-        return @prometheus_metrics_enabled if defined?(@prometheus_metrics_enabled)
-
-        @prometheus_metrics_enabled = prometheus_metrics_enabled_unmemoized
+        strong_memoize(:prometheus_metrics_enabled) do
+          prometheus_metrics_enabled_unmemoized
+        end
       end
 
       def registry
-        return @registry if @registry
-
-        REGISTRY_MUTEX.synchronize do
-          @registry ||= ::Prometheus::Client.registry
+        strong_memoize(:registry) do
+          REGISTRY_MUTEX.synchronize do
+            strong_memoize(:registry) do
+              ::Prometheus::Client.registry
+            end
+          end
         end
       end
 
diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb
index 9a3817ff00a..9a91f8bf96a 100644
--- a/lib/gitlab/path_regex.rb
+++ b/lib/gitlab/path_regex.rb
@@ -1,4 +1,3 @@
-# rubocop:disable Cop/ModuleWithInstanceVariables
 module Gitlab
   module PathRegex
     extend self
diff --git a/lib/gitlab/slash_commands/presenters/issue_base.rb b/lib/gitlab/slash_commands/presenters/issue_base.rb
index 5aaf3655396..31c1e97efba 100644
--- a/lib/gitlab/slash_commands/presenters/issue_base.rb
+++ b/lib/gitlab/slash_commands/presenters/issue_base.rb
@@ -10,36 +10,37 @@ module Gitlab
           issuable.open? ? 'Open' : 'Closed'
         end
 
-        # rubocop:disable Cop/ModuleWithInstanceVariables
         def project
-          @resource.project
+          resource.project
         end
 
-        # rubocop:disable Cop/ModuleWithInstanceVariables
         def author
-          @resource.author
+          resource.author
         end
 
-        # rubocop:disable Cop/ModuleWithInstanceVariables
         def fields
           [
             {
               title: "Assignee",
-              value: @resource.assignees.any? ? @resource.assignees.first.name : "_None_",
+              value: resource.assignees.any? ? resource.assignees.first.name : "_None_",
               short: true
             },
             {
               title: "Milestone",
-              value: @resource.milestone ? @resource.milestone.title : "_None_",
+              value: resource.milestone ? resource.milestone.title : "_None_",
               short: true
             },
             {
               title: "Labels",
-              value: @resource.labels.any? ? @resource.label_names.join(', ') : "_None_",
+              value: resource.labels.any? ? resource.label_names.join(', ') : "_None_",
               short: true
             }
           ]
         end
+
+        private
+
+        attr_reader :resource
       end
     end
   end
diff --git a/lib/tasks/gettext.rake b/lib/tasks/gettext.rake
index 35ba729c156..247d7be7d78 100644
--- a/lib/tasks/gettext.rake
+++ b/lib/tasks/gettext.rake
@@ -23,6 +23,7 @@ namespace :gettext do
   desc 'Lint all po files in `locale/'
   task lint: :environment do
     require 'simple_po_parser'
+    require 'gitlab/utils'
 
     FastGettext.silence_errors
     files = Dir.glob(Rails.root.join('locale/*/gitlab.po'))
diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb
index 2feacbb4a09..6723662703c 100644
--- a/lib/tasks/gitlab/task_helpers.rb
+++ b/lib/tasks/gitlab/task_helpers.rb
@@ -1,10 +1,13 @@
 require 'rainbow/ext/string'
+require 'gitlab/utils/strong_memoize'
 
 module Gitlab
   TaskFailedError = Class.new(StandardError)
   TaskAbortedByUserError = Class.new(StandardError)
 
   module TaskHelpers
+    include Gitlab::Utils::StrongMemoize
+
     extend self
 
     # Ask if the user wants to continue
@@ -104,19 +107,17 @@ module Gitlab
       Gitlab.config.gitlab.user
     end
 
-    # rubocop:disable Cop/ModuleWithInstanceVariables
     def gitlab_user?
-      return @is_gitlab_user unless @is_gitlab_user.nil?
-
-      current_user = run_command(%w(whoami)).chomp
-      @is_gitlab_user = current_user == gitlab_user
+      strong_memoize(:is_gitlab_user) do
+        current_user = run_command(%w(whoami)).chomp
+        current_user == gitlab_user
+      end
     end
 
-    # rubocop:disable Cop/ModuleWithInstanceVariables
     def warn_user_is_not_gitlab
-      return if @warned_user_not_gitlab
+      return if gitlab_user?
 
-      unless gitlab_user?
+      strong_memoize(:warned_user_not_gitlab) do
         current_user = run_command(%w(whoami)).chomp
 
         puts " Warning ".color(:black).background(:yellow)
@@ -124,8 +125,6 @@ module Gitlab
         puts "  Things may work\/fail for the wrong reasons."
         puts "  For correct results you should run this as user #{gitlab_user.color(:magenta)}."
         puts ""
-
-        @warned_user_not_gitlab = true
       end
     end