diff options
author | Toon Claes <toon@gitlab.com> | 2019-06-28 14:28:29 +0200 |
---|---|---|
committer | Toon Claes <toon@gitlab.com> | 2019-06-28 14:28:29 +0200 |
commit | 288b050e5b226e403f9c88c3fe16fb4279714d69 (patch) | |
tree | 9b3f96b71cab616d44000c192ab5870098ee6c5f | |
parent | 046527beb585b8086e8b280dee78f43417b9a753 (diff) | |
download | gitlab-ce-288b050e5b226e403f9c88c3fe16fb4279714d69.tar.gz |
Enable Style/SafeNavigation coptc-no-more-try
I ran `rubocop -a --only Style/SafeNavigation` to correct all.
I could not enable `ConvertTry` cause it does not seem to be supported
by the version we're using.
https://gitlab.com/gitlab-org/gitlab-ce/issues/63875
83 files changed, 105 insertions, 112 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index e5fe527e611..06b096411c2 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -36,7 +36,8 @@ Style/MutableConstant: # TODO: Move this to gitlab-styles Style/SafeNavigation: - Enabled: false + Enabled: true +# ConvertTry: true # Frozen String Literal Style/FrozenStringLiteralComment: diff --git a/app/controllers/admin/impersonations_controller.rb b/app/controllers/admin/impersonations_controller.rb index 65fe22bd8f4..f0e160258f8 100644 --- a/app/controllers/admin/impersonations_controller.rb +++ b/app/controllers/admin/impersonations_controller.rb @@ -12,6 +12,6 @@ class Admin::ImpersonationsController < Admin::ApplicationController private def authenticate_impersonator! - render_404 unless impersonator && impersonator.admin? && !impersonator.blocked? + render_404 unless impersonator&.admin? && !impersonator.blocked? end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 75108bf2646..ed7e5318c95 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -289,7 +289,7 @@ class ApplicationController < ActionController::Base end def ldap_security_check - if current_user && current_user.requires_ldap_check? + if current_user&.requires_ldap_check? return unless current_user.try_obtain_ldap_lease unless Gitlab::Auth::LDAP::Access.allowed?(current_user) @@ -340,7 +340,7 @@ class ApplicationController < ActionController::Base end def require_email - if current_user && current_user.temp_oauth_email? && session[:impersonator_id].nil? + if current_user&.temp_oauth_email? && session[:impersonator_id].nil? return redirect_to profile_path, notice: _('Please complete your profile with email address') end end diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index 4926062f9ca..471493df3e0 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -41,7 +41,7 @@ module AuthenticatesWithTwoFactor authenticate_with_two_factor_via_otp(user) elsif user_params[:device_response].present? && session[:otp_user_id] authenticate_with_two_factor_via_u2f(user) - elsif user && user.valid_password?(user_params[:password]) + elsif user&.valid_password?(user_params[:password]) prompt_for_two_factor(user) end end diff --git a/app/controllers/concerns/lfs_request.rb b/app/controllers/concerns/lfs_request.rb index f7137a04437..290cb5b4c95 100644 --- a/app/controllers/concerns/lfs_request.rb +++ b/app/controllers/concerns/lfs_request.rb @@ -84,7 +84,7 @@ module LfsRequest end def deploy_token_present? - user && user.is_a?(DeployToken) + user&.is_a?(DeployToken) end def deploy_token diff --git a/app/controllers/concerns/sends_blob.rb b/app/controllers/concerns/sends_blob.rb index 8ecdaced9f5..7f715d0d4ec 100644 --- a/app/controllers/concerns/sends_blob.rb +++ b/app/controllers/concerns/sends_blob.rb @@ -51,7 +51,7 @@ module SendsBlob def send_lfs_object(blob) lfs_object = find_lfs_object(blob) - if lfs_object && lfs_object.project_allowed_access?(project) + if lfs_object&.project_allowed_access?(project) send_upload(lfs_object.file, attachment: blob.name) else render_404 @@ -60,7 +60,7 @@ module SendsBlob def find_lfs_object(blob) lfs_object = LfsObject.find_by_oid(blob.lfs_oid) - if lfs_object && lfs_object.file.exists? + if lfs_object&.file&.exists? lfs_object else nil diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb index 59f6d3452a3..78767c1d169 100644 --- a/app/controllers/concerns/uploads_actions.rb +++ b/app/controllers/concerns/uploads_actions.rb @@ -113,7 +113,7 @@ module UploadsActions end def image_or_video? - uploader && uploader.exists? && uploader.image_or_video? + uploader&.exists? && uploader.image_or_video? end def find_model diff --git a/app/controllers/import/gitlab_projects_controller.rb b/app/controllers/import/gitlab_projects_controller.rb index 89889141be6..24441e31b22 100644 --- a/app/controllers/import/gitlab_projects_controller.rb +++ b/app/controllers/import/gitlab_projects_controller.rb @@ -31,7 +31,7 @@ class Import::GitlabProjectsController < Import::BaseController private def file_is_valid? - return false unless project_params[:file] && project_params[:file].respond_to?(:read) + return false unless project_params[:file]&.respond_to?(:read) filename = project_params[:file].original_filename diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 2a8dd997d04..4962a187824 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -35,7 +35,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController error ||= exception.message if exception.respond_to?(:message) error ||= request.env["omniauth.error.type"].to_s - error.to_s.humanize if error + error&.to_s&.humanize end def saml diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 77de5cb45c9..cd48584e987 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -56,7 +56,7 @@ class PasswordsController < Devise::PasswordsController end def throttle_reset - return unless resource && resource.recently_sent_password_reset? + return unless resource&.recently_sent_password_reset? # Throttle reset attempts, but return a normal message to # avoid user enumeration attack. diff --git a/app/controllers/projects/build_artifacts_controller.rb b/app/controllers/projects/build_artifacts_controller.rb index 4274c356227..7c1b98930f8 100644 --- a/app/controllers/projects/build_artifacts_controller.rb +++ b/app/controllers/projects/build_artifacts_controller.rb @@ -31,7 +31,7 @@ class Projects::BuildArtifactsController < Projects::ApplicationController private def validate_artifacts! - render_404 unless job && job.artifacts? + render_404 unless job&.artifacts? end def extract_ref_name_and_path diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb index 013e01b82aa..12abe7f5093 100644 --- a/app/controllers/projects/lfs_storage_controller.rb +++ b/app/controllers/projects/lfs_storage_controller.rb @@ -9,7 +9,7 @@ class Projects::LfsStorageController < Projects::GitHttpClientController def download lfs_object = LfsObject.find_by_oid(oid) - unless lfs_object && lfs_object.file.exists? + unless lfs_object&.file&.exists? render_lfs_not_found return end diff --git a/app/controllers/sent_notifications_controller.rb b/app/controllers/sent_notifications_controller.rb index 77757c4a3ef..7b7055c1fb1 100644 --- a/app/controllers/sent_notifications_controller.rb +++ b/app/controllers/sent_notifications_controller.rb @@ -6,7 +6,7 @@ class SentNotificationsController < ApplicationController def unsubscribe @sent_notification = SentNotification.for(params[:id]) - return render_404 unless @sent_notification && @sent_notification.unsubscribable? + return render_404 unless @sent_notification&.unsubscribable? return unsubscribe_and_redirect if current_user || params[:force] end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index a841859621e..fe5de61fa6b 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -128,7 +128,7 @@ class SessionsController < Devise::SessionsController user = User.admins.last - return unless user && user.require_password_creation_for_web? + return unless user&.require_password_creation_for_web? Users::UpdateService.new(current_user, user: user).execute do |user| @token = user.generate_reset_token diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index ffa5719fefb..1cfc4cf6cdc 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -69,7 +69,7 @@ module ApplicationHelper # rubocop: disable CodeReuse/ActiveRecord def show_last_push_widget?(event) # Skip if event is not about added or modified non-master branch - return false unless event && event.last_push_to_non_root? && !event.rm_ref? + return false unless event&.last_push_to_non_root? && !event.rm_ref? project = event.project diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 0d6a6496993..1a2abc19dcc 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -181,9 +181,9 @@ module BlobHelper { 'relative-url-root' => Rails.application.config.relative_url_root, 'assets-prefix' => Gitlab::Application.config.assets.prefix, - 'blob-filename' => @blob && @blob.path, + 'blob-filename' => @blob&.path, 'project-id' => project.id, - 'is-markdown' => @blob && @blob.path && Gitlab::MarkupHelper.gitlab_markdown?(@blob.path) + 'is-markdown' => @blob&.path && Gitlab::MarkupHelper.gitlab_markdown?(@blob.path) } end diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 32431959851..77c2622304c 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -71,12 +71,12 @@ module DiffHelper discussions_left = discussions_right = nil - if left && left.discussable? && (left.unchanged? || left.removed?) + if left&.discussable? && (left.unchanged? || left.removed?) line_code = diff_file.line_code(left) discussions_left = @grouped_diff_discussions[line_code] end - if right && right.discussable? && right.added? + if right&.discussable? && right.added? line_code = diff_file.line_code(right) discussions_right = @grouped_diff_discussions[line_code] end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 045de105b77..4036753fecf 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -214,7 +214,7 @@ module IssuablesHelper def issuable_labels_tooltip(labels, limit: 5) first, last = labels.partition.with_index { |_, i| i < limit } - if labels && labels.any? + if labels&.any? label_names = first.collect { |label| label.fetch(:title) } label_names << "and #{last.size} more" unless last.empty? diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb index 572d68cb4a3..5e271f18693 100644 --- a/app/helpers/namespaces_helper.rb +++ b/app/helpers/namespaces_helper.rb @@ -14,7 +14,7 @@ module NamespacesHelper extra_group = Group.find(extra_group) if Namespace.find(extra_group).kind == 'group' end - if extra_group && extra_group.is_a?(Group) + if extra_group&.is_a?(Group) extra_group = dedup_extra_group(extra_group) if Ability.allowed?(current_user, :read_group, extra_group) diff --git a/app/helpers/runners_helper.rb b/app/helpers/runners_helper.rb index 0d880c38a7b..d6dd4c3fe6d 100644 --- a/app/helpers/runners_helper.rb +++ b/app/helpers/runners_helper.rb @@ -20,7 +20,7 @@ module RunnersHelper display_name = truncate(runner.display_name, length: 15) id = "\##{runner.id}" - if current_user && current_user.admin + if current_user&.admin link_to admin_runner_path(runner) do display_name + id end diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index f5c4686a3bf..2544dfc7130 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -89,7 +89,7 @@ module SearchHelper # Autocomplete results for the current project, if it's defined def project_autocomplete - if @project && @project.repository.exists? && @project.repository.root_ref + if @project&.repository&.exists? && @project.repository.root_ref ref = @ref || @project.repository.root_ref [ diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index 4690b6ffbe1..3f4e3e24429 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -80,7 +80,7 @@ module TreeHelper def tree_edit_project(project = @project) if can?(current_user, :push_code, project) project - elsif current_user && current_user.already_forked?(project) + elsif current_user&.already_forked?(project) current_user.fork_of(project) end end diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index df4caed175d..30fbd6c4b37 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -250,7 +250,7 @@ module ApplicationSettingImplementation end def archive_builds_older_than - archive_builds_in_seconds.seconds.ago if archive_builds_in_seconds + archive_builds_in_seconds&.seconds&.ago end private diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 89cc082d0bc..a53471ef936 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -397,7 +397,7 @@ module Ci end def environment_action - self.options.fetch(:environment, {}).fetch(:action, 'start') if self.options + self.options&.fetch(:environment, {})&.fetch(:action, 'start') end def has_deployment? diff --git a/app/models/concerns/discussion_on_diff.rb b/app/models/concerns/discussion_on_diff.rb index e4e5928f5cf..1a1631c357a 100644 --- a/app/models/concerns/discussion_on_diff.rb +++ b/app/models/concerns/discussion_on_diff.rb @@ -79,8 +79,7 @@ module DiscussionOnDiff def fetch_preloaded_diff_file fetch_preloaded_diff = - context_noteable && - context_noteable.preloads_discussion_diff_highlighting? && + context_noteable&.preloads_discussion_diff_highlighting? && note_diff_file context_noteable.discussions_diffs.find_by_id(note_diff_file.id) if fetch_preloaded_diff diff --git a/app/models/concerns/ghost_user.rb b/app/models/concerns/ghost_user.rb index 15278c431fb..e7255ae710a 100644 --- a/app/models/concerns/ghost_user.rb +++ b/app/models/concerns/ghost_user.rb @@ -4,6 +4,6 @@ module GhostUser extend ActiveSupport::Concern def ghost_user? - user && user.ghost? + user&.ghost? end end diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb index 3deb86da6cf..73b6e74eaa0 100644 --- a/app/models/concerns/milestoneish.rb +++ b/app/models/concerns/milestoneish.rb @@ -66,7 +66,7 @@ module Milestoneish end def upcoming? - start_date && start_date.future? + start_date&.future? end def expires_at @@ -80,7 +80,7 @@ module Milestoneish end def expired? - due_date && due_date.past? + due_date&.past? end def group_milestone? diff --git a/app/models/concerns/spammable.rb b/app/models/concerns/spammable.rb index 3ff4b4046d3..efcf7c86ad5 100644 --- a/app/models/concerns/spammable.rb +++ b/app/models/concerns/spammable.rb @@ -26,7 +26,7 @@ module Spammable end def submittable_as_spam_by?(current_user) - current_user && current_user.admin? && submittable_as_spam? + current_user&.admin? && submittable_as_spam? end def submittable_as_spam? diff --git a/app/models/concerns/strip_attribute.rb b/app/models/concerns/strip_attribute.rb index 8f6a6244dd3..1be24e3d57d 100644 --- a/app/models/concerns/strip_attribute.rb +++ b/app/models/concerns/strip_attribute.rb @@ -30,7 +30,7 @@ module StripAttribute def strip_attributes self.class.strip_attrs.each do |attr| - self[attr].strip! if self[attr] && self[attr].respond_to?(:strip!) + self[attr].strip! if self[attr]&.respond_to?(:strip!) end end end diff --git a/app/models/group.rb b/app/models/group.rb index 8e89c7ecfb1..fa752201d3c 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -155,7 +155,7 @@ class Group < Namespace end def visibility_level_allowed_by_parent?(level = self.visibility_level) - return true unless parent_id && parent_id.nonzero? + return true unless parent_id&.nonzero? level <= parent.visibility_level end diff --git a/app/models/issue.rb b/app/models/issue.rb index 30e29911758..bb0dc336191 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -208,7 +208,7 @@ class Issue < ApplicationRecord # Returns `true` if the current issue can be viewed by either a logged in User # or an anonymous user. def visible_to_user?(user = nil) - return false unless project && project.feature_available?(:issues, user) + return false unless project&.feature_available?(:issues, user) return publicly_visible? unless user diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 82034f5946b..f1c41766015 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -930,7 +930,7 @@ class MergeRequest < ApplicationRecord end def source_project_namespace - if source_project && source_project.namespace + if source_project&.namespace source_project.namespace.full_path else "(removed)" @@ -938,7 +938,7 @@ class MergeRequest < ApplicationRecord end def target_project_namespace - if target_project && target_project.namespace + if target_project&.namespace target_project.namespace.full_path else "(removed)" @@ -1271,7 +1271,7 @@ class MergeRequest < ApplicationRecord end def has_complete_diff_refs? - diff_refs && diff_refs.complete? + diff_refs&.complete? end # rubocop: disable CodeReuse/ServiceClass diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index f54497fc6d8..4cbd368d0bd 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -95,8 +95,7 @@ class IssueTrackerService < Service private def enabled_in_gitlab_config - Gitlab.config.issues_tracker && - Gitlab.config.issues_tracker.values.any? && + Gitlab.config.issues_tracker&.values&.any? && issues_tracker end diff --git a/app/models/ref_matcher.rb b/app/models/ref_matcher.rb index fa7d2c0f06c..991b171bc7c 100644 --- a/app/models/ref_matcher.rb +++ b/app/models/ref_matcher.rb @@ -20,7 +20,7 @@ class RefMatcher # Checks if this protected ref contains a wildcard def wildcard? - @ref_name_or_pattern && @ref_name_or_pattern.include?('*') + @ref_name_or_pattern&.include?('*') end protected diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index 134de1c9ace..38cdacfd8a6 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -9,7 +9,7 @@ class GlobalPolicy < BasePolicy with_options scope: :user, score: 0 condition(:access_locked) { @user&.access_locked? } - condition(:can_create_fork, scope: :user) { @user && @user.manageable_namespaces.any? { |namespace| @user.can?(:create_projects, namespace) } } + condition(:can_create_fork, scope: :user) { @user&.manageable_namespaces&.any? { |namespace| @user.can?(:create_projects, namespace) } } condition(:required_terms_not_accepted, scope: :user, score: 0) do @user&.required_terms_not_accepted? diff --git a/app/serializers/environment_entity.rb b/app/serializers/environment_entity.rb index 8258135da4e..85d1ac370c6 100644 --- a/app/serializers/environment_entity.rb +++ b/app/serializers/environment_entity.rb @@ -55,7 +55,7 @@ class EnvironmentEntity < Grape::Entity end def cluster_platform_kubernetes? - deployment_platform && deployment_platform.is_a?(Clusters::Platforms::Kubernetes) + deployment_platform&.is_a?(Clusters::Platforms::Kubernetes) end def deployment_platform diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb index 3f0aedfbfb2..3723fa98ab0 100644 --- a/app/services/compare_service.rb +++ b/app/services/compare_service.rb @@ -15,7 +15,7 @@ class CompareService def execute(target_project, target_ref, base_sha: nil, straight: false) raw_compare = target_project.repository.compare_source_branch(target_ref, start_project.repository, start_ref_name, straight: straight) - return unless raw_compare && raw_compare.base && raw_compare.head + return unless raw_compare&.base && raw_compare.head Compare.new(raw_compare, target_project, diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 0066cd0491f..f8f1641a275 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -90,7 +90,7 @@ module MergeRequests merge_request.update(merge_error: nil) - if merge_request.head_pipeline && merge_request.head_pipeline.active? + if merge_request.head_pipeline&.active? AutoMergeService.new(project, current_user).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) else merge_request.merge_async(current_user.id, {}) diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 9f335cceb67..9ea1b8f2f2a 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -65,7 +65,7 @@ module Projects message = "Unable to save #{e.record.type}: #{e.record.errors.full_messages.join(", ")} " fail(error: message) rescue => e - @project.errors.add(:base, e.message) if @project + @project&.errors&.add(:base, e.message) fail(error: e.message) end @@ -153,7 +153,7 @@ module Projects log_message << " Project ID: #{@project.id}" if @project&.id Rails.logger.error(log_message) - if @project && @project.persisted? && @project.import_state + if @project&.persisted? && @project.import_state @project.import_state.mark_as_failed(message) end diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb index 9af59b0aceb..307350e76d5 100644 --- a/app/uploaders/avatar_uploader.rb +++ b/app/uploaders/avatar_uploader.rb @@ -7,7 +7,7 @@ class AvatarUploader < GitlabUploader prepend ObjectStorage::Extension::RecordsUploads def exists? - model.avatar.file && model.avatar.file.present? + model.avatar.file&.present? end def move_to_store diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index 0a44d60778d..6320700b038 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -438,7 +438,7 @@ module ObjectStorage file rescue => e # in case of failure delete new file - new_file.delete unless new_file.nil? + new_file&.delete # revert back to the old file self.object_store = from_object_store self.file = file_to_delete diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb index 00b51f92b12..08f83c84546 100644 --- a/app/uploaders/records_uploads.rb +++ b/app/uploaders/records_uploads.rb @@ -21,7 +21,7 @@ module RecordsUploads # rubocop: disable CodeReuse/ActiveRecord def record_upload(_tempfile = nil) return unless model - return unless file && file.exists? + return unless file&.exists? # MySQL InnoDB may encounter a deadlock if a deletion and an # insert is in the same transaction due to its next-key locking @@ -36,7 +36,7 @@ module RecordsUploads def readd_upload uploads.where(path: upload_path).delete_all - upload.delete if upload + upload&.delete self.upload = build_upload.tap(&:save!) end @@ -73,7 +73,7 @@ module RecordsUploads # Called `before :remove` # rubocop: disable CodeReuse/ActiveRecord def destroy_upload(*args) - return unless file && file.exists? + return unless file&.exists? self.upload = nil uploads.where(path: upload_path).delete_all diff --git a/app/validators/branch_filter_validator.rb b/app/validators/branch_filter_validator.rb index 6a0899be850..039dafff98a 100644 --- a/app/validators/branch_filter_validator.rb +++ b/app/validators/branch_filter_validator.rb @@ -14,7 +14,7 @@ # class BranchFilterValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - value.squish! unless value.nil? + value&.squish! if value.present? value_without_wildcards = value.tr('*', 'x') diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index dba7837bd12..2b9d8fe408e 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -50,8 +50,7 @@ class PostReceive Git::BranchPushService end - if service_klass - service_klass.new( + service_klass&.new( post_received.project, user, oldrev: oldrev, @@ -59,8 +58,7 @@ class PostReceive ref: ref, push_options: post_received.push_options, create_pipelines: index < PIPELINE_PROCESS_LIMIT || Feature.enabled?(:git_push_create_all_pipelines, post_received.project) - ).execute - end + )&.execute changes << Gitlab::DataBuilder::Repository.single_change(oldrev, newrev, ref) refs << ref diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index aeaf61cda39..4a4b13504c5 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -169,7 +169,7 @@ module Backup end def skipped?(item) - settings[:skipped] && settings[:skipped].include?(item) || disabled_features.include?(item) + settings[:skipped]&.include?(item) || disabled_features.include?(item) end private diff --git a/lib/banzai/filter/commit_reference_filter.rb b/lib/banzai/filter/commit_reference_filter.rb index c3e5ac41cb8..6b2c77a287b 100644 --- a/lib/banzai/filter/commit_reference_filter.rb +++ b/lib/banzai/filter/commit_reference_filter.rb @@ -21,7 +21,7 @@ module Banzai def find_object(project, id) return unless project.is_a?(Project) - if project && project.valid_repo? + if project&.valid_repo? # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/43894 Gitlab::GitalyClient.allow_n_plus_1_calls { project.commit(id) } end diff --git a/lib/banzai/filter/gollum_tags_filter.rb b/lib/banzai/filter/gollum_tags_filter.rb index 0c1bbd2d250..12a70b2dc29 100644 --- a/lib/banzai/filter/gollum_tags_filter.rb +++ b/lib/banzai/filter/gollum_tags_filter.rb @@ -171,7 +171,7 @@ module Banzai end def project_wiki_base_path - project_wiki && project_wiki.wiki_base_path + project_wiki&.wiki_base_path end # Ensure that a :project_wiki key exists in context diff --git a/lib/banzai/filter/table_of_contents_filter.rb b/lib/banzai/filter/table_of_contents_filter.rb index ade4d260be1..20b339ccd48 100644 --- a/lib/banzai/filter/table_of_contents_filter.rb +++ b/lib/banzai/filter/table_of_contents_filter.rb @@ -84,7 +84,7 @@ module Banzai @children = [] @parent = find_parent(previous_header) - @parent.children.push(self) if @parent + @parent&.children&.push(self) end def level diff --git a/lib/banzai/filter/wiki_link_filter/rewriter.rb b/lib/banzai/filter/wiki_link_filter/rewriter.rb index 77b5053f38c..caf6724b8a4 100644 --- a/lib/banzai/filter/wiki_link_filter/rewriter.rb +++ b/lib/banzai/filter/wiki_link_filter/rewriter.rb @@ -8,7 +8,7 @@ module Banzai def initialize(link_string, wiki:, slug:) @uri = Addressable::URI.parse(link_string) - @wiki_base_path = wiki && wiki.wiki_base_path + @wiki_base_path = wiki&.wiki_base_path @slug = slug end diff --git a/lib/bitbucket/page.rb b/lib/bitbucket/page.rb index 7cc1342ad65..300d16de278 100644 --- a/lib/bitbucket/page.rb +++ b/lib/bitbucket/page.rb @@ -24,7 +24,7 @@ module Bitbucket end def parse_values(raw, bitbucket_rep_class) - return [] unless raw['values'] && raw['values'].is_a?(Array) + return [] unless raw['values']&.is_a?(Array) bitbucket_rep_class.decorate(raw['values']) end diff --git a/lib/bitbucket_server/page.rb b/lib/bitbucket_server/page.rb index 5d9a3168876..356b57063a6 100644 --- a/lib/bitbucket_server/page.rb +++ b/lib/bitbucket_server/page.rb @@ -24,7 +24,7 @@ module BitbucketServer end def parse_values(raw, bitbucket_rep_class) - return [] unless raw['values'] && raw['values'].is_a?(Array) + return [] unless raw['values']&.is_a?(Array) bitbucket_rep_class.decorate(raw['values']) end diff --git a/lib/container_registry/client.rb b/lib/container_registry/client.rb index c80f49f5ae0..c4c85dec5c0 100644 --- a/lib/container_registry/client.rb +++ b/lib/container_registry/client.rb @@ -73,7 +73,7 @@ module ContainerRegistry response = redirect_response(response.headers['location']) end - response.body if response && response.success? + response.body if response&.success? end def redirect_response(location) diff --git a/lib/flowdock/git/builder.rb b/lib/flowdock/git/builder.rb index 6f4428d1f42..29cf325c383 100644 --- a/lib/flowdock/git/builder.rb +++ b/lib/flowdock/git/builder.rb @@ -52,7 +52,7 @@ module Flowdock def body content = @commit[:message][first_line.size..-1] - content.strip! if content + content&.strip! "<pre>#{content}</pre>" unless content.empty? end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 4317992d933..ee8ad1de8db 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -133,7 +133,7 @@ module Gitlab # in the Service.available_services_names whitelist. service = project.public_send("#{underscored_service}_service") # rubocop:disable GitlabSecurity/PublicSend - if service && service.activated? && service.valid_token?(password) + if service&.activated? && service.valid_token?(password) Gitlab::Auth::Result.new(nil, project, :ci, build_authentication_abilities) end end @@ -172,7 +172,7 @@ module Gitlab end def valid_oauth_token?(token) - token && token.accessible? && valid_scoped_token?(token, [:api]) + token&.accessible? && valid_scoped_token?(token, [:api]) end def valid_scoped_token?(token, scopes) diff --git a/lib/gitlab/auth/ldap/user.rb b/lib/gitlab/auth/ldap/user.rb index 9c71671f409..a91380736f3 100644 --- a/lib/gitlab/auth/ldap/user.rb +++ b/lib/gitlab/auth/ldap/user.rb @@ -17,7 +17,7 @@ module Gitlab def find_by_uid_and_provider(uid, provider) identity = ::Identity.with_extern_uid(provider, uid).take - identity && identity.user + identity&.user end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index 1c3ce08be76..6c170942a7d 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -98,7 +98,7 @@ module Gitlab def read_uint32(gz) binary = gz.read(4) - binary.unpack1('L>') if binary + binary&.unpack1('L>') end def read_string(gz) diff --git a/lib/gitlab/ci/trace/chunked_io.rb b/lib/gitlab/ci/trace/chunked_io.rb index 8c6fd56493f..4bbf3a44b84 100644 --- a/lib/gitlab/ci/trace/chunked_io.rb +++ b/lib/gitlab/ci/trace/chunked_io.rb @@ -88,9 +88,7 @@ module Gitlab out = out.join # If outbuf is passed, we put the output into the buffer. This supports IO.copy_stream functionality - if outbuf - outbuf.replace(out) - end + outbuf&.replace(out) out end diff --git a/lib/gitlab/cleanup/project_uploads.rb b/lib/gitlab/cleanup/project_uploads.rb index 82a405362c2..a45e99ac939 100644 --- a/lib/gitlab/cleanup/project_uploads.rb +++ b/lib/gitlab/cleanup/project_uploads.rb @@ -41,7 +41,7 @@ module Gitlab # rubocop: disable CodeReuse/ActiveRecord def find_correct_path(upload_path) upload = Upload.find_by(uploader: 'FileUploader', path: upload_path) - return unless upload && upload.local? && upload.model + return unless upload&.local? && upload.model upload.absolute_path rescue => e diff --git a/lib/gitlab/cluster/lifecycle_events.rb b/lib/gitlab/cluster/lifecycle_events.rb index e0f9eb59924..16d9bce8de8 100644 --- a/lib/gitlab/cluster/lifecycle_events.rb +++ b/lib/gitlab/cluster/lifecycle_events.rb @@ -68,7 +68,7 @@ module Gitlab end def do_master_restart - @master_restart_hooks && @master_restart_hooks.each do |block| + @master_restart_hooks&.each do |block| block.call end end diff --git a/lib/gitlab/config/entry/configurable.rb b/lib/gitlab/config/entry/configurable.rb index b7ec4b7c4f8..70a04ff95fe 100644 --- a/lib/gitlab/config/entry/configurable.rb +++ b/lib/gitlab/config/entry/configurable.rb @@ -85,7 +85,7 @@ module Gitlab end define_method("#{symbol}_value") do - return unless entries[symbol] && entries[symbol].valid? + return unless entries[symbol]&.valid? entries[symbol].value end diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb index d2484217ab9..16ca921a653 100644 --- a/lib/gitlab/diff/highlight.rb +++ b/lib/gitlab/diff/highlight.rb @@ -48,7 +48,7 @@ module Gitlab private def highlight_line(diff_line) - return unless diff_file && diff_file.diff_refs + return unless diff_file&.diff_refs rich_line = if diff_line.unchanged? || diff_line.added? diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 2c53f9b026d..6b520949330 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -34,7 +34,7 @@ module Gitlab private def fb_session - @import_data_credentials ||= project.import_data.credentials[:fb_session] if project.import_data && project.import_data.credentials + @import_data_credentials ||= project.import_data.credentials[:fb_session] if project.import_data&.credentials end def user_map diff --git a/lib/gitlab/github_import/representation/issue.rb b/lib/gitlab/github_import/representation/issue.rb index f3071b3e2b3..4ebf768e3db 100644 --- a/lib/gitlab/github_import/representation/issue.rb +++ b/lib/gitlab/github_import/representation/issue.rb @@ -64,7 +64,7 @@ module Gitlab end def labels? - label_names && label_names.any? + label_names&.any? end def pull_request? diff --git a/lib/gitlab/gitlab_import/importer.rb b/lib/gitlab/gitlab_import/importer.rb index e84863deba8..8281eebb96e 100644 --- a/lib/gitlab/gitlab_import/importer.rb +++ b/lib/gitlab/gitlab_import/importer.rb @@ -8,7 +8,7 @@ module Gitlab def initialize(project) @project = project import_data = project.import_data - if import_data && import_data.credentials && import_data.credentials[:password] + if import_data&.credentials && import_data.credentials[:password] @client = Client.new(import_data.credentials[:password]) @formatter = Gitlab::ImportFormatter.new else @@ -57,7 +57,7 @@ module Gitlab # rubocop: disable CodeReuse/ActiveRecord def gitlab_user_id(project, gitlab_id) user = User.joins(:identities).find_by("identities.extern_uid = ? AND identities.provider = 'gitlab'", gitlab_id.to_s) - (user && user.id) || project.creator_id + (user&.id) || project.creator_id end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/lib/gitlab/http_io.rb b/lib/gitlab/http_io.rb index 6a9fb85b054..94e40621dcc 100644 --- a/lib/gitlab/http_io.rb +++ b/lib/gitlab/http_io.rb @@ -95,9 +95,7 @@ module Gitlab out = out.join # If outbuf is passed, we put the output into the buffer. This supports IO.copy_stream functionality - if outbuf - outbuf.replace(out) - end + outbuf&.replace(out) out end diff --git a/lib/gitlab/import_export/after_export_strategies/web_upload_strategy.rb b/lib/gitlab/import_export/after_export_strategies/web_upload_strategy.rb index acb7f225b17..6bea8369cd6 100644 --- a/lib/gitlab/import_export/after_export_strategies/web_upload_strategy.rb +++ b/lib/gitlab/import_export/after_export_strategies/web_upload_strategy.rb @@ -39,7 +39,7 @@ module Gitlab def send_file Gitlab::HTTP.public_send(http_method.downcase, url, send_file_options) # rubocop:disable GitlabSecurity/PublicSend ensure - export_file.close if export_file + export_file&.close end def export_file diff --git a/lib/gitlab/incoming_email.rb b/lib/gitlab/incoming_email.rb index 8b346f6d7d2..347ba94896d 100644 --- a/lib/gitlab/incoming_email.rb +++ b/lib/gitlab/incoming_email.rb @@ -12,7 +12,7 @@ module Gitlab end def supports_wildcard? - config.address && config.address.include?(WILDCARD_PLACEHOLDER) + config.address&.include?(WILDCARD_PLACEHOLDER) end def supports_issue_creation? diff --git a/lib/gitlab/kubernetes/helm/install_command.rb b/lib/gitlab/kubernetes/helm/install_command.rb index e33ba9305ce..5436ef650d6 100644 --- a/lib/gitlab/kubernetes/helm/install_command.rb +++ b/lib/gitlab/kubernetes/helm/install_command.rb @@ -59,11 +59,11 @@ module Gitlab end def preinstall_command - preinstall.join("\n") if preinstall + preinstall&.join("\n") end def postinstall_command - postinstall.join("\n") if postinstall + postinstall&.join("\n") end def install_flag diff --git a/lib/gitlab/metrics/sidekiq_metrics_exporter.rb b/lib/gitlab/metrics/sidekiq_metrics_exporter.rb index 71a5406815f..c2b0b230f4d 100644 --- a/lib/gitlab/metrics/sidekiq_metrics_exporter.rb +++ b/lib/gitlab/metrics/sidekiq_metrics_exporter.rb @@ -33,7 +33,7 @@ module Gitlab end def stop_working - server.shutdown if server + server&.shutdown @server = nil end diff --git a/lib/gitlab/push_options.rb b/lib/gitlab/push_options.rb index 3137676ba4b..623e2612bac 100644 --- a/lib/gitlab/push_options.rb +++ b/lib/gitlab/push_options.rb @@ -64,7 +64,7 @@ module Gitlab def valid_option?(namespace, key) keys = VALID_OPTIONS.dig(namespace, :keys) - keys && keys.include?(key.to_sym) + keys&.include?(key.to_sym) end end end diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 00f817c2399..263e48cffea 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -34,7 +34,7 @@ module Gitlab end def issues - if project && project.jira_tracker? + if project&.jira_tracker? if project.issues_enabled? @references[:all_issues] ||= references(:external_issue) + references(:issue) else diff --git a/lib/gitlab/url_sanitizer.rb b/lib/gitlab/url_sanitizer.rb index 215454fe63c..a621367a522 100644 --- a/lib/gitlab/url_sanitizer.rb +++ b/lib/gitlab/url_sanitizer.rb @@ -92,7 +92,7 @@ module Gitlab end def valid_credentials? - credentials && credentials.is_a?(Hash) && credentials.any? + credentials&.is_a?(Hash) && credentials.any? end def encode_percent(string) diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index 097b502316e..d8a02f4283b 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -99,7 +99,7 @@ module Gitlab end def can_access_git? - user && user.can?(:access_git) + user&.can?(:access_git) end def protected_branch_accessible_to?(ref, action:) diff --git a/lib/tasks/gitlab/backup.rake b/lib/tasks/gitlab/backup.rake index 2bf71701b57..89f5b5a0592 100644 --- a/lib/tasks/gitlab/backup.rake +++ b/lib/tasks/gitlab/backup.rake @@ -82,7 +82,7 @@ namespace :gitlab do task create: :gitlab_environment do puts_time "Dumping repositories ...".color(:blue) - if ENV["SKIP"] && ENV["SKIP"].include?("repositories") + if ENV["SKIP"]&.include?("repositories") puts_time "[SKIPPED]".color(:cyan) else Backup::Repository.new(progress).dump @@ -101,7 +101,7 @@ namespace :gitlab do task create: :gitlab_environment do puts_time "Dumping database ... ".color(:blue) - if ENV["SKIP"] && ENV["SKIP"].include?("db") + if ENV["SKIP"]&.include?("db") puts_time "[SKIPPED]".color(:cyan) else Backup::Database.new(progress).dump @@ -120,7 +120,7 @@ namespace :gitlab do task create: :gitlab_environment do puts_time "Dumping builds ... ".color(:blue) - if ENV["SKIP"] && ENV["SKIP"].include?("builds") + if ENV["SKIP"]&.include?("builds") puts_time "[SKIPPED]".color(:cyan) else Backup::Builds.new(progress).dump @@ -139,7 +139,7 @@ namespace :gitlab do task create: :gitlab_environment do puts_time "Dumping uploads ... ".color(:blue) - if ENV["SKIP"] && ENV["SKIP"].include?("uploads") + if ENV["SKIP"]&.include?("uploads") puts_time "[SKIPPED]".color(:cyan) else Backup::Uploads.new(progress).dump @@ -158,7 +158,7 @@ namespace :gitlab do task create: :gitlab_environment do puts_time "Dumping artifacts ... ".color(:blue) - if ENV["SKIP"] && ENV["SKIP"].include?("artifacts") + if ENV["SKIP"]&.include?("artifacts") puts_time "[SKIPPED]".color(:cyan) else Backup::Artifacts.new(progress).dump @@ -177,7 +177,7 @@ namespace :gitlab do task create: :gitlab_environment do puts_time "Dumping pages ... ".color(:blue) - if ENV["SKIP"] && ENV["SKIP"].include?("pages") + if ENV["SKIP"]&.include?("pages") puts_time "[SKIPPED]".color(:cyan) else Backup::Pages.new(progress).dump @@ -196,7 +196,7 @@ namespace :gitlab do task create: :gitlab_environment do puts_time "Dumping lfs objects ... ".color(:blue) - if ENV["SKIP"] && ENV["SKIP"].include?("lfs") + if ENV["SKIP"]&.include?("lfs") puts_time "[SKIPPED]".color(:cyan) else Backup::Lfs.new(progress).dump @@ -216,7 +216,7 @@ namespace :gitlab do puts_time "Dumping container registry images ... ".color(:blue) if Gitlab.config.registry.enabled - if ENV["SKIP"] && ENV["SKIP"].include?("registry") + if ENV["SKIP"]&.include?("registry") puts_time "[SKIPPED]".color(:cyan) else Backup::Registry.new(progress).dump diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index d0fe2987b0a..fe115393ceb 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -101,7 +101,7 @@ module QA # replace with (..., page = self.class) def click_element(name, page = nil) find_element(name).click - page.validate_elements_present! if page + page&.validate_elements_present! end def fill_element(name, content) diff --git a/qa/qa/resource/base.rb b/qa/qa/resource/base.rb index 283fc6cdbcb..3885f284eb6 100644 --- a/qa/qa/resource/base.rb +++ b/qa/qa/resource/base.rb @@ -88,7 +88,7 @@ module QA end def self.do_fabricate!(resource:, prepare_block:, parents: []) - prepare_block.call(resource) if prepare_block + prepare_block&.call(resource) resource_web_url = yield resource.web_url = resource_web_url diff --git a/qa/qa/runtime/env.rb b/qa/qa/runtime/env.rb index 96f337dc081..181a65527cd 100644 --- a/qa/qa/runtime/env.rb +++ b/qa/qa/runtime/env.rb @@ -57,7 +57,7 @@ module QA end def qa_cookies - ENV['QA_COOKIES'] && ENV['QA_COOKIES'].split(';') + ENV['QA_COOKIES']&.split(';') end def signup_disabled? diff --git a/qa/qa/service/kubernetes_cluster.rb b/qa/qa/service/kubernetes_cluster.rb index 16a89591eb2..d188f2553e7 100644 --- a/qa/qa/service/kubernetes_cluster.rb +++ b/qa/qa/service/kubernetes_cluster.rb @@ -136,7 +136,7 @@ module QA gcloud_account_email = Runtime::Env.gcloud_account_email shell("gcloud auth activate-service-account #{gcloud_account_email} --key-file #{gcloud_account_key.path}") ensure - gcloud_account_key && gcloud_account_key.unlink + gcloud_account_key&.unlink end end end diff --git a/qa/qa/support/retrier.rb b/qa/qa/support/retrier.rb index 8be4e9f5365..4b01850db45 100644 --- a/qa/qa/support/retrier.rb +++ b/qa/qa/support/retrier.rb @@ -15,7 +15,7 @@ module QA yield rescue StandardError, RSpec::Expectations::ExpectationNotMetError sleep sleep_interval - reload_page.refresh if reload_page + reload_page&.refresh attempts += 1 retry if attempts < max_attempts diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 28fa5d12d9c..f4c9bd3e70b 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -28,7 +28,7 @@ describe Issues::UpdateService, :mailer do describe 'execute' do def find_note(starting_with) issue.notes.find do |note| - note && note.note.start_with?(starting_with) + note&.note&.start_with?(starting_with) end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index f566d235787..ca2caba00a5 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -29,7 +29,7 @@ describe MergeRequests::UpdateService, :mailer do describe 'execute' do def find_note(starting_with) @merge_request.notes.find do |note| - note && note.note.start_with?(starting_with) + note&.note&.start_with?(starting_with) end end diff --git a/spec/support/import_export/export_file_helper.rb b/spec/support/import_export/export_file_helper.rb index 388b88f0331..5ae5d629b47 100644 --- a/spec/support/import_export/export_file_helper.rb +++ b/spec/support/import_export/export_file_helper.rb @@ -91,7 +91,7 @@ module ExportFileHelper loop do object_with_parent = deep_find_with_parent(sensitive_word, project_hash) - return unless object_with_parent && object_with_parent.object + return unless object_with_parent&.object if is_safe_hash?(object_with_parent.parent, sensitive_word) # It's in the safe list, remove hash and keep looking diff --git a/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb b/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb index 439c068471b..e6ab54521de 100644 --- a/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb @@ -6,7 +6,7 @@ shared_examples 'issuable quick actions' do # issuable state needs to be changed before # the quick action is executed. def call_before_action - before_action.call if before_action + before_action&.call end def skip_access_check |