From 56258237ddf1826cd7145ffe42d9e8cc21b56aed Mon Sep 17 00:00:00 2001
From: Divyam Tayal <divyamtayal18@gmail.com>
Date: Fri, 1 Nov 2024 17:12:29 +0000
Subject: [PATCH] Fix Style/IfUnlessModifier offences
Changelog: other
---
.rubocop_todo/style/if_unless_modifier.yml | 10 --------
app/models/appearance.rb | 4 +---
.../application_setting_implementation.rb | 18 ++++++++++----
app/models/bulk_imports/entity.rb | 8 ++-----
app/models/ci/application_record.rb | 4 +---
app/models/ci/build.rb | 12 ++++------
app/models/ci/build_trace_chunk.rb | 4 +---
app/models/ci/job_artifact.rb | 4 +---
app/models/ci/pending_build.rb | 4 +---
app/models/ci/pipeline.rb | 12 +++-------
app/models/ci/runner.rb | 24 +++++--------------
11 files changed, 34 insertions(+), 70 deletions(-)
diff --git a/.rubocop_todo/style/if_unless_modifier.yml b/.rubocop_todo/style/if_unless_modifier.yml
index 6fb6b73e09330..83c07bed09125 100644
--- a/.rubocop_todo/style/if_unless_modifier.yml
+++ b/.rubocop_todo/style/if_unless_modifier.yml
@@ -34,16 +34,6 @@ Style/IfUnlessModifier:
- 'app/mailers/emails/members.rb'
- 'app/mailers/emails/merge_requests.rb'
- 'app/models/ability.rb'
- - 'app/models/appearance.rb'
- - 'app/models/application_setting_implementation.rb'
- - 'app/models/bulk_imports/entity.rb'
- - 'app/models/ci/application_record.rb'
- - 'app/models/ci/build.rb'
- - 'app/models/ci/build_trace_chunk.rb'
- - 'app/models/ci/job_artifact.rb'
- - 'app/models/ci/pending_build.rb'
- - 'app/models/ci/pipeline.rb'
- - 'app/models/ci/runner.rb'
- 'app/models/concerns/ci/artifactable.rb'
- 'app/models/concerns/deprecated_assignee.rb'
- 'app/models/concerns/group_descendant.rb'
diff --git a/app/models/appearance.rb b/app/models/appearance.rb
index 35286a9ec94f6..2352f6f736c0f 100644
--- a/app/models/appearance.rb
+++ b/app/models/appearance.rb
@@ -69,9 +69,7 @@ def self.current_without_cache
end
def single_appearance_row
- if self.class.any?
- errors.add(:base, _('Only 1 appearances row can exist'))
- end
+ errors.add(:base, _('Only 1 appearances row can exist')) if self.class.any?
end
def pwa_icon_path_scaled(width)
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index dd972377e8d85..fe1c75366415f 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -537,7 +537,9 @@ def usage_ping_can_be_configured?
def usage_ping_features_enabled
return false unless usage_ping_enabled? && super
- return include_optional_metrics_in_service_ping if Gitlab.ee? && respond_to?(:include_optional_metrics_in_service_ping)
+ if Gitlab.ee? && respond_to?(:include_optional_metrics_in_service_ping)
+ return include_optional_metrics_in_service_ping
+ end
true
end
@@ -699,14 +701,20 @@ def check_repository_storages_weighted
repository_storages_weighted.each do |key, val|
next unless val.present?
- errors.add(:repository_storages_weighted, _("value for '%{storage}' must be an integer") % { storage: key }) unless val.is_a?(Integer)
- errors.add(:repository_storages_weighted, _("value for '%{storage}' must be between 0 and 100") % { storage: key }) unless val.between?(0, 100)
+ unless val.is_a?(Integer)
+ errors.add(:repository_storages_weighted, _("value for '%{storage}' must be an integer") % { storage: key })
+ end
+
+ unless val.between?(0, 100)
+ errors.add(:repository_storages_weighted, _("value for '%{storage}' must be between 0 and 100") % { storage: key })
+ end
end
end
def check_valid_runner_registrars
- valid = valid_runner_registrar_combinations.include?(valid_runner_registrars)
- errors.add(:valid_runner_registrars, _("%{value} is not included in the list") % { value: valid_runner_registrars }) unless valid
+ return if valid_runner_registrar_combinations.include?(valid_runner_registrars)
+
+ errors.add(:valid_runner_registrars, _("%{value} is not included in the list") % { value: valid_runner_registrars })
end
def valid_runner_registrar_combinations
diff --git a/app/models/bulk_imports/entity.rb b/app/models/bulk_imports/entity.rb
index 0c5b3ec0c66f5..10f55b5454e55 100644
--- a/app/models/bulk_imports/entity.rb
+++ b/app/models/bulk_imports/entity.rb
@@ -175,9 +175,7 @@ def relation_download_url_path(relation, batch_number = nil)
url = File.join(export_relations_url_path_base, 'download')
params = { relation: relation }
- if batch_number && bulk_import.supports_batched_export?
- params.merge!(batched: true, batch_number: batch_number)
- end
+ params.merge!(batched: true, batch_number: batch_number) if batch_number && bulk_import.supports_batched_export?
Gitlab::Utils.add_url_parameters(url, params)
end
@@ -241,9 +239,7 @@ def propagate_cancel
private
def validate_parent_is_a_group
- unless parent.group_entity?
- errors.add(:parent, s_('BulkImport|must be a group'))
- end
+ errors.add(:parent, s_('BulkImport|must be a group')) unless parent.group_entity?
end
def validate_imported_entity_type
diff --git a/app/models/ci/application_record.rb b/app/models/ci/application_record.rb
index 52f02bfb2fd81..f0d9abb656236 100644
--- a/app/models/ci/application_record.rb
+++ b/app/models/ci/application_record.rb
@@ -4,9 +4,7 @@ module Ci
class ApplicationRecord < ::ApplicationRecord
self.abstract_class = true
- if Gitlab::Database.has_config?(:ci)
- connects_to database: { writing: :ci, reading: :ci }
- end
+ connects_to database: { writing: :ci, reading: :ci } if Gitlab::Database.has_config?(:ci)
def self.table_name_prefix
'ci_'
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 774218a0ab9c3..b425a079408ea 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -828,7 +828,9 @@ def artifact_access_setting_in_config
artifacts_public = options.dig(:artifacts, :public)
artifacts_access = options.dig(:artifacts, :access)
- raise ArgumentError, 'artifacts:public and artifacts:access are mutually exclusive' if !artifacts_public.nil? && !artifacts_access.nil?
+ if !artifacts_public.nil? && !artifacts_access.nil?
+ raise ArgumentError, 'artifacts:public and artifacts:access are mutually exclusive'
+ end
return :public if artifacts_public == true || artifacts_access == 'all'
return :private if artifacts_public == false || artifacts_access == 'developer'
@@ -867,9 +869,7 @@ def artifacts_expire_in
def artifacts_expire_in=(value)
self.artifacts_expire_at =
- if value
- ChronicDuration.parse(value)&.seconds&.from_now
- end
+ (ChronicDuration.parse(value)&.seconds&.from_now if value)
end
def has_expired_locked_archive_artifacts?
@@ -997,9 +997,7 @@ def hide_secrets(data, metrics = ::Gitlab::Ci::Trace::Metrics.new)
Gitlab::Ci::MaskSecret.mask!(trace, project.runners_token) if project
Gitlab::Ci::MaskSecret.mask!(trace, token) if token
- if trace != data
- metrics.increment_trace_operation(operation: :mutated)
- end
+ metrics.increment_trace_operation(operation: :mutated) if trace != data
end
end
diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb
index 10b19a83c7135..54cb217315480 100644
--- a/app/models/ci/build_trace_chunk.rb
+++ b/app/models/ci/build_trace_chunk.rb
@@ -275,9 +275,7 @@ def unsafe_set_data!(value)
def unsafe_append_data!(value, offset)
new_size = value.bytesize + offset
- if new_size > CHUNK_SIZE
- raise ArgumentError, 'New data size exceeds chunk size'
- end
+ raise ArgumentError, 'New data size exceeds chunk size' if new_size > CHUNK_SIZE
current_store.append_data(self, value, offset).then do |stored|
metrics.increment_trace_operation(operation: :appended)
diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb
index f50468c1678f3..9cbb9f6fa47d3 100644
--- a/app/models/ci/job_artifact.rb
+++ b/app/models/ci/job_artifact.rb
@@ -179,9 +179,7 @@ def expire_in
def expire_in=(value)
self.expire_at =
- if value
- ::Gitlab::Ci::Build::DurationParser.new(value).seconds_from_now
- end
+ (::Gitlab::Ci::Build::DurationParser.new(value).seconds_from_now if value)
end
def stored?
diff --git a/app/models/ci/pending_build.rb b/app/models/ci/pending_build.rb
index 762f6d5917d3d..316a780a0a430 100644
--- a/app/models/ci/pending_build.rb
+++ b/app/models/ci/pending_build.rb
@@ -58,9 +58,7 @@ def args_from_build(build)
instance_runners_enabled: shared_runners_enabled?(project)
}
- if group_runners_enabled?(project)
- args.store(:namespace_traversal_ids, project.namespace.traversal_ids)
- end
+ args.store(:namespace_traversal_ids, project.namespace.traversal_ids) if group_runners_enabled?(project)
args
end
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 13c9810002614..a05cb9877f305 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -636,9 +636,7 @@ def triggered_pipelines_with_preloads
end
def valid_commit_sha
- if Gitlab::Git.blank_ref?(self.sha)
- self.errors.add(:sha, "can't be 00000000 (branch removal)")
- end
+ self.errors.add(:sha, "can't be 00000000 (branch removal)") if Gitlab::Git.blank_ref?(self.sha)
end
def git_author_name
@@ -751,9 +749,7 @@ def retried
def coverage
coverage_array = latest_statuses.map(&:coverage).compact
- if coverage_array.size >= 1
- coverage_array.sum / coverage_array.size
- end
+ coverage_array.sum / coverage_array.size if coverage_array.size >= 1
end
def update_builds_coverage
@@ -1396,9 +1392,7 @@ def has_test_reports?
def age_in_minutes
return 0 unless persisted?
- unless has_attribute?(:created_at)
- raise ArgumentError, 'pipeline not fully loaded'
- end
+ raise ArgumentError, 'pipeline not fully loaded' unless has_attribute?(:created_at)
return 0 unless created_at
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 19ecc411560d9..6763afdb3a15c 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -582,39 +582,27 @@ def tag_constraints
end
def no_sharding_key_id
- if sharding_key_id
- errors.add(:runner, 'cannot have sharding_key_id assigned')
- end
+ errors.add(:runner, 'cannot have sharding_key_id assigned') if sharding_key_id
end
def no_projects
- if runner_projects.any?
- errors.add(:runner, 'cannot have projects assigned')
- end
+ errors.add(:runner, 'cannot have projects assigned') if runner_projects.any?
end
def no_groups
- if runner_namespaces.any?
- errors.add(:runner, 'cannot have groups assigned')
- end
+ errors.add(:runner, 'cannot have groups assigned') if runner_namespaces.any?
end
def any_project
- unless runner_projects.any?
- errors.add(:runner, 'needs to be assigned to at least one project')
- end
+ errors.add(:runner, 'needs to be assigned to at least one project') unless runner_projects.any?
end
def exactly_one_group
- unless runner_namespaces.size == 1
- errors.add(:runner, 'needs to be assigned to exactly one group')
- end
+ errors.add(:runner, 'needs to be assigned to exactly one group') unless runner_namespaces.size == 1
end
def no_allowed_plan_ids
- unless allowed_plan_ids.empty?
- errors.add(:runner, 'cannot have allowed plans assigned')
- end
+ errors.add(:runner, 'cannot have allowed plans assigned') unless allowed_plan_ids.empty?
end
def partition_id_prefix_in_16_bit_encode
--
GitLab