Resolve "Make a Rubocop that forbids returning from a block"

parent 3529ccae
......@@ -217,7 +217,7 @@ module NotesActions
def note_project
strong_memoize(:note_project) do
return nil unless project
next nil unless project
note_project_id = params[:note_project_id]
......@@ -228,7 +228,7 @@ module NotesActions
project
end
return access_denied! unless can?(current_user, :create_note, the_project)
next access_denied! unless can?(current_user, :create_note, the_project)
the_project
end
......
......@@ -15,7 +15,7 @@ module Groups
def update
if @group.update(group_variables_params)
respond_to do |format|
format.json { return render_group_variables }
format.json { render_group_variables }
end
else
respond_to do |format|
......
......@@ -60,13 +60,13 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end
format.patch do
return render_404 unless @merge_request.diff_refs
break render_404 unless @merge_request.diff_refs
send_git_patch @project.repository, @merge_request.diff_refs
end
format.diff do
return render_404 unless @merge_request.diff_refs
break render_404 unless @merge_request.diff_refs
send_git_diff @project.repository, @merge_request.diff_refs
end
......
......@@ -12,7 +12,7 @@ class Projects::VariablesController < Projects::ApplicationController
def update
if @project.update(variables_params)
respond_to do |format|
format.json { return render_variables }
format.json { render_variables }
end
else
respond_to do |format|
......
......@@ -479,7 +479,7 @@ module Ci
def user_variables
Gitlab::Ci::Variables::Collection.new.tap do |variables|
return variables if user.blank?
break variables if user.blank?
variables.append(key: 'GITLAB_USER_ID', value: user.id.to_s)
variables.append(key: 'GITLAB_USER_EMAIL', value: user.email)
......@@ -594,7 +594,7 @@ module Ci
def persisted_variables
Gitlab::Ci::Variables::Collection.new.tap do |variables|
return variables unless persisted?
break variables unless persisted?
variables
.append(key: 'CI_JOB_ID', value: id.to_s)
......@@ -643,7 +643,7 @@ module Ci
def persisted_environment_variables
Gitlab::Ci::Variables::Collection.new.tap do |variables|
return variables unless persisted? && persisted_environment.present?
break variables unless persisted? && persisted_environment.present?
variables.concat(persisted_environment.predefined_variables)
......
......@@ -83,14 +83,14 @@ class NotificationRecipient
def has_access?
DeclarativePolicy.subject_scope do
return false unless user.can?(:receive_notifications)
return true if @skip_read_ability
break false unless user.can?(:receive_notifications)
break true if @skip_read_ability
return false if @target && !user.can?(:read_cross_project)
return false if @project && !user.can?(:read_project, @project)
break false if @target && !user.can?(:read_cross_project)
break false if @project && !user.can?(:read_project, @project)
return true unless read_ability
return true unless DeclarativePolicy.has_policy?(@target)
break true unless read_ability
break true unless DeclarativePolicy.has_policy?(@target)
user.can?(read_ability, @target)
end
......
......@@ -1637,7 +1637,7 @@ class Project < ActiveRecord::Base
def container_registry_variables
Gitlab::Ci::Variables::Collection.new.tap do |variables|
return variables unless Gitlab.config.registry.enabled
break variables unless Gitlab.config.registry.enabled
variables.append(key: 'CI_REGISTRY', value: Gitlab.config.registry.host_port)
......
......@@ -44,7 +44,7 @@ module Ci
build.run!
register_success(build)
return Result.new(build, true)
return Result.new(build, true) # rubocop:disable Cop/AvoidReturnFromBlocks
rescue Ci::Build::MissingDependenciesError
build.drop!(:missing_dependency_failure)
end
......
......@@ -17,7 +17,7 @@ module Clusters
when 'DONE'
finalize_creation
else
return provider.make_errored!("Unexpected operation status; #{operation.status} #{operation.status_message}")
provider.make_errored!("Unexpected operation status; #{operation.status} #{operation.status_message}")
end
end
end
......
......@@ -19,8 +19,8 @@ class CreateDeploymentService
environment.fire_state_event(action)
return unless environment.save
return if environment.stopped?
break unless environment.save
break if environment.stopped?
deploy.tap(&:update_merge_request_metrics!)
end
......
......@@ -10,7 +10,7 @@ class ImportExportCleanUpService
def execute
Gitlab::Metrics.measure(:import_export_clean_up) do
return unless File.directory?(path)
next unless File.directory?(path)
clean_up_export_files
end
......
......@@ -137,7 +137,7 @@ module Projects
return true unless Gitlab.config.registry.enabled
ContainerRepository.build_root_repository(project).tap do |repository|
return repository.has_tags? ? repository.delete_tags! : true
break repository.has_tags? ? repository.delete_tags! : true
end
end
......
......@@ -10,7 +10,7 @@ class RepositoryArchiveCleanUpService
def execute
Gitlab::Metrics.measure(:repository_archive_clean_up) do
return unless File.directory?(path)
next unless File.directory?(path)
clean_up_old_archives
clean_up_empty_directories
......
......@@ -19,7 +19,7 @@ module TestHooks
error_message = catch(:validation_error) do
sample_data = self.__send__(trigger_data_method) # rubocop:disable GitlabSecurity/PublicSend
return hook.execute(sample_data, trigger_key)
return hook.execute(sample_data, trigger_key) # rubocop:disable Cop/AvoidReturnFromBlocks
end
error(error_message)
......
......@@ -33,7 +33,7 @@ class PostReceive
unless @user
log("Triggered hook for non-existing user \"#{post_received.identifier}\"")
return false
return false # rubocop:disable Cop/AvoidReturnFromBlocks
end
if Gitlab::Git.tag_ref?(ref)
......
......@@ -38,7 +38,7 @@ class StuckCiJobsWorker
def drop_stuck(status, timeout)
search(status, timeout) do |build|
return unless build.stuck?
break unless build.stuck?
drop_build :stuck, build, status, timeout
end
......
---
title: Rubocop rule to avoid returning from a block
merge_request: 18000
author: Jacopo Beschi @jacopo-beschi
type: added
......@@ -25,7 +25,7 @@ module API
get ":id/#{noteables_str}/:noteable_id/discussions" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
return not_found!("Discussions") unless can?(current_user, noteable_read_ability_name(noteable), noteable)
break not_found!("Discussions") unless can?(current_user, noteable_read_ability_name(noteable), noteable)
notes = noteable.notes
.inc_relations_for_view
......@@ -50,7 +50,7 @@ module API
notes = readable_discussion_notes(noteable, params[:discussion_id])
if notes.empty? || !can?(current_user, noteable_read_ability_name(noteable), noteable)
return not_found!("Discussion")
break not_found!("Discussion")
end
discussion = Discussion.build(notes, noteable)
......@@ -98,7 +98,7 @@ module API
notes = readable_discussion_notes(noteable, params[:discussion_id])
if notes.empty? || !can?(current_user, noteable_read_ability_name(noteable), noteable)
return not_found!("Notes")
break not_found!("Notes")
end
present notes, with: Entities::Note
......@@ -117,8 +117,8 @@ module API
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
notes = readable_discussion_notes(noteable, params[:discussion_id])
return not_found!("Discussion") if notes.empty?
return bad_request!("Discussion is an individual note.") unless notes.first.part_of_discussion?
break not_found!("Discussion") if notes.empty?
break bad_request!("Discussion is an individual note.") unless notes.first.part_of_discussion?
opts = {
note: params[:body],
......
......@@ -31,7 +31,7 @@ module API
key = params[:key]
variable = user_group.variables.find_by(key: key)
return not_found!('GroupVariable') unless variable
break not_found!('GroupVariable') unless variable
present variable, with: Entities::Variable
end
......@@ -67,7 +67,7 @@ module API
put ':id/variables/:key' do
variable = user_group.variables.find_by(key: params[:key])
return not_found!('GroupVariable') unless variable
break not_found!('GroupVariable') unless variable
variable_params = declared_params(include_missing: false).except(:key)
......
......@@ -50,7 +50,7 @@ module API
access_checker.check(params[:action], params[:changes])
@project ||= access_checker.project
rescue Gitlab::GitAccess::UnauthorizedError, Gitlab::GitAccess::NotFoundError => e
return { status: false, message: e.message }
break { status: false, message: e.message }
end
log_user_activity(actor)
......@@ -142,21 +142,21 @@ module API
if key
key.update_last_used_at
else
return { 'success' => false, 'message' => 'Could not find the given key' }
break { 'success' => false, 'message' => 'Could not find the given key' }
end
if key.is_a?(DeployKey)
return { success: false, message: 'Deploy keys cannot be used to retrieve recovery codes' }
break { success: false, message: 'Deploy keys cannot be used to retrieve recovery codes' }
end
user = key.user
unless user
return { success: false, message: 'Could not find a user for the given key' }
break { success: false, message: 'Could not find a user for the given key' }
end
unless user.two_factor_enabled?
return { success: false, message: 'Two-factor authentication is not enabled for this user' }
break { success: false, message: 'Two-factor authentication is not enabled for this user' }
end
codes = nil
......
......@@ -310,7 +310,7 @@ module API
issue = find_project_issue(params[:issue_iid])
return not_found!('UserAgentDetail') unless issue.user_agent_detail
break not_found!('UserAgentDetail') unless issue.user_agent_detail
present issue.user_agent_detail, with: Entities::UserAgentDetail
end
......
......@@ -77,7 +77,7 @@ module API
build = find_build!(params[:job_id])
authorize!(:update_build, build)
return not_found!(build) unless build.artifacts?
break not_found!(build) unless build.artifacts?
build.keep_artifacts!
......
......@@ -120,7 +120,7 @@ module API
build = find_build!(params[:job_id])
authorize!(:update_build, build)
return forbidden!('Job is not retryable') unless build.retryable?
break forbidden!('Job is not retryable') unless build.retryable?
build = Ci::Build.retry(build, current_user)
......@@ -138,7 +138,7 @@ module API
build = find_build!(params[:job_id])
authorize!(:erase_build, build)
return forbidden!('Job is not erasable!') unless build.erasable?
break forbidden!('Job is not erasable!') unless build.erasable?
build.erase(erased_by: current_user)
present build, with: Entities::Job
......
......@@ -145,7 +145,7 @@ module API
snippet = Snippet.find_by!(id: params[:snippet_id], project_id: params[:id])
return not_found!('UserAgentDetail') unless snippet.user_agent_detail
break not_found!('UserAgentDetail') unless snippet.user_agent_detail
present snippet.user_agent_detail, with: Entities::UserAgentDetail
end
......
......@@ -402,7 +402,7 @@ module API
end
unless user_project.allowed_to_share_with_group?
return render_api_error!("The project sharing with group is disabled", 400)
break render_api_error!("The project sharing with group is disabled", 400)
end
link = user_project.project_group_links.new(declared_params(include_missing: false))
......
......@@ -29,7 +29,7 @@ module API
project.runners.create(attributes)
end
return forbidden! unless runner
break forbidden! unless runner
if runner.id
present runner, with: Entities::RunnerRegistrationDetails
......@@ -83,7 +83,7 @@ module API
if current_runner.runner_queue_value_latest?(params[:last_update])
header 'X-GitLab-Last-Update', params[:last_update]
Gitlab::Metrics.add_event(:build_not_found_cached)
return no_content!
break no_content!
end
new_update = current_runner.ensure_runner_queue_value
......@@ -152,7 +152,7 @@ module API
stream_size = job.trace.append(request.body.read, content_range[0].to_i)
if stream_size < 0
return error!('416 Range Not Satisfiable', 416, { 'Range' => "0-#{-stream_size}" })
break error!('416 Range Not Satisfiable', 416, { 'Range' => "0-#{-stream_size}" })
end
status 202
......
......@@ -94,7 +94,7 @@ module API
end
put ':id' do
snippet = snippets_for_current_user.find_by(id: params.delete(:id))
return not_found!('Snippet') unless snippet
break not_found!('Snippet') unless snippet
authorize! :update_personal_snippet, snippet
......@@ -120,7 +120,7 @@ module API
end
delete ':id' do
snippet = snippets_for_current_user.find_by(id: params.delete(:id))
return not_found!('Snippet') unless snippet
break not_found!('Snippet') unless snippet
authorize! :destroy_personal_snippet, snippet
......@@ -135,7 +135,7 @@ module API
end
get ":id/raw" do
snippet = snippets_for_current_user.find_by(id: params.delete(:id))
return not_found!('Snippet') unless snippet
break not_found!('Snippet') unless snippet
env['api.format'] = :txt
content_type 'text/plain'
......@@ -153,7 +153,7 @@ module API
snippet = Snippet.find_by!(id: params[:id])
return not_found!('UserAgentDetail') unless snippet.user_agent_detail
break not_found!('UserAgentDetail') unless snippet.user_agent_detail
present snippet.user_agent_detail, with: Entities::UserAgentDetail
end
......
......@@ -62,7 +62,7 @@ module API
authorize! :admin_build, user_project
trigger = user_project.triggers.find(params.delete(:trigger_id))
return not_found!('Trigger') unless trigger
break not_found!('Trigger') unless trigger
present trigger, with: Entities::Trigger
end
......@@ -99,7 +99,7 @@ module API
authorize! :admin_build, user_project
trigger = user_project.triggers.find(params.delete(:trigger_id))
return not_found!('Trigger') unless trigger
break not_found!('Trigger') unless trigger
if trigger.update(declared_params(include_missing: false))
present trigger, with: Entities::Trigger
......@@ -119,7 +119,7 @@ module API
authorize! :admin_build, user_project
trigger = user_project.triggers.find(params.delete(:trigger_id))
return not_found!('Trigger') unless trigger
break not_found!('Trigger') unless trigger
if trigger.update(owner: current_user)
status :ok
......@@ -140,7 +140,7 @@ module API
authorize! :admin_build, user_project
trigger = user_project.triggers.find(params.delete(:trigger_id))
return not_found!('Trigger') unless trigger
break not_found!('Trigger') unless trigger
destroy_conditionally!(trigger)
end
......
......@@ -51,7 +51,7 @@ module API
get ':id/repository/commits/:sha/builds' do
authorize_read_builds!
return not_found! unless user_project.commit(params[:sha])
break not_found! unless user_project.commit(params[:sha])
pipelines = user_project.pipelines.where(sha: params[:sha])
builds = user_project.builds.where(pipeline: pipelines).order('id DESC')
......@@ -153,7 +153,7 @@ module API
build = get_build!(params[:build_id])
authorize!(:update_build, build)
return forbidden!('Build is not retryable') unless build.retryable?
break forbidden!('Build is not retryable') unless build.retryable?
build = Ci::Build.retry(build, current_user)
......@@ -171,7 +171,7 @@ module API
build = get_build!(params[:build_id])
authorize!(:erase_build, build)
return forbidden!('Build is not erasable!') unless build.erasable?
break forbidden!('Build is not erasable!') unless build.erasable?
build.erase(erased_by: current_user)
present build, with: ::API::V3::Entities::Build
......@@ -188,7 +188,7 @@ module API
build = get_build!(params[:build_id])
authorize!(:update_build, build)
return not_found!(build) unless build.artifacts?
break not_found!(build) unless build.artifacts?
build.keep_artifacts!
......
......@@ -423,7 +423,7 @@ module API
end
unless user_project.allowed_to_share_with_group?
return render_api_error!("The project sharing with group is disabled", 400)
break render_api_error!("The project sharing with group is disabled", 400)
end
link = user_project.project_group_links.new(declared_params(include_missing: false))
......
......@@ -90,7 +90,7 @@ module API
end
put ':id' do
snippet = snippets_for_current_user.find_by(id: params.delete(:id))
return not_found!('Snippet') unless snippet
break not_found!('Snippet') unless snippet
authorize! :update_personal_snippet, snippet
......@@ -114,7 +114,7 @@ module API
end
delete ':id' do
snippet = snippets_for_current_user.find_by(id: params.delete(:id))
return not_found!('Snippet') unless snippet
break not_found!('Snippet') unless snippet
authorize! :destroy_personal_snippet, snippet
snippet.destroy
......@@ -129,7 +129,7 @@ module API
end
get ":id/raw" do
snippet = snippets_for_current_user.find_by(id: params.delete(:id))
return not_found!('Snippet') unless snippet
break not_found!('Snippet') unless snippet
env['api.format'] = :txt
content_type 'text/plain'
......
......@@ -72,7 +72,7 @@ module API
authorize! :admin_build, user_project
trigger = user_project.triggers.find_by(token: params[:token].to_s)
return not_found!('Trigger') unless trigger
break not_found!('Trigger') unless trigger
present trigger, with: ::API::V3::Entities::Trigger
end
......@@ -100,7 +100,7 @@ module API
authorize! :admin_build, user_project
trigger = user_project.triggers.find_by(token: params[:token].to_s)
return not_found!('Trigger') unless trigger
break not_found!('Trigger') unless trigger
trigger.destroy
......
......@@ -31,7 +31,7 @@ module API
key = params[:key]
variable = user_project.variables.find_by(key: key)
return not_found!('Variable') unless variable
break not_found!('Variable') unless variable
present variable, with: Entities::Variable
end
......@@ -67,7 +67,7 @@ module API
put ':id/variables/:key' do
variable = user_project.variables.find_by(key: params[:key])
return not_found!('Variable') unless variable
break not_found!('Variable') unless variable
variable_params = declared_params(include_missing: false).except(:key)
......
......@@ -77,7 +77,7 @@ module DeclarativePolicy
@state = State.new
steps_by_score do |step, score|
return if !debug && @state.prevented?
break if !debug && @state.prevented?
passed = nil
case step.action
......
......@@ -51,7 +51,7 @@ module Gitlab
Gitlab::Auth::UniqueIpsLimiter.limit_user! do
user = User.by_login(login)
return if user && !user.active?
break if user && !user.active?
authenticators = []
......
......@@ -45,7 +45,7 @@ module Gitlab
def append(data, offset)
write do |stream|
current_length = stream.size
return -current_length unless current_length == offset
break -current_length unless current_length == offset
data = job.hide_secrets(data)
stream.append(data, offset)
......
......@@ -87,7 +87,7 @@ module Gitlab
match = matches.flatten.last
coverage = match.gsub(/\d+(\.\d+)?/).first
return coverage if coverage.present?
return coverage if coverage.present? # rubocop:disable Cop/AvoidReturnFromBlocks
end
nil
......
......@@ -30,7 +30,7 @@ module Gitlab
return unless enabled?
@mutex.synchronize do
return thread if thread?
break thread if thread?
@thread = Thread.new { start_working }
end
......@@ -38,7 +38,7 @@ module Gitlab
def stop
@mutex.synchronize do
return unless thread?
break unless thread?
stop_working
......
......@@ -21,7 +21,7 @@ module Gitlab
@text.gsub(@pattern) do |markdown|
file = find_file(@source_project, $~[:secret], $~[:file])
return markdown unless file.try(:exists?)
break markdown unless file.try(:exists?)
new_uploader = FileUploader.new(target_project)
with_link_in_tmp_dir(file.file) do |open_tmp_file|
......
......@@ -249,7 +249,7 @@ module Gitlab
if size >= SIZE_LIMIT
too_large!
return true
return true # rubocop:disable Cop/AvoidReturnFromBlocks
end
end
end
......
......@@ -25,7 +25,9 @@ module Gitlab
stdin.close
if lazy_block
return [lazy_block.call(stdout.lazy), 0]
cmd_output = lazy_block.call(stdout.lazy)
cmd_status = 0
break
else
cmd_output << stdout.read
end
......
......@@ -26,7 +26,7 @@ module Gitlab
# When the remote repo does not have tags.
if target.nil? || path.nil?
Rails.logger.info "Empty or invalid list of tags for remote: #{remote}. Output: #{output}"
return []
break []
end
name = path.split('/', 3).last
......
......@@ -3,18 +3,15 @@ module Gitlab
module_function
def retry_lock(subject, retries = 100, &block)
loop do
begin
ActiveRecord::Base.transaction do
return yield(subject)
end
rescue ActiveRecord::StaleObjectError
retries -= 1
raise unless retries >= 0
subject.reload
end
ActiveRecord::Base.transaction do
yield(subject)
end
rescue ActiveRecord::StaleObjectError
retries -= 1
raise unless retries >= 0
subject.reload
retry
end
alias_method :retry_optimistic_lock, :retry_lock
......
......@@ -340,7 +340,7 @@ module Gitlab
if enabled
gitaly_namespace_client(storage).rename(old_name, new_name)
else
return false if exists?(storage, new_name) || !exists?(storage, old_name)
break false if exists?(storage, new_name) || !exists?(storage, old_name)
FileUtils.mv(full_path(storage, old_name), full_path(storage, new_name))