diff options
32 files changed, 100 insertions, 104 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 1b2e3010ea0..70e11be29cf 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -965,5 +965,8 @@ Style/ColonMethodCall: Style/CommentAnnotation: Enabled: false +Style/ConditionalAssignment: + Enabled: true + Style/DoubleNegation: Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 72930fa51c1..78fceb74881 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -38,13 +38,6 @@ RSpec/SingleArgumentMessageChain: Exclude: - 'spec/requests/api/internal_spec.rb' -# Offense count: 32 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, SupportedStyles, SingleLineConditionsOnly. -# SupportedStyles: assign_to_condition, assign_inside_condition -Style/ConditionalAssignment: - Enabled: false - # Offense count: 6 # Cop supports --auto-correct. Style/EachWithObject: diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index 88d180fcc2e..a6cf2b274a2 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -101,12 +101,12 @@ module CreatesCommit # TODO: We should really clean this up def set_commit_variables - if can?(current_user, :push_code, @project) + @mr_source_project = if can?(current_user, :push_code, @project) # Edit file in this project - @mr_source_project = @project + @project else # Merge request from fork to this project - @mr_source_project = current_user.fork_of(@project) + current_user.fork_of(@project) end # Merge request to this project diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index 216c158e41e..0305dafe8da 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -76,10 +76,10 @@ class Projects::GitHttpClientController < Projects::ApplicationController return @project if defined?(@project) project_id, _ = project_id_with_suffix - if project_id.blank? - @project = nil + @project = if project_id.blank? + nil else - @project = Project.find_by_full_path("#{params[:namespace_id]}/#{project_id}") + Project.find_by_full_path("#{params[:namespace_id]}/#{project_id}") end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index c0c71c11dc2..ff77c2b5e01 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -381,13 +381,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def merge_widget_refresh - if merge_request.merge_when_build_succeeds - @status = :merge_when_build_succeeds + @status = if merge_request.merge_when_build_succeeds + :merge_when_build_succeeds else # Only MRs that can be merged end in this action # MR can be already picked up for merge / merged already or can be waiting for worker to be picked up # in last case it does not have any special status. Possible error is handled inside widget js function - @status = :success + :success end render 'merge' diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 93a180b9036..529a8c9b4b4 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -15,10 +15,10 @@ class SessionsController < Devise::SessionsController def new set_minimum_password_length - if Gitlab.config.ldap.enabled - @ldap_servers = Gitlab::LDAP::Config.servers + @ldap_servers = if Gitlab.config.ldap.enabled + Gitlab::LDAP::Config.servers else - @ldap_servers = [] + [] end super diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index 4bd8c83081a..3279cd4a941 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -28,10 +28,10 @@ class NotesFinder private def init_collection - if @params[:target_id] - @notes = on_target(@params[:target_type], @params[:target_id]) + @notes = if @params[:target_id] + on_target(@params[:target_type], @params[:target_id]) else - @notes = notes_of_any_type + notes_of_any_type end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 6db813d4a02..ed4be180f04 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -69,10 +69,10 @@ module ApplicationHelper end def avatar_icon(user_or_email = nil, size = nil, scale = 2) - if user_or_email.is_a?(User) - user = user_or_email + user = if user_or_email.is_a?(User) + user_or_email else - user = User.find_by_any_email(user_or_email.try(:downcase)) + User.find_by_any_email(user_or_email.try(:downcase)) end if user diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 311a70725ab..6cba11f1b63 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -153,15 +153,15 @@ module BlobHelper # Because we are opionated we set the cache headers ourselves. response.cache_control[:public] = @project.public? - if @ref && @commit && @ref == @commit.id + response.cache_control[:max_age] = if @ref && @commit && @ref == @commit.id # This is a link to a commit by its commit SHA. That means that the blob # is immutable. The only reason to invalidate the cache is if the commit # was deleted or if the user lost access to the repository. - response.cache_control[:max_age] = Blob::CACHE_TIME_IMMUTABLE + Blob::CACHE_TIME_IMMUTABLE else # A branch or tag points at this blob. That means that the expected blob # value may change over time. - response.cache_control[:max_age] = Blob::CACHE_TIME + Blob::CACHE_TIME end response.etag = @blob.id diff --git a/app/mailers/repository_check_mailer.rb b/app/mailers/repository_check_mailer.rb index 21db2fe04a0..11a0c0b7700 100644 --- a/app/mailers/repository_check_mailer.rb +++ b/app/mailers/repository_check_mailer.rb @@ -1,9 +1,9 @@ class RepositoryCheckMailer < BaseMailer def notify(failed_count) - if failed_count == 1 - @message = "One project failed its last repository check" + @message = if failed_count == 1 + "One project failed its last repository check" else - @message = "#{failed_count} projects failed their last repository check" + "#{failed_count} projects failed their last repository check" end mail( diff --git a/app/models/commit.rb b/app/models/commit.rb index 7afc8f4add8..feb35bb3e92 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -122,10 +122,10 @@ class Commit def full_title return @full_title if @full_title - if safe_message.blank? - @full_title = no_commit_message + @full_title = if safe_message.blank? + no_commit_message else - @full_title = safe_message.split("\n", 2).first + safe_message.split("\n", 2).first end end diff --git a/app/models/concerns/case_sensitivity.rb b/app/models/concerns/case_sensitivity.rb index fe0cea8465f..2cfb6127c96 100644 --- a/app/models/concerns/case_sensitivity.rb +++ b/app/models/concerns/case_sensitivity.rb @@ -13,10 +13,10 @@ module CaseSensitivity params.each do |key, value| column = ActiveRecord::Base.connection.quote_table_name(key) - if cast_lower - condition = "LOWER(#{column}) = LOWER(:value)" + condition = if cast_lower + "LOWER(#{column}) = LOWER(:value)" else - condition = "#{column} = :value" + "#{column} = :value" end criteria = criteria.where(condition, value: value) diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb index 7edb0acd56c..fdcb4f3ba64 100644 --- a/app/models/concerns/sortable.rb +++ b/app/models/concerns/sortable.rb @@ -46,10 +46,10 @@ module Sortable where("label_links.target_id = #{target_column}"). reorder(nil) - if target_type_column - query = query.where("label_links.target_type = #{target_type_column}") + query = if target_type_column + query.where("label_links.target_type = #{target_type_column}") else - query = query.where(label_links: { target_type: target_type }) + query.where(label_links: { target_type: target_type }) end query = query.where.not(title: excluded_labels) if excluded_labels.present? diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb index b16ecc424dd..b859d40da24 100644 --- a/app/models/network/graph.rb +++ b/app/models/network/graph.rb @@ -188,10 +188,10 @@ module Network end # and mark it as reserved - if parent_time.nil? - min_time = leaves.first.time + min_time = if parent_time.nil? + leaves.first.time else - min_time = parent_time + 1 + parent_time + 1 end max_time = leaves.last.time diff --git a/app/models/project.rb b/app/models/project.rb index 0da21be8351..a0bc82af7d5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -453,12 +453,12 @@ class Project < ActiveRecord::Base end def add_import_job - if forked? - job_id = RepositoryForkWorker.perform_async(id, forked_from_project.repository_storage_path, + job_id = if forked? + RepositoryForkWorker.perform_async(id, forked_from_project.repository_storage_path, forked_from_project.path_with_namespace, self.namespace.full_path) else - job_id = RepositoryImportWorker.perform_async(self.id) + RepositoryImportWorker.perform_async(self.id) end if job_id diff --git a/app/models/project_services/pushover_service.rb b/app/models/project_services/pushover_service.rb index f623bf9851b..cd185f6e120 100644 --- a/app/models/project_services/pushover_service.rb +++ b/app/models/project_services/pushover_service.rb @@ -72,12 +72,12 @@ class PushoverService < Service before = data[:before] after = data[:after] - if Gitlab::Git.blank_ref?(before) - message = "#{data[:user_name]} pushed new branch \"#{ref}\"." + message = if Gitlab::Git.blank_ref?(before) + "#{data[:user_name]} pushed new branch \"#{ref}\"." elsif Gitlab::Git.blank_ref?(after) - message = "#{data[:user_name]} deleted branch \"#{ref}\"." + "#{data[:user_name]} deleted branch \"#{ref}\"." else - message = "#{data[:user_name]} push to branch \"#{ref}\"." + "#{data[:user_name]} push to branch \"#{ref}\"." end if data[:total_commits_count] > 0 diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 71b4f8605a4..3e362bb2ca9 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -408,11 +408,11 @@ module SystemNoteService # Initial scope should be system notes of this noteable type notes = Note.system.where(noteable_type: noteable.class) - if noteable.is_a?(Commit) + notes = if noteable.is_a?(Commit) # Commits have non-integer IDs, so they're stored in `commit_id` - notes = notes.where(commit_id: noteable.id) + notes.where(commit_id: noteable.id) else - notes = notes.where(noteable_id: noteable.id) + notes.where(noteable_id: noteable.id) end notes_for_mentioner(mentioner, noteable, notes).exists? diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 680deee6d46..ecae5453736 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -14,10 +14,10 @@ class Settings < Settingslogic end def build_gitlab_ci_url - if on_standard_port?(gitlab) - custom_port = nil + custom_port = if on_standard_port?(gitlab) + nil else - custom_port = ":#{gitlab.port}" + ":#{gitlab.port}" end [gitlab.protocol, "://", @@ -160,10 +160,10 @@ if github_settings github_settings["args"] ||= Settingslogic.new({}) - if github_settings["url"].include?(github_default_url) - github_settings["args"]["client_options"] = OmniAuth::Strategies::GitHub.default_options[:client_options] + github_settings["args"]["client_options"] = if github_settings["url"].include?(github_default_url) + OmniAuth::Strategies::GitHub.default_options[:client_options] else - github_settings["args"]["client_options"] = { + { "site" => File.join(github_settings["url"], "api/v3"), "authorize_url" => File.join(github_settings["url"], "login/oauth/authorize"), "token_url" => File.join(github_settings["url"], "login/oauth/access_token") diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index 3b15ff6566f..1c42ae3363d 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -160,10 +160,10 @@ module Banzai data = data_attributes_for(link_content || match, project, object, link: !!link_content) - if matches.names.include?("url") && matches[:url] - url = matches[:url] + url = if matches.names.include?("url") && matches[:url] + matches[:url] else - url = url_for_object_cached(object, project) + url_for_object_cached(object, project) end content = link_content || object_link_text(object, matches) diff --git a/lib/banzai/filter/gollum_tags_filter.rb b/lib/banzai/filter/gollum_tags_filter.rb index d08267a9d6c..35d21733967 100644 --- a/lib/banzai/filter/gollum_tags_filter.rb +++ b/lib/banzai/filter/gollum_tags_filter.rb @@ -149,10 +149,10 @@ module Banzai name, reference = *parts.compact.map(&:strip) end - if url?(reference) - href = reference + href = if url?(reference) + reference else - href = ::File.join(project_wiki_base_path, reference) + ::File.join(project_wiki_base_path, reference) end content_tag(:a, name || reference, href: href, class: 'gfm') diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb index fd6b9704132..e5082895e82 100644 --- a/lib/banzai/filter/issue_reference_filter.rb +++ b/lib/banzai/filter/issue_reference_filter.rb @@ -39,10 +39,10 @@ module Banzai projects_per_reference.each do |path, project| issue_ids = references_per_project[path] - if project.default_issues_tracker? - issues = project.issues.where(iid: issue_ids.to_a) + issues = if project.default_issues_tracker? + project.issues.where(iid: issue_ids.to_a) else - issues = issue_ids.map { |id| ExternalIssue.new(id, project) } + issue_ids.map { |id| ExternalIssue.new(id, project) } end issues.each do |issue| diff --git a/lib/gitlab/award_emoji.rb b/lib/gitlab/award_emoji.rb index 39b43ab5489..fcb3542d181 100644 --- a/lib/gitlab/award_emoji.rb +++ b/lib/gitlab/award_emoji.rb @@ -69,10 +69,10 @@ module Gitlab end JSON.parse(File.read(path)).map do |hash| - if digest - fname = "#{hash['unicode']}-#{hash['digest']}" + fname = if digest + "#{hash['unicode']}-#{hash['digest']}" else - fname = hash['unicode'] + hash['unicode'] end { name: hash['name'], path: File.join(base, prefix, "#{fname}.png") } diff --git a/lib/gitlab/conflict/file.rb b/lib/gitlab/conflict/file.rb index c843315782d..fd43a224b3d 100644 --- a/lib/gitlab/conflict/file.rb +++ b/lib/gitlab/conflict/file.rb @@ -91,10 +91,10 @@ module Gitlab our_highlight = Gitlab::Highlight.highlight(our_path, our_file, repository: repository).lines lines.each do |line| - if line.type == 'old' - line.rich_text = their_highlight[line.old_line - 1].try(:html_safe) + line.rich_text = if line.type == 'old' + their_highlight[line.old_line - 1].try(:html_safe) else - line.rich_text = our_highlight[line.new_line - 1].try(:html_safe) + our_highlight[line.new_line - 1].try(:html_safe) end end end diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb index ecf62dead35..81c93f1aab5 100644 --- a/lib/gitlab/diff/position.rb +++ b/lib/gitlab/diff/position.rb @@ -140,10 +140,10 @@ module Gitlab def find_diff_file(repository) # We're at the initial commit, so just get that as we can't compare to anything. - if Gitlab::Git.blank_ref?(start_sha) - compare = Gitlab::Git::Commit.find(repository.raw_repository, head_sha) + compare = if Gitlab::Git.blank_ref?(start_sha) + Gitlab::Git::Commit.find(repository.raw_repository, head_sha) else - compare = Gitlab::Git::Compare.new( + Gitlab::Git::Compare.new( repository.raw_repository, start_sha, head_sha diff --git a/lib/gitlab/email/reply_parser.rb b/lib/gitlab/email/reply_parser.rb index 8c8dd1b9cef..4ddfd78130e 100644 --- a/lib/gitlab/email/reply_parser.rb +++ b/lib/gitlab/email/reply_parser.rb @@ -31,10 +31,10 @@ module Gitlab private def select_body(message) - if message.multipart? - part = message.text_part || message.html_part || message + part = if message.multipart? + message.text_part || message.html_part || message else - part = message + message end decoded = fix_charset(part) diff --git a/lib/gitlab/metrics/instrumentation.rb b/lib/gitlab/metrics/instrumentation.rb index 4b7a791e497..9a07abfed06 100644 --- a/lib/gitlab/metrics/instrumentation.rb +++ b/lib/gitlab/metrics/instrumentation.rb @@ -143,10 +143,10 @@ module Gitlab # signature this would break things. As a result we'll make sure the # generated method _only_ accepts regular arguments if the underlying # method also accepts them. - if method.arity == 0 - args_signature = '' + args_signature = if method.arity == 0 + '' else - args_signature = '*args' + '*args' end proxy_module.class_eval <<-EOF, __FILE__, __LINE__ + 1 diff --git a/lib/gitlab/saml/user.rb b/lib/gitlab/saml/user.rb index f253dc7477e..abde05d9b68 100644 --- a/lib/gitlab/saml/user.rb +++ b/lib/gitlab/saml/user.rb @@ -28,10 +28,10 @@ module Gitlab if external_users_enabled? && @user # Check if there is overlap between the user's groups and the external groups # setting then set user as external or internal. - if (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty? - @user.external = false + @user.external = if (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty? + false else - @user.external = true + true end end diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index c9c65f76f4b..a1a6929103b 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -56,10 +56,10 @@ module Gitlab def issues issues = IssuesFinder.new(current_user).execute.where(project_id: project_ids_relation) - if query =~ /#(\d+)\z/ - issues = issues.where(iid: $1) + issues = if query =~ /#(\d+)\z/ + issues.where(iid: $1) else - issues = issues.full_search(query) + issues.full_search(query) end issues.order('updated_at DESC') @@ -73,10 +73,10 @@ module Gitlab def merge_requests merge_requests = MergeRequestsFinder.new(current_user).execute.in_projects(project_ids_relation) - if query =~ /[#!](\d+)\z/ - merge_requests = merge_requests.where(iid: $1) + merge_requests = if query =~ /[#!](\d+)\z/ + merge_requests.where(iid: $1) else - merge_requests = merge_requests.full_search(query) + merge_requests.full_search(query) end merge_requests.order('updated_at DESC') end diff --git a/lib/gitlab/sherlock/query.rb b/lib/gitlab/sherlock/query.rb index 4917c4ae2ac..086fcddf721 100644 --- a/lib/gitlab/sherlock/query.rb +++ b/lib/gitlab/sherlock/query.rb @@ -94,10 +94,10 @@ module Gitlab private def raw_explain(query) - if Gitlab::Database.postgresql? - explain = "EXPLAIN ANALYZE #{query};" + explain = if Gitlab::Database.postgresql? + "EXPLAIN ANALYZE #{query};" else - explain = "EXPLAIN #{query};" + "EXPLAIN #{query};" end ActiveRecord::Base.connection.execute(explain) diff --git a/spec/features/issuables/issuable_list_spec.rb b/spec/features/issuables/issuable_list_spec.rb index 0bf7977fb02..2867fcf8819 100644 --- a/spec/features/issuables/issuable_list_spec.rb +++ b/spec/features/issuables/issuable_list_spec.rb @@ -47,10 +47,10 @@ describe 'issuable list', feature: true do def create_issuables(issuable_type) 3.times do - if issuable_type == :issue - issuable = create(:issue, project: project, author: user) + issuable = if issuable_type == :issue + create(:issue, project: project, author: user) else - issuable = create(:merge_request, title: FFaker::Lorem.sentence, source_project: project, source_branch: FFaker::Name.name) + create(:merge_request, title: FFaker::Lorem.sentence, source_project: project, source_branch: FFaker::Name.name) end 2.times do diff --git a/spec/support/issuables_list_metadata_shared_examples.rb b/spec/support/issuables_list_metadata_shared_examples.rb index dac94dfc31e..c61f3e05cea 100644 --- a/spec/support/issuables_list_metadata_shared_examples.rb +++ b/spec/support/issuables_list_metadata_shared_examples.rb @@ -3,10 +3,10 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| @issuable_ids = [] 2.times do - if issuable_type == :issue - issuable = create(issuable_type, project: project) + issuable = if issuable_type == :issue + create(issuable_type, project: project) else - issuable = create(issuable_type, title: FFaker::Lorem.sentence, source_project: project, source_branch: FFaker::Name.name) + create(issuable_type, title: FFaker::Lorem.sentence, source_project: project, source_branch: FFaker::Name.name) end @issuable_ids << issuable.id diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb index ad1eed5b369..b94ffa30736 100644 --- a/spec/support/login_helpers.rb +++ b/spec/support/login_helpers.rb @@ -15,10 +15,10 @@ module LoginHelpers # user = create(:user) # login_as(user) def login_as(user_or_role) - if user_or_role.kind_of?(User) - @user = user_or_role + @user = if user_or_role.kind_of?(User) + user_or_role else - @user = create(user_or_role) + create(user_or_role) end login_with(@user) |