diff options
43 files changed, 198 insertions, 240 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index c427f219a0d..d103a14518f 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1185,7 +1185,20 @@ RSpec/SubjectStub: RSpec/VerifiedDoubles: Enabled: false -# GitlabSecurity ############################################################## +# Gitlab ################################################################### + +Gitlab/ModuleWithInstanceVariables: + Enable: true + Exclude: + # We ignore Rails helpers right now because it's hard to workaround it + - app/helpers/*_helper.rb + # We ignore Rails mailers right now because it's hard to workaround it + - app/mailers/emails/*.rb + # We ignore spec helpers because it usually doesn't matter + - spec/support/**/*.rb + - features/steps/**/*.rb + +# GitlabSecurity ########################################################### GitlabSecurity/DeepMunge: Enabled: true diff --git a/app/controllers/concerns/boards_responses.rb b/app/controllers/concerns/boards_responses.rb index 2246ad7a4e6..a145049dc7d 100644 --- a/app/controllers/concerns/boards_responses.rb +++ b/app/controllers/concerns/boards_responses.rb @@ -24,11 +24,11 @@ module BoardsResponses end def respond_with_boards - respond_with(@boards) # rubocop:disable Cop/ModuleWithInstanceVariables + respond_with(@boards) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def respond_with_board - respond_with(@board) # rubocop:disable Cop/ModuleWithInstanceVariables + respond_with(@board) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def respond_with(resource) diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index 0f0a04a4ce1..6f4fdcdaa4f 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -2,7 +2,7 @@ module CreatesCommit extend ActiveSupport::Concern include Gitlab::Utils::StrongMemoize - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil) if can?(current_user, :push_code, @project) @project_to_commit_into = @project @@ -47,7 +47,7 @@ module CreatesCommit end end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def authorize_edit_tree! return if can_collaborate_with_project? @@ -80,7 +80,7 @@ module CreatesCommit end end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def new_merge_request_path project_new_merge_request_path( @project_to_commit_into, @@ -92,13 +92,13 @@ module CreatesCommit } ) end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def existing_merge_request_path - project_merge_request_path(@project, @merge_request) # rubocop:disable Cop/ModuleWithInstanceVariables + project_merge_request_path(@project, @merge_request) # rubocop:disable Gitlab/ModuleWithInstanceVariables end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def merge_request_exists? strong_memoize(:merge_request) do MergeRequestsFinder.new(current_user, project_id: @project.id) @@ -110,10 +110,10 @@ module CreatesCommit target_branch: @start_branch) end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def different_project? - @project_to_commit_into != @project # rubocop:disable Cop/ModuleWithInstanceVariables + @project_to_commit_into != @project # rubocop:disable Gitlab/ModuleWithInstanceVariables end def create_merge_request? @@ -121,6 +121,6 @@ module CreatesCommit # as the target branch in the same project, # we don't want to create a merge request. params[:create_merge_request].present? && - (different_project? || @start_branch != @branch_name) # rubocop:disable Cop/ModuleWithInstanceVariables + (different_project? || @start_branch != @branch_name) # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/app/controllers/concerns/group_tree.rb b/app/controllers/concerns/group_tree.rb index 418fcc4a18f..b10147835f3 100644 --- a/app/controllers/concerns/group_tree.rb +++ b/app/controllers/concerns/group_tree.rb @@ -1,5 +1,5 @@ module GroupTree - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def render_group_tree(groups) @groups = if params[:filter].present? Gitlab::GroupHierarchy.new(groups.search(params[:filter])) @@ -21,6 +21,6 @@ module GroupTree render json: serializer.represent(@groups) end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables end end diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index b1a7a53f94a..a725aad43e7 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -17,7 +17,7 @@ module IssuableActions end def update - @issuable = update_service.execute(issuable) # rubocop:disable Cop/ModuleWithInstanceVariables + @issuable = update_service.execute(issuable) # rubocop:disable Gitlab/ModuleWithInstanceVariables respond_to do |format| format.html do @@ -83,7 +83,7 @@ module IssuableActions def render_conflict_response respond_to do |format| format.html do - @conflict = true # rubocop:disable Cop/ModuleWithInstanceVariables + @conflict = true # rubocop:disable Gitlab/ModuleWithInstanceVariables render :edit end @@ -98,7 +98,7 @@ module IssuableActions end def labels - @labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute # rubocop:disable Cop/ModuleWithInstanceVariables + @labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute # rubocop:disable Gitlab/ModuleWithInstanceVariables end def authorize_destroy_issuable! @@ -108,7 +108,7 @@ module IssuableActions end def authorize_admin_issuable! - unless can?(current_user, :"admin_#{resource_name}", @project) # rubocop:disable Cop/ModuleWithInstanceVariables + unless can?(current_user, :"admin_#{resource_name}", @project) # rubocop:disable Gitlab/ModuleWithInstanceVariables return access_denied! end end @@ -142,7 +142,7 @@ module IssuableActions @resource_name ||= controller_name.singularize end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def render_entity_json if @issuable.valid? render json: serializer.represent(@issuable) @@ -150,7 +150,7 @@ module IssuableActions render json: { errors: @issuable.errors.full_messages }, status: :unprocessable_entity end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def serializer raise NotImplementedError @@ -161,6 +161,6 @@ module IssuableActions end def parent - @project || @group # rubocop:disable Cop/ModuleWithInstanceVariables + @project || @group # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 9083c2b6b5c..cfee2071472 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -10,7 +10,7 @@ module IssuableCollections private - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def set_issuables_index @issuables = issuables_collection @issuables = @issuables.page(params[:page]) @@ -35,7 +35,7 @@ module IssuableCollections @users.push(author) if author end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def issuables_collection finder.execute.preload(preload_for_collection) @@ -44,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 # rubocop:disable Cop/ModuleWithInstanceVariables + out_of_range = @issuables.current_page > total_pages # rubocop:disable Gitlab/ModuleWithInstanceVariables if out_of_range redirect_to(url_for(params.merge(page: total_pages, only_path: true))) @@ -54,7 +54,7 @@ module IssuableCollections end def issuable_page_count - page_count_for_relation(@issuables, finder.row_count) # rubocop:disable Cop/ModuleWithInstanceVariables + page_count_for_relation(@issuables, finder.row_count) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def page_count_for_relation(relation, row_count) @@ -69,7 +69,7 @@ module IssuableCollections finder_class.new(current_user, filter_params) end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def filter_params set_sort_order_from_cookie set_default_state @@ -94,7 +94,7 @@ module IssuableCollections @filter_params.permit(IssuableFinder::VALID_PARAMS) end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def set_default_state params[:state] = 'opened' if params[:state].blank? @@ -135,7 +135,7 @@ module IssuableCollections def finder strong_memoize(:finder) do - issuable_finder_for(@finder_type) # rubocop:disable Cop/ModuleWithInstanceVariables + issuable_finder_for(@finder_type) # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/app/controllers/concerns/issues_action.rb b/app/controllers/concerns/issues_action.rb index 4423c7fa0aa..d4cccbe6442 100644 --- a/app/controllers/concerns/issues_action.rb +++ b/app/controllers/concerns/issues_action.rb @@ -2,7 +2,7 @@ module IssuesAction extend ActiveSupport::Concern include IssuableCollections - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def issues @finder_type = IssuesFinder @label = finder.labels.first @@ -18,5 +18,5 @@ module IssuesAction format.atom { render layout: 'xml.atom' } end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/controllers/concerns/merge_requests_action.rb b/app/controllers/concerns/merge_requests_action.rb index de1710e7161..4d44df3bba9 100644 --- a/app/controllers/concerns/merge_requests_action.rb +++ b/app/controllers/concerns/merge_requests_action.rb @@ -2,7 +2,7 @@ module MergeRequestsAction extend ActiveSupport::Concern include IssuableCollections - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def merge_requests @finder_type = MergeRequestsFinder @label = finder.labels.first @@ -11,7 +11,7 @@ module MergeRequestsAction @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type) end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables private diff --git a/app/controllers/concerns/milestone_actions.rb b/app/controllers/concerns/milestone_actions.rb index 4996c6b90f3..7bec64c5d60 100644 --- a/app/controllers/concerns/milestone_actions.rb +++ b/app/controllers/concerns/milestone_actions.rb @@ -6,7 +6,7 @@ module MilestoneActions format.html { redirect_to milestone_redirect_path } format.json do render json: tabs_json("shared/milestones/_merge_requests_tab", { - merge_requests: @milestone.sorted_merge_requests, # rubocop:disable Cop/ModuleWithInstanceVariables + merge_requests: @milestone.sorted_merge_requests, # rubocop:disable Gitlab/ModuleWithInstanceVariables show_project_name: true }) end @@ -18,7 +18,7 @@ module MilestoneActions format.html { redirect_to milestone_redirect_path } format.json do render json: tabs_json("shared/milestones/_participants_tab", { - users: @milestone.participants # rubocop:disable Cop/ModuleWithInstanceVariables + users: @milestone.participants # rubocop:disable Gitlab/ModuleWithInstanceVariables }) end end @@ -29,7 +29,7 @@ module MilestoneActions format.html { redirect_to milestone_redirect_path } format.json do render json: tabs_json("shared/milestones/_labels_tab", { - labels: @milestone.labels # rubocop:disable Cop/ModuleWithInstanceVariables + labels: @milestone.labels # rubocop:disable Gitlab/ModuleWithInstanceVariables }) end @@ -44,7 +44,7 @@ module MilestoneActions } end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def milestone_redirect_path if @project project_milestone_path(@project, @milestone) @@ -54,5 +54,5 @@ module MilestoneActions dashboard_milestone_path(@milestone.safe_title, title: @milestone.title) end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index e6ef1f6f5e4..e82a5650935 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -31,7 +31,7 @@ module NotesActions render json: notes_json end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def create create_params = note_params.merge( merge_request_diff_head_sha: params[:merge_request_diff_head_sha], @@ -49,9 +49,9 @@ module NotesActions format.html { redirect_back_or_default } end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def update @note = Notes::UpdateService.new(project, current_user, note_params).execute(note) @@ -64,7 +64,7 @@ module NotesActions format.html { redirect_back_or_default } end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def destroy if note.editable? @@ -143,7 +143,7 @@ module NotesActions end else template = "discussions/_diff_discussion" - @fresh_discussion = true # rubocop:disable Cop/ModuleWithInstanceVariables + @fresh_discussion = true # rubocop:disable Gitlab/ModuleWithInstanceVariables locals = { discussions: [discussion], on_image: on_image } end @@ -196,7 +196,7 @@ module NotesActions end def noteable - @noteable ||= notes_finder.target || @note&.noteable # rubocop:disable Cop/ModuleWithInstanceVariables + @noteable ||= notes_finder.target || @note&.noteable # rubocop:disable Gitlab/ModuleWithInstanceVariables end def require_noteable! diff --git a/app/controllers/concerns/preview_markdown.rb b/app/controllers/concerns/preview_markdown.rb index 01c95612922..738de424dac 100644 --- a/app/controllers/concerns/preview_markdown.rb +++ b/app/controllers/concerns/preview_markdown.rb @@ -1,7 +1,7 @@ module PreviewMarkdown extend ActiveSupport::Concern - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def preview_markdown result = PreviewMarkdownService.new(@project, current_user, params).execute @@ -20,5 +20,5 @@ module PreviewMarkdown } } end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/controllers/concerns/renders_commits.rb b/app/controllers/concerns/renders_commits.rb index 7b8cee435ee..fb41dc1e8a8 100644 --- a/app/controllers/concerns/renders_commits.rb +++ b/app/controllers/concerns/renders_commits.rb @@ -1,6 +1,6 @@ module RendersCommits def prepare_commits_for_rendering(commits) - Banzai::CommitRenderer.render(commits, @project, current_user) # rubocop:disable Cop/ModuleWithInstanceVariables + Banzai::CommitRenderer.render(commits, @project, current_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables commits end diff --git a/app/controllers/concerns/renders_notes.rb b/app/controllers/concerns/renders_notes.rb index 4ca6d9db5cf..e7ef297879f 100644 --- a/app/controllers/concerns/renders_notes.rb +++ b/app/controllers/concerns/renders_notes.rb @@ -1,5 +1,5 @@ module RendersNotes - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def prepare_notes_for_rendering(notes, noteable = nil) preload_noteable_for_regular_notes(notes) preload_max_access_for_authors(notes, @project) @@ -8,7 +8,7 @@ module RendersNotes notes end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables private diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb index 47b10c3ef3b..3d61458c064 100644 --- a/app/controllers/concerns/service_params.rb +++ b/app/controllers/concerns/service_params.rb @@ -66,7 +66,7 @@ module ServiceParams FILTER_BLANK_PARAMS = [:password].freeze def service_params - dynamic_params = @service.event_channel_names + @service.event_names # rubocop:disable Cop/ModuleWithInstanceVariables + dynamic_params = @service.event_channel_names + @service.event_names # rubocop:disable Gitlab/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 046dae480c1..9095cc7f783 100644 --- a/app/controllers/concerns/snippets_actions.rb +++ b/app/controllers/concerns/snippets_actions.rb @@ -4,7 +4,7 @@ module SnippetsActions def edit end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def raw disposition = params[:inline] == 'false' ? 'attachment' : 'inline' @@ -15,7 +15,7 @@ module SnippetsActions filename: @snippet.sanitized_file_name ) end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables private diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb index fdea8bcb918..86f28f30032 100644 --- a/app/models/concerns/noteable.rb +++ b/app/models/concerns/noteable.rb @@ -46,7 +46,7 @@ module Noteable notes.inc_relations_for_view.grouped_diff_discussions(*args) end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def resolvable_discussions @resolvable_discussions ||= if defined?(@discussions) @@ -55,7 +55,7 @@ module Noteable discussion_notes.resolvable.discussions(self) end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/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 2ecce745ab9..835f26aa57b 100644 --- a/app/models/concerns/relative_positioning.rb +++ b/app/models/concerns/relative_positioning.rb @@ -52,7 +52,7 @@ 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] # rubocop:disable Cop/ModuleWithInstanceVariables + @positionable_neighbours = [before] # rubocop:disable Gitlab/ModuleWithInstanceVariables end self.relative_position = position_between(before.relative_position, after.relative_position) @@ -65,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] # rubocop:disable Cop/ModuleWithInstanceVariables + @positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables pos_after = issue_to_move.relative_position end @@ -80,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] # rubocop:disable Cop/ModuleWithInstanceVariables + @positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables pos_before = issue_to_move.relative_position end @@ -132,7 +132,7 @@ module RelativePositioning end end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def save_positionable_neighbours return unless @positionable_neighbours @@ -141,5 +141,5 @@ module RelativePositioning status end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/models/concerns/resolvable_discussion.rb b/app/models/concerns/resolvable_discussion.rb index fc54a8bbca0..b6c7b6735b9 100644 --- a/app/models/concerns/resolvable_discussion.rb +++ b/app/models/concerns/resolvable_discussion.rb @@ -45,13 +45,13 @@ module ResolvableDiscussion def first_note_to_resolve return unless resolvable? - @first_note_to_resolve ||= notes.find(&:to_be_resolved?) # rubocop:disable Cop/ModuleWithInstanceVariables + @first_note_to_resolve ||= notes.find(&:to_be_resolved?) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def last_resolved_note return unless resolved? - @last_resolved_note ||= resolved_notes.sort_by(&:resolved_at).last # rubocop:disable Cop/ModuleWithInstanceVariables + @last_resolved_note ||= resolved_notes.sort_by(&:resolved_at).last # rubocop:disable Gitlab/ModuleWithInstanceVariables end def resolved_notes @@ -91,7 +91,7 @@ module ResolvableDiscussion yield(notes_relation) # Set the notes array to the updated notes - @notes = notes_relation.fresh.to_a # rubocop:disable Cop/ModuleWithInstanceVariables + @notes = notes_relation.fresh.to_a # rubocop:disable Gitlab/ModuleWithInstanceVariables self.class.memoized_values.each do |var| instance_variable_set(:"@#{var}", nil) diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index efec55d7376..5c1cce98ad4 100644 --- a/app/models/concerns/routable.rb +++ b/app/models/concerns/routable.rb @@ -88,7 +88,7 @@ module Routable def full_name if route && route.name.present? - @full_name ||= route.name # rubocop:disable Cop/ModuleWithInstanceVariables + @full_name ||= route.name # rubocop:disable Gitlab/ModuleWithInstanceVariables else update_route if persisted? @@ -112,7 +112,7 @@ module Routable def expires_full_path_cache RequestStore.delete(full_path_key) if RequestStore.active? - @full_path = nil # rubocop:disable Cop/ModuleWithInstanceVariables + @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables end def build_full_path @@ -127,7 +127,7 @@ module Routable def uncached_full_path if route && route.path.present? - @full_path ||= route.path # rubocop:disable Cop/ModuleWithInstanceVariables + @full_path ||= route.path # rubocop:disable Gitlab/ModuleWithInstanceVariables else update_route if persisted? @@ -166,7 +166,7 @@ module Routable route || build_route(source: self) route.path = build_full_path route.name = build_full_name - @full_path = nil # rubocop:disable Cop/ModuleWithInstanceVariables - @full_name = nil # rubocop:disable Cop/ModuleWithInstanceVariables + @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables + @full_name = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb index 378a3ede2aa..d07041c2fdf 100644 --- a/app/models/concerns/taskable.rb +++ b/app/models/concerns/taskable.rb @@ -39,7 +39,7 @@ module Taskable def task_list_items return [] if description.blank? - @task_list_items ||= Taskable.get_tasks(description) # rubocop:disable Cop/ModuleWithInstanceVariables + @task_list_items ||= Taskable.get_tasks(description) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def tasks diff --git a/app/models/concerns/time_trackable.rb b/app/models/concerns/time_trackable.rb index f1b43cb38e1..fc44a7da178 100644 --- a/app/models/concerns/time_trackable.rb +++ b/app/models/concerns/time_trackable.rb @@ -20,7 +20,7 @@ module TimeTrackable has_many :timelogs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def spend_time(options) @time_spent = options[:duration] @time_spent_user = options[:user] @@ -36,7 +36,7 @@ module TimeTrackable end end alias_method :spend_time=, :spend_time - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def total_time_spent timelogs.sum(:time_spent) @@ -53,10 +53,10 @@ module TimeTrackable private def reset_spent_time - timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) # rubocop:disable Cop/ModuleWithInstanceVariables + timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def add_or_subtract_spent_time timelogs.new( time_spent: time_spent, @@ -64,7 +64,7 @@ module TimeTrackable spent_at: @spent_at ) end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def check_negative_time_spent return if time_spent.nil? || time_spent == :reset diff --git a/app/services/concerns/issues/resolve_discussions.rb b/app/services/concerns/issues/resolve_discussions.rb index fc312aad8bd..26eb274f4d5 100644 --- a/app/services/concerns/issues/resolve_discussions.rb +++ b/app/services/concerns/issues/resolve_discussions.rb @@ -4,12 +4,12 @@ module Issues attr_reader :merge_request_to_resolve_discussions_of_iid, :discussion_to_resolve_id - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def filter_resolve_discussion_params @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:enable Gitlab/ModuleWithInstanceVariables def merge_request_to_resolve_discussions_of strong_memoize(:merge_request_to_resolve_discussions_of) do @@ -22,7 +22,7 @@ module Issues def discussions_to_resolve return [] unless merge_request_to_resolve_discussions_of - @discussions_to_resolve ||= # rubocop:disable Cop/ModuleWithInstanceVariables + @discussions_to_resolve ||= # rubocop:disable Gitlab/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 31ec1e9713e..d4ade869777 100644 --- a/app/services/spam_check_service.rb +++ b/app/services/spam_check_service.rb @@ -7,19 +7,19 @@ # - params with :request # module SpamCheckService - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def filter_spam_check_params @request = params.delete(:request) @api = params.delete(:api) @recaptcha_verified = params.delete(:recaptcha_verified) @spam_log_id = params.delete(:spam_log_id) end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/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 # attribute values. - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def spam_check(spammable, user) spam_service = SpamService.new(spammable, @request) @@ -27,5 +27,5 @@ module SpamCheckService user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true) end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/workers/concerns/new_issuable.rb b/app/workers/concerns/new_issuable.rb index 9a15e79d0c3..526ed0bad07 100644 --- a/app/workers/concerns/new_issuable.rb +++ b/app/workers/concerns/new_issuable.rb @@ -9,15 +9,15 @@ module NewIssuable end def set_user(user_id) - @user = User.find_by(id: user_id) # rubocop:disable Cop/ModuleWithInstanceVariables + @user = User.find_by(id: user_id) # rubocop:disable Gitlab/ModuleWithInstanceVariables - log_error(User, user_id) unless @user # rubocop:disable Cop/ModuleWithInstanceVariables + log_error(User, user_id) unless @user # rubocop:disable Gitlab/ModuleWithInstanceVariables end def set_issuable(issuable_id) - @issuable = issuable_class.find_by(id: issuable_id) # rubocop:disable Cop/ModuleWithInstanceVariables + @issuable = issuable_class.find_by(id: issuable_id) # rubocop:disable Gitlab/ModuleWithInstanceVariables - log_error(issuable_class, issuable_id) unless @issuable # rubocop:disable Cop/ModuleWithInstanceVariables + log_error(issuable_class, issuable_id) unless @issuable # rubocop:disable Gitlab/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 1f043408b4e..2644ee6a7d3 100644 --- a/config/initializers/fix_local_cache_middleware.rb +++ b/config/initializers/fix_local_cache_middleware.rb @@ -6,7 +6,7 @@ module LocalCacheRegistryCleanupWithEnsure def call(env) LocalCacheRegistry.set_cache_for(local_cache_key, LocalStore.new) - response = @app.call(env) # rubocop:disable Cop/ModuleWithInstanceVariables + response = @app.call(env) # rubocop:disable Gitlab/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 c732e303f03..2de310753a9 100644 --- a/config/initializers/rspec_profiling.rb +++ b/config/initializers/rspec_profiling.rb @@ -19,10 +19,10 @@ module RspecProfilingExt def example_finished(*args) super rescue => err - return if @already_logged_example_finished_error # rubocop:disable Cop/ModuleWithInstanceVariables + return if @already_logged_example_finished_error # rubocop:disable Gitlab/ModuleWithInstanceVariables $stderr.puts "rspec_profiling couldn't collect an example: #{err}. Further warnings suppressed." - @already_logged_example_finished_error = true # rubocop:disable Cop/ModuleWithInstanceVariables + @already_logged_example_finished_error = true # rubocop:disable Gitlab/ModuleWithInstanceVariables end alias_method :example_passed, :example_finished diff --git a/doc/development/module_with_instance_variables.md b/doc/development/module_with_instance_variables.md index 1c64b35b782..46e7177e704 100644 --- a/doc/development/module_with_instance_variables.md +++ b/doc/development/module_with_instance_variables.md @@ -185,7 +185,7 @@ Put the disabling comment right after your code in the same line: ``` ruby module M def violating_method - @f + @g # rubocop:disable Cop/ModuleWithInstanceVariables + @f + @g # rubocop:disable Gitlab/ModuleWithInstanceVariables end end ``` @@ -194,13 +194,13 @@ If there are multiple lines, you could also enable and disable for a section: ``` ruby module M - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def violating_method @f = 0 @g = 1 @h = 2 end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables end ``` diff --git a/features/support/env.rb b/features/support/env.rb index a1413522f3a..b8df707a8ee 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -42,11 +42,11 @@ module StdoutReporterWithScenarioLocation # Override the standard reporter to show filename and line number next to each # scenario for easy, focused re-runs def before_scenario_run(scenario, step_definitions = nil) - @max_step_name_length = scenario.steps.map(&:name).map(&:length).max if scenario.steps.any? # rubocop:disable Cop/ModuleWithInstanceVariables + @max_step_name_length = scenario.steps.map(&:name).map(&:length).max if scenario.steps.any? # rubocop:disable Gitlab/ModuleWithInstanceVariables name = scenario.name # This number has no significance, it's just to line things up - max_length = @max_step_name_length + 19 # rubocop:disable Cop/ModuleWithInstanceVariables + max_length = @max_step_name_length + 19 # rubocop:disable Gitlab/ModuleWithInstanceVariables out.puts "\n #{'Scenario:'.green} #{name.light_green.ljust(max_length)}" \ " # #{scenario.feature.filename}:#{scenario.line}" end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index c4f81443282..fc1a3948bb4 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -32,7 +32,7 @@ module API end end - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/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`. @@ -50,7 +50,7 @@ module API @current_user end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def sudo? initial_current_user != current_user @@ -399,7 +399,7 @@ module API private - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def initial_current_user return @initial_current_user if defined?(@initial_current_user) @@ -409,7 +409,7 @@ module API unauthorized! end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def sudo! return unless sudo_identifier @@ -429,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 # rubocop:disable Cop/ModuleWithInstanceVariables + @current_user = sudoed_user # rubocop:disable Gitlab/ModuleWithInstanceVariables end def sudo_identifier diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index e2077d33b9f..520bf65c3b3 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -9,13 +9,13 @@ module API attr_reader :redirected_path def wiki? - set_project unless defined?(@wiki) # rubocop:disable Cop/ModuleWithInstanceVariables - @wiki # rubocop:disable Cop/ModuleWithInstanceVariables + set_project unless defined?(@wiki) # rubocop:disable Gitlab/ModuleWithInstanceVariables + @wiki # rubocop:disable Gitlab/ModuleWithInstanceVariables end def project - set_project unless defined?(@project) # rubocop:disable Cop/ModuleWithInstanceVariables - @project # rubocop:disable Cop/ModuleWithInstanceVariables + set_project unless defined?(@project) # rubocop:disable Gitlab/ModuleWithInstanceVariables + @project # rubocop:disable Gitlab/ModuleWithInstanceVariables end def ssh_authentication_abilities @@ -67,7 +67,7 @@ module API private - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def set_project if params[:gl_repository] @project, @wiki = Gitlab::GlRepository.parse(params[:gl_repository]) @@ -76,7 +76,7 @@ module API @project, @wiki, @redirected_path = Gitlab::RepoPath.parse(params[:project]) end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/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 40a65aad631..26f699f4c9d 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -40,7 +40,7 @@ module ExtractsPath def extract_ref(id) pair = ['', ''] - return pair unless @project # rubocop:disable Cop/ModuleWithInstanceVariables + return pair unless @project # rubocop:disable Gitlab/ModuleWithInstanceVariables if id =~ /^(\h{40})(.+)/ # If the ref appears to be a SHA, we're done, just split the string @@ -104,7 +104,7 @@ module ExtractsPath # # Automatically renders `not_found!` if a valid tree path could not be # resolved (e.g., when a user inserts an invalid path or ref). - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def assign_ref_vars # assign allowed options allowed_options = ["filter_ref"] @@ -132,10 +132,10 @@ module ExtractsPath rescue RuntimeError, NoMethodError, InvalidPathError render_404 end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def tree - @tree ||= @repo.tree(@commit.id, @path) # rubocop:disable Cop/ModuleWithInstanceVariables + @tree ||= @repo.tree(@commit.id, @path) # rubocop:disable Gitlab/ModuleWithInstanceVariables end private @@ -148,8 +148,8 @@ module ExtractsPath end def ref_names - return [] unless @project # rubocop:disable Cop/ModuleWithInstanceVariables + return [] unless @project # rubocop:disable Gitlab/ModuleWithInstanceVariables - @ref_names ||= @project.repository.ref_names # rubocop:disable Cop/ModuleWithInstanceVariables + @ref_names ||= @project.repository.ref_names # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/lib/gitlab/ci/charts.rb b/lib/gitlab/ci/charts.rb index e94166aee0c..525563a97f5 100644 --- a/lib/gitlab/ci/charts.rb +++ b/lib/gitlab/ci/charts.rb @@ -6,7 +6,7 @@ module Gitlab query .group("DATE(#{::Ci::Pipeline.table_name}.created_at)") .count(:created_at) - .transform_keys { |date| date.strftime(@format) } # rubocop:disable Cop/ModuleWithInstanceVariables + .transform_keys { |date| date.strftime(@format) } # rubocop:disable Gitlab/ModuleWithInstanceVariables end def interval_step diff --git a/lib/gitlab/ci/config/entry/validatable.rb b/lib/gitlab/ci/config/entry/validatable.rb index 0dc359a86c3..e45787773a8 100644 --- a/lib/gitlab/ci/config/entry/validatable.rb +++ b/lib/gitlab/ci/config/entry/validatable.rb @@ -13,7 +13,7 @@ module Gitlab end def errors - @validator.messages + descendants.flat_map(&:errors) # rubocop:disable Cop/ModuleWithInstanceVariables + @validator.messages + descendants.flat_map(&:errors) # rubocop:disable Gitlab/ModuleWithInstanceVariables end class_methods do diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index dfb2cfe42b7..91fd9cc7631 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -53,7 +53,7 @@ module Gitlab end def in_memory_application_settings - @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) # rubocop:disable Cop/ModuleWithInstanceVariables + @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) # rubocop:disable Gitlab/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 05b4928c3b9..dcbdf9a64b0 100644 --- a/lib/gitlab/cycle_analytics/base_query.rb +++ b/lib/gitlab/cycle_analytics/base_query.rb @@ -14,9 +14,9 @@ module Gitlab 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)) # rubocop:disable Cop/ModuleWithInstanceVariables + .where(issue_table[:project_id].eq(@project.id)) # rubocop:disable Gitlab/ModuleWithInstanceVariables .where(issue_table[:deleted_at].eq(nil)) - .where(issue_table[:created_at].gteq(@options[:from])) # rubocop:disable Cop/ModuleWithInstanceVariables + .where(issue_table[:created_at].gteq(@options[:from])) # rubocop:disable Gitlab/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 ebbff1b2f33..7a889b3877f 100644 --- a/lib/gitlab/cycle_analytics/production_helper.rb +++ b/lib/gitlab/cycle_analytics/production_helper.rb @@ -4,7 +4,7 @@ module Gitlab def stage_query super .where(mr_metrics_table[:first_deployed_to_production_at] - .gteq(@options[:from])) # rubocop:disable Cop/ModuleWithInstanceVariables + .gteq(@options[:from])) # rubocop:disable Gitlab/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 9ae1f66a182..403afbe3b9a 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 @@ -6,7 +6,7 @@ module Gitlab module Routable def full_path if route && route.path.present? - @full_path ||= route.path # rubocop:disable Cop/ModuleWithInstanceVariables + @full_path ||= route.path # rubocop:disable Gitlab/ModuleWithInstanceVariables else update_route if persisted? @@ -30,7 +30,7 @@ module Gitlab def prepare_route route || build_route(source: self) route.path = build_full_path - @full_path = nil # rubocop:disable Cop/ModuleWithInstanceVariables + @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/lib/gitlab/import_export/command_line_util.rb b/lib/gitlab/import_export/command_line_util.rb index 19dab0037bb..0135b3c6f22 100644 --- a/lib/gitlab/import_export/command_line_util.rb +++ b/lib/gitlab/import_export/command_line_util.rb @@ -32,7 +32,7 @@ module Gitlab def execute(cmd) output, status = Gitlab::Popen.popen(cmd) - @shared.error(Gitlab::ImportExport::Error.new(output.to_s)) unless status.zero? # rubocop:disable Cop/ModuleWithInstanceVariables + @shared.error(Gitlab::ImportExport::Error.new(output.to_s)) unless status.zero? # rubocop:disable Gitlab/ModuleWithInstanceVariables status.zero? end diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb index fa88d41be73..6ea132fc5bf 100644 --- a/lib/gitlab/metrics/influx_db.rb +++ b/lib/gitlab/metrics/influx_db.rb @@ -154,7 +154,7 @@ module Gitlab # When enabled this should be set before being used as the usual pattern # "@foo ||= bar" is _not_ thread-safe. - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def pool if influx_metrics_enabled? if @pool.nil? @@ -171,7 +171,7 @@ module Gitlab @pool end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables end end end diff --git a/rubocop/cop/gitlab/module_with_instance_variables.rb b/rubocop/cop/gitlab/module_with_instance_variables.rb new file mode 100644 index 00000000000..5c9cde98512 --- /dev/null +++ b/rubocop/cop/gitlab/module_with_instance_variables.rb @@ -0,0 +1,63 @@ +module RuboCop + module Cop + module Gitlab + class ModuleWithInstanceVariables < RuboCop::Cop::Cop + MSG = <<~EOL.freeze + Do not use instance variables in a module. Please read this + for the rationale behind it: + + https://docs.gitlab.com/ee/development/module_with_instance_variables.html + EOL + + def on_module(node) + check_method_definition(node) + + # Not sure why some module would have an extra begin wrapping around + node.each_child_node(:begin) do |begin_node| + check_method_definition(begin_node) + end + end + + private + + def check_method_definition(node) + node.each_child_node(:def) do |definition| + # We allow this pattern: + # + # def f + # @f ||= true + # end + if only_ivar_or_assignment?(definition) + # We don't allow if any other ivar is used + definition.each_descendant(:ivar) do |offense| + add_offense(offense, :expression) + end + # We allow initialize method and single ivar + elsif !initialize_method?(definition) && !single_ivar?(definition) + definition.each_descendant(:ivar, :ivasgn) do |offense| + add_offense(offense, :expression) + end + end + end + end + + def only_ivar_or_assignment?(definition) + node = definition.child_nodes.last + + definition.child_nodes.size == 2 && + node.or_asgn_type? && node.child_nodes.first.ivasgn_type? + end + + def single_ivar?(definition) + node = definition.child_nodes.last + + definition.child_nodes.size == 2 && node.ivar_type? + end + + def initialize_method?(definition) + definition.children.first == :initialize + end + end + end + end +end diff --git a/rubocop/cop/module_with_instance_variables.rb b/rubocop/cop/module_with_instance_variables.rb deleted file mode 100644 index f101ae09ad2..00000000000 --- a/rubocop/cop/module_with_instance_variables.rb +++ /dev/null @@ -1,82 +0,0 @@ -module RuboCop - module Cop - class ModuleWithInstanceVariables < RuboCop::Cop::Cop - MSG = <<~EOL.freeze - Do not use instance variables in a module. Please read this - for the rationale behind it: - - https://docs.gitlab.com/ee/development/module_with_instance_variables.html - EOL - - def on_module(node) - return if - rails_helper?(node) || rails_mailer?(node) || spec_helper?(node) - - check_method_definition(node) - - # Not sure why some module would have an extra begin wrapping around - node.each_child_node(:begin) do |begin_node| - check_method_definition(begin_node) - end - end - - private - - # We ignore Rails helpers right now because it's hard to workaround it - def rails_helper?(node) - node.source_range.source_buffer.name =~ - %r{app/helpers/\w+_helper.rb\z} - end - - # We ignore Rails mailers right now because it's hard to workaround it - def rails_mailer?(node) - node.source_range.source_buffer.name =~ - %r{app/mailers/emails/} - end - - # We ignore spec helpers because it usually doesn't matter - def spec_helper?(node) - node.source_range.source_buffer.name =~ - %r{spec/support/|features/steps/} - end - - def check_method_definition(node) - node.each_child_node(:def) do |definition| - # We allow this pattern: - # - # def f - # @f ||= true - # end - if only_ivar_or_assignment?(definition) - # We don't allow if any other ivar is used - definition.each_descendant(:ivar) do |offense| - add_offense(offense, :expression) - end - # We allow initialize method and single ivar - elsif !initialize_method?(definition) && !single_ivar?(definition) - definition.each_descendant(:ivar, :ivasgn) do |offense| - add_offense(offense, :expression) - end - end - end - end - - def only_ivar_or_assignment?(definition) - node = definition.child_nodes.last - - definition.child_nodes.size == 2 && - node.or_asgn_type? && node.child_nodes.first.ivasgn_type? - end - - def single_ivar?(definition) - node = definition.child_nodes.last - - definition.child_nodes.size == 2 && node.ivar_type? - end - - def initialize_method?(definition) - definition.children.first == :initialize - end - end - end -end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 65a37f578cd..c4bab18faee 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -5,8 +5,8 @@ require_relative 'cop/gem_fetcher' require_relative 'cop/in_batches' require_relative 'cop/polymorphic_associations' require_relative 'cop/project_path_helper' -require_relative 'cop/module_with_instance_variables' require_relative 'cop/redirect_with_status' +require_relative 'cop/gitlab/module_with_instance_variables' require_relative 'cop/migration/add_column' require_relative 'cop/migration/add_column_with_default_to_large_table' require_relative 'cop/migration/add_concurrent_foreign_key' diff --git a/spec/rubocop/cop/module_with_instance_variables_spec.rb b/spec/rubocop/cop/gitlab/module_with_instance_variables_spec.rb index df5e2dd2f04..1fd40653f79 100644 --- a/spec/rubocop/cop/module_with_instance_variables_spec.rb +++ b/spec/rubocop/cop/gitlab/module_with_instance_variables_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' require 'rubocop' require 'rubocop/rspec/support' -require_relative '../../../rubocop/cop/module_with_instance_variables' +require_relative '../../../../rubocop/cop/gitlab/module_with_instance_variables' -describe RuboCop::Cop::ModuleWithInstanceVariables do +describe RuboCop::Cop::Gitlab::ModuleWithInstanceVariables do include CopHelper subject(:cop) { described_class.new } @@ -83,42 +83,6 @@ describe RuboCop::Cop::ModuleWithInstanceVariables do end end - context 'with regular ivar assignment' do - let(:source) do - <<~RUBY - module M - def f - @f = true - end - end - RUBY - end - - context 'when source is offending but it is a rails helper' do - before do - allow(cop).to receive(:rails_helper?).and_return(true) - end - - it_behaves_like 'not registering offense' - end - - context 'when source is offending but it is a rails mailer' do - before do - allow(cop).to receive(:rails_mailer?).and_return(true) - end - - it_behaves_like 'not registering offense' - end - - context 'when source is offending but it is a spec helper' do - before do - allow(cop).to receive(:spec_helper?).and_return(true) - end - - it_behaves_like 'not registering offense' - end - end - context 'when source is using simple or ivar assignment' do it_behaves_like 'not registering offense' do let(:source) do |