diff options
48 files changed, 193 insertions, 169 deletions
diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index 27f77af71d1..0f0a04a4ce1 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -1,5 +1,6 @@ module CreatesCommit extend ActiveSupport::Concern + include Gitlab::Utils::StrongMemoize # rubocop:disable Cop/ModuleWithInstanceVariables def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil) @@ -94,15 +95,20 @@ module CreatesCommit # rubocop:enable Cop/ModuleWithInstanceVariables def existing_merge_request_path - project_merge_request_path(@project, @merge_request) + project_merge_request_path(@project, @merge_request) # rubocop:disable Cop/ModuleWithInstanceVariables end # rubocop:disable Cop/ModuleWithInstanceVariables def merge_request_exists? - return @merge_request if defined?(@merge_request) - - @merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened - .find_by(source_project_id: @project_to_commit_into, source_branch: @branch_name, target_branch: @start_branch) + strong_memoize(:merge_request) do + MergeRequestsFinder.new(current_user, project_id: @project.id) + .execute + .opened + .find_by( + source_project_id: @project_to_commit_into, + source_branch: @branch_name, + target_branch: @start_branch) + end end # rubocop:enable Cop/ModuleWithInstanceVariables diff --git a/app/controllers/concerns/group_tree.rb b/app/controllers/concerns/group_tree.rb index 9d4f97aa443..418fcc4a18f 100644 --- a/app/controllers/concerns/group_tree.rb +++ b/app/controllers/concerns/group_tree.rb @@ -1,4 +1,5 @@ module GroupTree + # rubocop:disable Cop/ModuleWithInstanceVariables def render_group_tree(groups) @groups = if params[:filter].present? Gitlab::GroupHierarchy.new(groups.search(params[:filter])) @@ -20,5 +21,6 @@ module GroupTree render json: serializer.represent(@groups) end end + # rubocop:enable Cop/ModuleWithInstanceVariables end end diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 5b9c016bb8b..b1a7a53f94a 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -142,6 +142,7 @@ module IssuableActions @resource_name ||= controller_name.singularize end + # rubocop:disable Cop/ModuleWithInstanceVariables def render_entity_json if @issuable.valid? render json: serializer.represent(@issuable) @@ -149,6 +150,7 @@ module IssuableActions render json: { errors: @issuable.errors.full_messages }, status: :unprocessable_entity end end + # rubocop:enable Cop/ModuleWithInstanceVariables def serializer raise NotImplementedError @@ -159,6 +161,6 @@ module IssuableActions end def parent - @project || @group + @project || @group # rubocop:disable Cop/ModuleWithInstanceVariables end end diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index aa5bcbef7ea..9083c2b6b5c 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -2,6 +2,7 @@ module IssuableCollections extend ActiveSupport::Concern include SortingHelper include Gitlab::IssuableMetadata + include Gitlab::Utils::StrongMemoize included do helper_method :finder @@ -43,7 +44,7 @@ module IssuableCollections def redirect_out_of_range(total_pages) return false if total_pages.zero? - out_of_range = @issuables.current_page > total_pages + out_of_range = @issuables.current_page > total_pages # rubocop:disable Cop/ModuleWithInstanceVariables if out_of_range redirect_to(url_for(params.merge(page: total_pages, only_path: true))) @@ -53,7 +54,7 @@ module IssuableCollections end def issuable_page_count - page_count_for_relation(@issuables, finder.row_count) + page_count_for_relation(@issuables, finder.row_count) # rubocop:disable Cop/ModuleWithInstanceVariables end def page_count_for_relation(relation, row_count) @@ -133,9 +134,9 @@ module IssuableCollections end def finder - return @finder if defined?(@finder) - - @finder = issuable_finder_for(@finder_type) + strong_memoize(:finder) do + issuable_finder_for(@finder_type) # rubocop:disable Cop/ModuleWithInstanceVariables + end end def collection_type diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index be153d9fdbd..e6ef1f6f5e4 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -1,6 +1,6 @@ -# rubocop:disable Cop/ModuleWithInstanceVariables module NotesActions include RendersNotes + include Gitlab::Utils::StrongMemoize extend ActiveSupport::Concern included do @@ -31,6 +31,7 @@ module NotesActions render json: notes_json end + # rubocop:disable Cop/ModuleWithInstanceVariables def create create_params = note_params.merge( merge_request_diff_head_sha: params[:merge_request_diff_head_sha], @@ -48,7 +49,9 @@ module NotesActions format.html { redirect_back_or_default } end end + # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:disable Cop/ModuleWithInstanceVariables def update @note = Notes::UpdateService.new(project, current_user, note_params).execute(note) @@ -61,6 +64,7 @@ module NotesActions format.html { redirect_back_or_default } end end + # rubocop:enable Cop/ModuleWithInstanceVariables def destroy if note.editable? @@ -139,7 +143,7 @@ module NotesActions end else template = "discussions/_diff_discussion" - @fresh_discussion = true + @fresh_discussion = true # rubocop:disable Cop/ModuleWithInstanceVariables locals = { discussions: [discussion], on_image: on_image } end @@ -192,7 +196,7 @@ module NotesActions end def noteable - @noteable ||= notes_finder.target || @note&.noteable + @noteable ||= notes_finder.target || @note&.noteable # rubocop:disable Cop/ModuleWithInstanceVariables end def require_noteable! @@ -212,20 +216,21 @@ module NotesActions end def note_project - return @note_project if defined?(@note_project) - return nil unless project + strong_memoize(:note_project) do + return nil unless project - note_project_id = params[:note_project_id] + note_project_id = params[:note_project_id] - @note_project = - if note_project_id.present? - Project.find(note_project_id) - else - project - end + the_project = + if note_project_id.present? + Project.find(note_project_id) + else + project + end - return access_denied! unless can?(current_user, :create_note, @note_project) + return access_denied! unless can?(current_user, :create_note, the_project) - @note_project + the_project + end end end diff --git a/app/controllers/concerns/preview_markdown.rb b/app/controllers/concerns/preview_markdown.rb index 5ce602b55a8..01c95612922 100644 --- a/app/controllers/concerns/preview_markdown.rb +++ b/app/controllers/concerns/preview_markdown.rb @@ -1,6 +1,7 @@ module PreviewMarkdown extend ActiveSupport::Concern + # rubocop:disable Cop/ModuleWithInstanceVariables def preview_markdown result = PreviewMarkdownService.new(@project, current_user, params).execute @@ -19,4 +20,5 @@ module PreviewMarkdown } } end + # rubocop:enable Cop/ModuleWithInstanceVariables end diff --git a/app/controllers/concerns/renders_commits.rb b/app/controllers/concerns/renders_commits.rb index 675fefd0d36..7b8cee435ee 100644 --- a/app/controllers/concerns/renders_commits.rb +++ b/app/controllers/concerns/renders_commits.rb @@ -1,7 +1,6 @@ module RendersCommits - # rubocop:disable Cop/ModuleWithInstanceVariables def prepare_commits_for_rendering(commits) - Banzai::CommitRenderer.render(commits, @project, current_user) + Banzai::CommitRenderer.render(commits, @project, current_user) # rubocop:disable Cop/ModuleWithInstanceVariables commits end diff --git a/app/controllers/concerns/renders_notes.rb b/app/controllers/concerns/renders_notes.rb index 754e88660bf..4ca6d9db5cf 100644 --- a/app/controllers/concerns/renders_notes.rb +++ b/app/controllers/concerns/renders_notes.rb @@ -8,6 +8,7 @@ module RendersNotes notes end + # rubocop:enable Cop/ModuleWithInstanceVariables private diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb index ce60267f345..47b10c3ef3b 100644 --- a/app/controllers/concerns/service_params.rb +++ b/app/controllers/concerns/service_params.rb @@ -65,9 +65,8 @@ module ServiceParams # Parameters to ignore if no value is specified FILTER_BLANK_PARAMS = [:password].freeze - # rubocop:disable Cop/ModuleWithInstanceVariables def service_params - dynamic_params = @service.event_channel_names + @service.event_names + dynamic_params = @service.event_channel_names + @service.event_names # rubocop:disable Cop/ModuleWithInstanceVariables service_params = params.permit(:id, service: ALLOWED_PARAMS_CE + dynamic_params) if service_params[:service].is_a?(Hash) diff --git a/app/controllers/concerns/snippets_actions.rb b/app/controllers/concerns/snippets_actions.rb index 4216c1fe063..046dae480c1 100644 --- a/app/controllers/concerns/snippets_actions.rb +++ b/app/controllers/concerns/snippets_actions.rb @@ -15,6 +15,7 @@ module SnippetsActions filename: @snippet.sanitized_file_name ) end + # rubocop:enable Cop/ModuleWithInstanceVariables private diff --git a/app/controllers/concerns/spammable_actions.rb b/app/controllers/concerns/spammable_actions.rb index ef6e14c9e4c..c9cddc7a1ba 100644 --- a/app/controllers/concerns/spammable_actions.rb +++ b/app/controllers/concerns/spammable_actions.rb @@ -2,6 +2,7 @@ module SpammableActions extend ActiveSupport::Concern include Recaptcha::Verify + include Gitlab::Utils::StrongMemoize included do before_action :authorize_submit_spammable!, only: :mark_as_spam @@ -17,11 +18,10 @@ module SpammableActions private - # rubocop:disable Cop/ModuleWithInstanceVariables def ensure_spam_config_loaded! - return @spam_config_loaded if defined?(@spam_config_loaded) - - @spam_config_loaded = Gitlab::Recaptcha.load_configurations! + strong_memoize(:spam_config_loaded) do + Gitlab::Recaptcha.load_configurations! + end end def recaptcha_check_with_fallback(&fallback) diff --git a/app/controllers/concerns/toggle_subscription_action.rb b/app/controllers/concerns/toggle_subscription_action.rb index 0a6d40d36ea..776583579e8 100644 --- a/app/controllers/concerns/toggle_subscription_action.rb +++ b/app/controllers/concerns/toggle_subscription_action.rb @@ -11,9 +11,8 @@ module ToggleSubscriptionAction private - # rubocop:disable Cop/ModuleWithInstanceVariables def subscribable_project - @project || raise(NotImplementedError) + @project ||= raise(NotImplementedError) end def subscribable_resource 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 diff --git a/app/serializers/concerns/with_pagination.rb b/app/serializers/concerns/with_pagination.rb index d29e22d6740..89631b73fcf 100644 --- a/app/serializers/concerns/with_pagination.rb +++ b/app/serializers/concerns/with_pagination.rb @@ -14,7 +14,7 @@ module WithPagination # we shouldn't try to paginate single resources def represent(resource, opts = {}) if paginated? && resource.respond_to?(:page) - super(@paginator.paginate(resource), opts) + super(paginator.paginate(resource), opts) else super(resource, opts) end diff --git a/app/services/concerns/issues/resolve_discussions.rb b/app/services/concerns/issues/resolve_discussions.rb index df78ffbb6bd..fc312aad8bd 100644 --- a/app/services/concerns/issues/resolve_discussions.rb +++ b/app/services/concerns/issues/resolve_discussions.rb @@ -1,5 +1,7 @@ module Issues module ResolveDiscussions + include Gitlab::Utils::StrongMemoize + attr_reader :merge_request_to_resolve_discussions_of_iid, :discussion_to_resolve_id # rubocop:disable Cop/ModuleWithInstanceVariables @@ -7,21 +9,20 @@ module Issues @merge_request_to_resolve_discussions_of_iid ||= params.delete(:merge_request_to_resolve_discussions_of) @discussion_to_resolve_id ||= params.delete(:discussion_to_resolve) end + # rubocop:enable Cop/ModuleWithInstanceVariables - # rubocop:disable Cop/ModuleWithInstanceVariables def merge_request_to_resolve_discussions_of - return @merge_request_to_resolve_discussions_of if defined?(@merge_request_to_resolve_discussions_of) - - @merge_request_to_resolve_discussions_of = MergeRequestsFinder.new(current_user, project_id: project.id) - .execute - .find_by(iid: merge_request_to_resolve_discussions_of_iid) + strong_memoize(:merge_request_to_resolve_discussions_of) do + MergeRequestsFinder.new(current_user, project_id: project.id) + .execute + .find_by(iid: merge_request_to_resolve_discussions_of_iid) + end end - # rubocop:disable Cop/ModuleWithInstanceVariables def discussions_to_resolve return [] unless merge_request_to_resolve_discussions_of - @discussions_to_resolve ||= + @discussions_to_resolve ||= # rubocop:disable Cop/ModuleWithInstanceVariables if discussion_to_resolve_id discussion_or_nil = merge_request_to_resolve_discussions_of .find_discussion(discussion_to_resolve_id) diff --git a/app/services/spam_check_service.rb b/app/services/spam_check_service.rb index 9b2a601f84b..31ec1e9713e 100644 --- a/app/services/spam_check_service.rb +++ b/app/services/spam_check_service.rb @@ -14,6 +14,7 @@ module SpamCheckService @recaptcha_verified = params.delete(:recaptcha_verified) @spam_log_id = params.delete(:spam_log_id) end + # rubocop:enable Cop/ModuleWithInstanceVariables # In order to be proceed to the spam check process, @spammable has to be # a dirty instance, which means it should be already assigned with the new @@ -26,4 +27,5 @@ module SpamCheckService user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true) end end + # rubocop:enable Cop/ModuleWithInstanceVariables end diff --git a/app/workers/concerns/new_issuable.rb b/app/workers/concerns/new_issuable.rb index b6aea921aae..9a15e79d0c3 100644 --- a/app/workers/concerns/new_issuable.rb +++ b/app/workers/concerns/new_issuable.rb @@ -8,18 +8,16 @@ module NewIssuable user && issuable end - # rubocop:disable Cop/ModuleWithInstanceVariables def set_user(user_id) - @user = User.find_by(id: user_id) + @user = User.find_by(id: user_id) # rubocop:disable Cop/ModuleWithInstanceVariables - log_error(User, user_id) unless @user + log_error(User, user_id) unless @user # rubocop:disable Cop/ModuleWithInstanceVariables end - # rubocop:disable Cop/ModuleWithInstanceVariables def set_issuable(issuable_id) - @issuable = issuable_class.find_by(id: issuable_id) + @issuable = issuable_class.find_by(id: issuable_id) # rubocop:disable Cop/ModuleWithInstanceVariables - log_error(issuable_class, issuable_id) unless @issuable + log_error(issuable_class, issuable_id) unless @issuable # rubocop:disable Cop/ModuleWithInstanceVariables end def log_error(record_class, record_id) diff --git a/config/initializers/fix_local_cache_middleware.rb b/config/initializers/fix_local_cache_middleware.rb index c9abe0c1255..1f043408b4e 100644 --- a/config/initializers/fix_local_cache_middleware.rb +++ b/config/initializers/fix_local_cache_middleware.rb @@ -4,10 +4,9 @@ module LocalCacheRegistryCleanupWithEnsure LocalStore = ActiveSupport::Cache::Strategy::LocalCache::LocalStore - # rubocop:disable Cop/ModuleWithInstanceVariables def call(env) LocalCacheRegistry.set_cache_for(local_cache_key, LocalStore.new) - response = @app.call(env) + response = @app.call(env) # rubocop:disable Cop/ModuleWithInstanceVariables response[2] = ::Rack::BodyProxy.new(response[2]) do LocalCacheRegistry.set_cache_for(local_cache_key, nil) end diff --git a/config/initializers/rspec_profiling.rb b/config/initializers/rspec_profiling.rb index d6c75a611b9..c732e303f03 100644 --- a/config/initializers/rspec_profiling.rb +++ b/config/initializers/rspec_profiling.rb @@ -16,14 +16,13 @@ module RspecProfilingExt end module Run - # rubocop:disable Cop/ModuleWithInstanceVariables def example_finished(*args) super rescue => err - return if @already_logged_example_finished_error + return if @already_logged_example_finished_error # rubocop:disable Cop/ModuleWithInstanceVariables $stderr.puts "rspec_profiling couldn't collect an example: #{err}. Further warnings suppressed." - @already_logged_example_finished_error = true + @already_logged_example_finished_error = true # rubocop:disable Cop/ModuleWithInstanceVariables end alias_method :example_passed, :example_finished diff --git a/config/initializers/rugged_use_gitlab_git_attributes.rb b/config/initializers/rugged_use_gitlab_git_attributes.rb index abfa2c354cd..1cfb3bcb4bd 100644 --- a/config/initializers/rugged_use_gitlab_git_attributes.rb +++ b/config/initializers/rugged_use_gitlab_git_attributes.rb @@ -14,10 +14,12 @@ module Rugged class Repository module UseGitlabGitAttributes - # rubocop:disable Cop/ModuleWithInstanceVariables def fetch_attributes(name, *) + attributes.attributes(name) + end + + def attributes @attributes ||= Gitlab::Git::Attributes.new(path) - @attributes.attributes(name) end end 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 diff --git a/qa/qa/runtime/scenario.rb b/qa/qa/runtime/scenario.rb index 7ef59046640..15d4112d347 100644 --- a/qa/qa/runtime/scenario.rb +++ b/qa/qa/runtime/scenario.rb @@ -6,13 +6,15 @@ module QA module Scenario extend self - attr_reader :attributes + def attributes + @attributes ||= {} + end def define(attribute, value) - (@attributes ||= {}).store(attribute.to_sym, value) + attributes.store(attribute.to_sym, value) define_singleton_method(attribute) do - @attributes[attribute.to_sym].tap do |value| + attributes[attribute.to_sym].tap do |value| if value.to_s.empty? raise ArgumentError, "Empty `#{attribute}` attribute!" end |