Merge branch 'id-redundant-return' into 'master'

Enable Style/RedundantReturn cop

See merge request gitlab-org/gitlab!36430

.rubocop_todo.yml

@@ -666,12 +666,6 @@ Style/RedundantFreeze:
 Style/RedundantInterpolation:
   Enabled: false
 
-# Offense count: 33
-# Cop supports --auto-correct.
-# Configuration parameters: AllowMultipleReturnValues.
-Style/RedundantReturn:
-  Enabled: false
-
 # Offense count: 801
 # Cop supports --auto-correct.
 Style/RedundantSelf:

app/controllers/application_controller.rb

@@ -306,7 +306,7 @@ class ApplicationController < ActionController::Base
     return if session[:impersonator_id] || !current_user&.allow_password_authentication?
 
     if current_user&.password_expired?
-      return redirect_to new_profile_password_path
+      redirect_to new_profile_password_path
     end
   end
 
@@ -364,7 +364,7 @@ class ApplicationController < ActionController::Base
 
   def require_email
     if current_user && current_user.temp_oauth_email? && session[:impersonator_id].nil?
-      return redirect_to profile_path, notice: _('Please complete your profile with email address')
+      redirect_to profile_path, notice: _('Please complete your profile with email address')
     end
   end
 

app/controllers/chaos_controller.rb

@@ -45,7 +45,6 @@ class ChaosController < ActionController::Base
     unless Devise.secure_compare(chaos_secret_configured, chaos_secret_request)
       render plain: "To experience chaos, please set a valid `X-Chaos-Secret` header or `token` param",
              status: :unauthorized
-      return
     end
   end
 

app/controllers/concerns/issuable_actions.rb

@@ -197,13 +197,13 @@ module IssuableActions
 
   def authorize_destroy_issuable!
     unless can?(current_user, :"destroy_#{issuable.to_ability_name}", issuable)
-      return access_denied!
+      access_denied!
     end
   end
 
   def authorize_admin_issuable!
     unless can?(current_user, :"admin_#{resource_name}", parent)
-      return access_denied!
+      access_denied!
     end
   end
 

app/controllers/groups/application_controller.rb

@@ -30,25 +30,25 @@ class Groups::ApplicationController < ApplicationController
 
   def authorize_admin_group!
     unless can?(current_user, :admin_group, group)
-      return render_404
+      render_404
     end
   end
 
   def authorize_create_deploy_token!
     unless can?(current_user, :create_deploy_token, group)
-      return render_404
+      render_404
     end
   end
 
   def authorize_destroy_deploy_token!
     unless can?(current_user, :destroy_deploy_token, group)
-      return render_404
+      render_404
     end
   end
 
   def authorize_admin_group_member!
     unless can?(current_user, :admin_group_member, group)
-      return render_403
+      render_403
     end
   end
 

app/controllers/omniauth_callbacks_controller.rb

@@ -200,7 +200,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
   def fail_login(user)
     error_message = user.errors.full_messages.to_sentence
 
-    return redirect_to omniauth_error_path(oauth['provider'], error: error_message)
+    redirect_to omniauth_error_path(oauth['provider'], error: error_message)
   end
 
   def fail_auth0_login

app/controllers/projects/application_controller.rb

@@ -42,7 +42,7 @@ class Projects::ApplicationController < ApplicationController
 
   def authorize_action!(action)
     unless can?(current_user, action, project)
-      return access_denied!
+      access_denied!
     end
   end
 

app/controllers/projects/blob_controller.rb

@@ -129,7 +129,7 @@ class Projects::BlobController < Projects::ApplicationController
         end
       end
 
-      return redirect_to_tree_root_for_missing_path(@project, @ref, @path)
+      redirect_to_tree_root_for_missing_path(@project, @ref, @path)
     end
   end
 

app/controllers/projects/releases_controller.rb

@@ -30,7 +30,7 @@ class Projects::ReleasesController < Projects::ApplicationController
 
   def new
     unless Feature.enabled?(:new_release_page, project)
-      return redirect_to(new_project_tag_path(@project))
+      redirect_to(new_project_tag_path(@project))
     end
   end
 

app/controllers/projects/tree_controller.rb

@@ -20,9 +20,9 @@ class Projects::TreeController < Projects::ApplicationController
 
     if tree.entries.empty?
       if @repository.blob_at(@commit.id, @path)
-        return redirect_to project_blob_path(@project, File.join(@ref, @path))
+        redirect_to project_blob_path(@project, File.join(@ref, @path))
       elsif @path.present?
-        return redirect_to_tree_root_for_missing_path(@project, @ref, @path)
+        redirect_to_tree_root_for_missing_path(@project, @ref, @path)
       end
     end
   end

app/graphql/resolvers/projects/jira_projects_resolver.rb

@@ -37,7 +37,7 @@ module Resolvers
       def jira_projects(name:)
         args = { query: name }.compact
 
-        return Jira::Requests::Projects::ListService.new(project.jira_service, args).execute
+        Jira::Requests::Projects::ListService.new(project.jira_service, args).execute
       end
     end
   end

app/services/access_token_validation_service.rb

@@ -17,21 +17,21 @@ class AccessTokenValidationService
 
   def validate(scopes: [])
     if token.expired?
-      return EXPIRED
+      EXPIRED
 
     elsif token.revoked?
-      return REVOKED
+      REVOKED
 
     elsif !self.include_any_scope?(scopes)
-      return INSUFFICIENT_SCOPE
+      INSUFFICIENT_SCOPE
 
     elsif token.respond_to?(:impersonation) &&
         token.impersonation &&
         !Gitlab.config.gitlab.impersonation_enabled
-      return IMPERSONATION_DISABLED
+      IMPERSONATION_DISABLED
 
     else
-      return VALID
+      VALID
     end
   end
 

app/services/spam/spam_verdict_service.rb

@@ -57,7 +57,7 @@ module Spam
       rescue *Gitlab::HTTP::HTTP_ERRORS => e
         # @TODO: log error via try_post https://gitlab.com/gitlab-org/gitlab/-/issues/219223
         Gitlab::ErrorTracking.log_exception(e)
-        return
+        nil
       rescue
         # @TODO log
         ALLOW

app/workers/delete_merged_branches_worker.rb

@@ -17,7 +17,6 @@ class DeleteMergedBranchesWorker # rubocop:disable Scalability/IdempotentWorker
     begin
       ::Branches::DeleteMergedService.new(project, user).execute
     rescue Gitlab::Access::AccessDeniedError
-      return
     end
   end
 end

ee/app/helpers/ee/application_helper.rb

@@ -33,7 +33,7 @@ module EE
 
       if unprocessed_too_old?
         minutes_behind = time_ago_in_words(next_unprocessed_event.created_at)
-        return (s_('Geo|The node is currently %{minutes_behind} behind the primary node.') %
+        (s_('Geo|The node is currently %{minutes_behind} behind the primary node.') %
           { minutes_behind: minutes_behind }).html_safe
       end
     end

ee/app/services/epics/tree_reorder_service.rb

@@ -74,7 +74,7 @@ module Epics
       return 'Relative position is not valid.' unless valid_relative_position?
 
       if different_epic_parent?
-        return "The sibling object's parent must match the #{new_parent ? "new" : "current"} parent epic."
+        "The sibling object's parent must match the #{new_parent ? "new" : "current"} parent epic."
       end
     end
 

ee/lib/gitlab/geo/geo_node_status_check.rb

@@ -57,7 +57,7 @@ module Gitlab
       end
 
       def replication_verification_complete?
-        return replication_complete? && verification_complete?
+        replication_complete? && verification_complete?
       end
 
       private

ee/lib/gitlab/geo/jwt_request_decoder.rb

@@ -84,7 +84,7 @@ module Gitlab
           raise InvalidSignatureTimeError.new(message)
         rescue JWT::DecodeError => e
           log_error("Error decoding Geo request: #{e}")
-          return
+          nil
         end
       end
 

ee/lib/gitlab/graphql/aggregations/epics/lazy_epic_aggregate.rb

@@ -57,7 +57,7 @@ module Gitlab
             end
 
             unless PERMITTED_FACETS.include?(aggregate_facet.to_sym)
-              return "Invalid aggregate facet #{aggregate_facet} provided."
+              "Invalid aggregate facet #{aggregate_facet} provided."
             end
           end
 

lib/gitlab/ci/pipeline/chain/validate/abilities.rb

@@ -19,7 +19,7 @@ module Gitlab
               end
 
               unless allowed_to_write_ref?
-                return error("Insufficient permissions for protected ref '#{command.ref}'")
+                error("Insufficient permissions for protected ref '#{command.ref}'")
               end
             end
 

lib/gitlab/ci/pipeline/chain/validate/repository.rb

@@ -18,7 +18,7 @@ module Gitlab
               end
 
               if @command.ambiguous_ref?
-                return error('Ref is ambiguous')
+                error('Ref is ambiguous')
               end
             end
 

lib/gitlab/git_ref_validator.rb

@@ -19,7 +19,7 @@ module Gitlab
       begin
         Rugged::Reference.valid_name?("refs/heads/#{ref_name}")
       rescue ArgumentError
-        return false
+        false
       end
     end
 
@@ -35,7 +35,7 @@ module Gitlab
       begin
         Rugged::Reference.valid_name?(expanded_name)
       rescue ArgumentError
-        return false
+        false
       end
     end
   end

lib/gitlab/gitaly_client/repository_service.rb

@@ -201,9 +201,9 @@ module Gitlab
         response = GitalyClient.call(@storage, :repository_service, :fsck, request, timeout: GitalyClient.long_timeout)
 
         if response.error.empty?
-          return "", 0
+          ["", 0]
         else
-          return response.error.b, 1
+          [response.error.b, 1]
         end
       end
 

lib/gitlab/middleware/go.rb

@@ -101,7 +101,7 @@ module Gitlab
 
         if project
           # If a project is found and the user has access, we return the full project path
-          return project.full_path, project.default_branch
+          [project.full_path, project.default_branch]
         else
           # If not, we return the first two components as if it were a simple `namespace/project` path,
           # so that we don't reveal the existence of a nested project the user doesn't have access to.
@@ -112,7 +112,7 @@ module Gitlab
           # `go get gitlab.com/group/subgroup/project/subpackage` will not work for private projects.
           # `go get gitlab.com/group/subgroup/project.git/subpackage` will work, since Go is smart enough
           # to figure that out. `import 'gitlab.com/...'` behaves the same as `go get`.
-          return simple_project_path, 'master'
+          [simple_project_path, 'master']
         end
       end
 

lib/gitlab/utils.rb

@@ -56,7 +56,7 @@ module Gitlab
     #   * Maximum length is 63 bytes
     #   * First/Last Character is not a hyphen
     def slugify(str)
-      return str.downcase
+      str.downcase
         .gsub(/[^a-z0-9]/, '-')[0..62]
         .gsub(/(\A-+|-+\z)/, '')
     end

lib/google_api/auth.rb

@@ -22,7 +22,7 @@ module GoogleApi
 
     def get_token(code)
       ret = client.auth_code.get_token(code, redirect_uri: redirect_uri)
-      return ret.token, ret.expires_at
+      [ret.token, ret.expires_at]
     end
 
     protected

qa/qa/specs/features/ee/browser_ui/1_manage/group/group_file_template_spec.rb

@@ -121,17 +121,14 @@ module QA
       def set_file_template_if_not_already_set
         response = get Runtime::API::Request.new(@api_client, "/groups/#{@group.id}").url
 
-        if parse_body(response)[:file_template_project_id]
-          return
-        else
+        return if parse_body(response)[:file_template_project_id]
+
         @group.visit!
         Page::Group::Menu.perform(&:click_group_general_settings_item)
-
         Page::Group::Settings::General.perform do |general|
           general.choose_file_template_repository(@file_template_project.name)
         end
       end
-      end
 
       def remove_group_file_template_if_set
         response = get Runtime::API::Request.new(@api_client, "/groups/#{@group.id}").url