diff options
author | 🙈 jacopo beschi 🙉 <intrip@gmail.com> | 2018-01-11 16:34:01 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2018-01-11 16:34:01 +0000 |
commit | 729f05f0e3c4835c91e20ccd1ddb630eb7ef4379 (patch) | |
tree | 3d543c84930ce20b7c91d48295bf3e106c788cde | |
parent | ca8394e8e4931c85b5f2a2bde1951bcb9419e220 (diff) | |
download | gitlab-ce-729f05f0e3c4835c91e20ccd1ddb630eb7ef4379.tar.gz |
Adds Rubocop rule for line break around conditionals
109 files changed, 673 insertions, 0 deletions
diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb index 38b808cdc31..4b01904f2a1 100644 --- a/app/controllers/admin/runners_controller.rb +++ b/app/controllers/admin/runners_controller.rb @@ -65,6 +65,7 @@ class Admin::RunnersController < Admin::ApplicationController else Project.all end + @projects = @projects.where.not(id: runner.projects.select(:id)) if runner.projects.any? @projects = @projects.page(params[:page]).per(30) end diff --git a/app/controllers/concerns/group_tree.rb b/app/controllers/concerns/group_tree.rb index b10147835f3..b569029283f 100644 --- a/app/controllers/concerns/group_tree.rb +++ b/app/controllers/concerns/group_tree.rb @@ -8,6 +8,7 @@ module GroupTree # Only show root groups if no parent-id is given groups.where(parent_id: params[:parent_id]) end + @groups = @groups.with_selects_for_list(archived: params[:archived]) .sort(@sort = params[:sort]) .page(params[:page]) diff --git a/app/controllers/concerns/routable_actions.rb b/app/controllers/concerns/routable_actions.rb index 4199da9cdf5..f745deb083c 100644 --- a/app/controllers/concerns/routable_actions.rb +++ b/app/controllers/concerns/routable_actions.rb @@ -32,6 +32,7 @@ module RoutableActions if canonical_path.casecmp(requested_full_path) != 0 flash[:notice] = "#{routable.class.to_s.titleize} '#{requested_full_path}' was moved to '#{canonical_path}'. Please update any links and bookmarks that may still have the old path." end + redirect_to build_canonical_path(routable) end end diff --git a/app/controllers/metrics_controller.rb b/app/controllers/metrics_controller.rb index d81ad135198..33b682d2859 100644 --- a/app/controllers/metrics_controller.rb +++ b/app/controllers/metrics_controller.rb @@ -12,6 +12,7 @@ class MetricsController < ActionController::Base ) "# Metrics are disabled, see: #{help_page}\n" end + render text: response, content_type: 'text/plain; version=0.0.4' end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index e3c18cba1dd..689d2e3db22 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -83,6 +83,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController if ticket handle_service_ticket oauth['provider'], ticket end + handle_omniauth end @@ -90,6 +91,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController if params['sid'] handle_service_ticket oauth['provider'], params['sid'] end + handle_omniauth end @@ -124,6 +126,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController # Only allow properly saved users to login. if @user.persisted? && @user.valid? log_audit_event(@user, with: oauth['provider']) + if @user.two_factor_enabled? params[:remember_me] = '1' if remember_me? prompt_for_two_factor(@user) diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index d838b8dc29e..35e67730a27 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -150,6 +150,7 @@ class Projects::BlobController < Projects::ApplicationController if params[:file].present? params[:file_name] = params[:file].original_filename end + File.join(@path, params[:file_name]) elsif params[:file_path].present? params[:file_path] diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb index cf8829ba95b..e06dda1baa4 100644 --- a/app/controllers/projects/deploy_keys_controller.rb +++ b/app/controllers/projects/deploy_keys_controller.rb @@ -27,6 +27,7 @@ class Projects::DeployKeysController < Projects::ApplicationController unless @key.valid? && @project.deploy_keys << @key flash[:alert] = @key.errors.full_messages.join(', ').html_safe end + redirect_to_repository_settings(@project) end diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index 85d35900c71..6f51e7b9b40 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -21,6 +21,7 @@ class Projects::HooksController < Projects::ApplicationController @hooks = @project.hooks.select(&:persisted?) flash[:alert] = @hook.errors.full_messages.join.html_safe end + redirect_to project_settings_integrations_path(@project) end diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index dc524b790a0..3d2926d5d75 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -48,6 +48,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap else [] end + @diff_notes_disabled = true @environment = @merge_request.environments_for(current_user).last diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 6f229b08c0c..e6e2b219e6a 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -203,6 +203,7 @@ class ProjectsController < Projects::ApplicationController else flash[:alert] = _("Project export could not be deleted.") end + redirect_to(edit_project_path(@project)) end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index d79108c88fb..c73306a6b66 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -28,6 +28,7 @@ class SessionsController < Devise::SessionsController resource.update_attributes(reset_password_token: nil, reset_password_sent_at: nil) end + # hide the signed-in notification flash[:notice] = nil log_audit_event(current_user, resource, with: authentication_method) diff --git a/app/finders/group_descendants_finder.rb b/app/finders/group_descendants_finder.rb index 1a5f6063437..58570a580f1 100644 --- a/app/finders/group_descendants_finder.rb +++ b/app/finders/group_descendants_finder.rb @@ -63,6 +63,7 @@ class GroupDescendantsFinder groups_table = Group.arel_table visible_to_user = groups_table[:visibility_level] .in(Gitlab::VisibilityLevel.levels_for_user(current_user)) + if current_user authorized_groups = GroupsFinder.new(current_user, all_available: false) @@ -115,6 +116,7 @@ class GroupDescendantsFinder else direct_child_groups end + groups.with_selects_for_list(archived: params[:archived]).order_by(sort) end @@ -140,6 +142,7 @@ class GroupDescendantsFinder else direct_child_projects end + projects.with_route.order_by(sort) end diff --git a/app/finders/group_projects_finder.rb b/app/finders/group_projects_finder.rb index 6e8733bb49c..f2d3b90b8e2 100644 --- a/app/finders/group_projects_finder.rb +++ b/app/finders/group_projects_finder.rb @@ -34,6 +34,7 @@ class GroupProjectsFinder < ProjectsFinder else collection_without_user end + union(projects) end diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb index f78d41a0448..2fe1927a189 100644 --- a/app/helpers/markup_helper.rb +++ b/app/helpers/markup_helper.rb @@ -203,6 +203,7 @@ module MarkupHelper node.content = node.content.truncate(num_remaining) truncated = true end + content_length += node.content.length end diff --git a/app/helpers/nav_helper.rb b/app/helpers/nav_helper.rb index 8ada746b244..680ea96a556 100644 --- a/app/helpers/nav_helper.rb +++ b/app/helpers/nav_helper.rb @@ -12,6 +12,7 @@ module NavHelper current_path?('projects/merge_requests/conflicts#show') || current_path?('issues#show') || current_path?('milestones#show') + if cookies[:collapsed_gutter] == 'true' %w[page-gutter right-sidebar-collapsed] else diff --git a/app/helpers/snippets_helper.rb b/app/helpers/snippets_helper.rb index b447d4952e7..00e7e4230b9 100644 --- a/app/helpers/snippets_helper.rb +++ b/app/helpers/snippets_helper.rb @@ -89,6 +89,7 @@ module SnippetsHelper snippet_chunk = [lined_content[line_number]] snippet_start_line = line_number end + last_line = line_number end # Add final chunk to chunk array diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb index 40d69e30188..1db9ae3839c 100644 --- a/app/helpers/submodule_helper.rb +++ b/app/helpers/submodule_helper.rb @@ -58,6 +58,7 @@ module SubmoduleHelper url_no_dotgit = url.chomp('.git') return true if url_no_dotgit == [Gitlab.config.gitlab.url, '/', namespace, '/', project].join('') + url_with_dotgit = url_no_dotgit + '.git' url_with_dotgit == Gitlab::Shell.new.url_to_repo([namespace, '/', project].join('')) end diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index 2a7aa299e83..e7c953e749e 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -30,6 +30,7 @@ module TodosHelper else todo.target_reference end + link_to text, todo_target_path(todo), class: 'has-tooltip', title: todo.target.title end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 8ab338d873d..80bda7f22ff 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -418,6 +418,7 @@ class ApplicationSetting < ActiveRecord::Base super(group_full_path) Gitlab::PerformanceBar.expire_allowed_user_ids_cache end + return end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 4251561a0a0..7049f340c9d 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -314,6 +314,7 @@ module Issuable includes = [] includes << :author unless notes.authors_loaded? includes << :award_emoji unless notes.award_emojis_loaded? + if includes.any? notes.includes(includes) else diff --git a/app/models/concerns/loaded_in_group_list.rb b/app/models/concerns/loaded_in_group_list.rb index dcb3b2b5ff3..935e9d10133 100644 --- a/app/models/concerns/loaded_in_group_list.rb +++ b/app/models/concerns/loaded_in_group_list.rb @@ -25,6 +25,7 @@ module LoadedInGroupList base_count = projects.project(Arel.star.count.as('preloaded_project_count')) .where(projects[:namespace_id].eq(namespaces[:id])) + if archived == 'only' base_count.where(projects[:archived].eq(true)) elsif Gitlab::Utils.to_boolean(archived) diff --git a/app/models/label.rb b/app/models/label.rb index b5bfa6ea2dd..7538f2d8718 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -132,6 +132,7 @@ class Label < ActiveRecord::Base else priorities.find_by(project: project) end + priority.try(:priority) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 8fdeddf1ed1..2669d2a6ff3 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -793,6 +793,7 @@ class MergeRequest < ActiveRecord::Base if !include_description && closes_issues_references.present? message << "Closes #{closes_issues_references.to_sentence}" end + message << "#{description}" if include_description && description.present? message << "See merge request #{to_reference(full: true)}" diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb index aec7b01e23a..c351d2012c6 100644 --- a/app/models/network/graph.rb +++ b/app/models/network/graph.rb @@ -224,6 +224,7 @@ module Network space_base = parents.first.space end end + space_base end diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb index 183e098d819..ab5a96209c7 100644 --- a/app/models/notification_recipient.rb +++ b/app/models/notification_recipient.rb @@ -9,6 +9,7 @@ class NotificationRecipient group: nil, skip_read_ability: false ) + unless NotificationSetting.levels.key?(type) || type == :subscription raise ArgumentError, "invalid type: #{type.inspect}" end diff --git a/app/models/project.rb b/app/models/project.rb index 7dc5e980c1b..029f2da2e4e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -633,6 +633,7 @@ class Project < ActiveRecord::Base project_import_data.data ||= {} project_import_data.data = project_import_data.data.merge(data) end + if credentials project_import_data.credentials ||= {} project_import_data.credentials = project_import_data.credentials.merge(credentials) diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index 768f0a7472e..bfe7ac29c18 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -110,6 +110,7 @@ class HipchatService < Service message = "" message << "#{push[:user_name]} " + if Gitlab::Git.blank_ref?(before) message << "pushed new #{ref_type} <a href=\""\ "#{project_url}/commits/#{CGI.escape(ref)}\">#{ref}</a>"\ diff --git a/app/models/repository.rb b/app/models/repository.rb index a84d6a1426b..d27212b2058 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1014,6 +1014,7 @@ class Repository else cache.fetch(key, &block) end + instance_variable_set(ivar, value) rescue Rugged::ReferenceError, Gitlab::Git::Repository::NoRepository # Even if the above `#exists?` check passes these errors might still diff --git a/app/models/service.rb b/app/models/service.rb index 24ba3039707..7f260f7a96b 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -250,6 +250,7 @@ class Service < ActiveRecord::Base teamcity microsoft_teams ] + if Rails.env.development? service_names += %w[mock_ci mock_deployment mock_monitoring] end diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb index 63b85c3de7d..88dfb7a4a90 100644 --- a/app/services/create_deployment_service.rb +++ b/app/services/create_deployment_service.rb @@ -16,6 +16,7 @@ class CreateDeploymentService ActiveRecord::Base.transaction do environment.external_url = expanded_environment_url if expanded_environment_url + environment.fire_state_event(action) return unless environment.save diff --git a/app/workers/pages_worker.rb b/app/workers/pages_worker.rb index 3ec81d040b4..d3b95009364 100644 --- a/app/workers/pages_worker.rb +++ b/app/workers/pages_worker.rb @@ -13,6 +13,7 @@ class PagesWorker if result[:status] == :success result = Projects::UpdatePagesConfigurationService.new(build.project).execute end + result end diff --git a/changelogs/unreleased/18040-line-breaks-around-conditional-blocks.yml b/changelogs/unreleased/18040-line-breaks-around-conditional-blocks.yml new file mode 100644 index 00000000000..447c65a3764 --- /dev/null +++ b/changelogs/unreleased/18040-line-breaks-around-conditional-blocks.yml @@ -0,0 +1,5 @@ +--- +title: Adds Rubocop rule for line break around conditionals +merge_request: 15739 +author: Jacopo Beschi @jacopo-beschi +type: added diff --git a/config/application.rb b/config/application.rb index cd5e3d45bab..ea9a07cbde9 100644 --- a/config/application.rb +++ b/config/application.rb @@ -151,6 +151,7 @@ module Gitlab caching_config_hash[:pool_size] = Sidekiq.options[:concurrency] + 5 caching_config_hash[:pool_timeout] = 1 end + config.cache_store = :redis_store, caching_config_hash config.active_record.raise_in_transactional_callbacks = true diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index f10f0cdf42c..abc992e49dc 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -68,6 +68,7 @@ class Settings < Settingslogic end values.delete_if { |value| value.nil? } end + values end @@ -78,6 +79,7 @@ class Settings < Settingslogic if current.is_a? String value = modul.const_get(current.upcase) rescue default end + value end diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 051ef93b205..fa25f3778fa 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -241,6 +241,7 @@ Devise.setup do |config| true end end + if provider['name'] == 'authentiq' provider['args'][:remote_sign_out_handler] = lambda do |request| authentiq_session = request.params['sid'] diff --git a/config/initializers/peek.rb b/config/initializers/peek.rb index 581397b26f8..e74b95f1646 100644 --- a/config/initializers/peek.rb +++ b/config/initializers/peek.rb @@ -2,6 +2,7 @@ Rails.application.config.peek.adapter = :redis, { client: ::Redis.new(Gitlab::Re Peek.into Peek::Views::Host Peek.into Peek::Views::PerformanceBar + if Gitlab::Database.mysql? require 'peek-mysql2' PEEK_DB_CLIENT = ::Mysql2::Client @@ -11,6 +12,7 @@ else PEEK_DB_CLIENT = ::PG::Connection PEEK_DB_VIEW = Peek::Views::PG end + Peek.into PEEK_DB_VIEW Peek.into Peek::Views::Redis Peek.into Peek::Views::Sidekiq diff --git a/db/migrate/20170928124105_create_fork_networks.rb b/db/migrate/20170928124105_create_fork_networks.rb index ca906b953a3..89e5b871967 100644 --- a/db/migrate/20170928124105_create_fork_networks.rb +++ b/db/migrate/20170928124105_create_fork_networks.rb @@ -23,6 +23,7 @@ class CreateForkNetworks < ActiveRecord::Migration if foreign_keys_for(:fork_networks, :root_project_id).any? remove_foreign_key :fork_networks, column: :root_project_id end + drop_table :fork_networks end end diff --git a/db/migrate/20170928133643_create_fork_network_members.rb b/db/migrate/20170928133643_create_fork_network_members.rb index 836f023efdc..8c7d9ba859a 100644 --- a/db/migrate/20170928133643_create_fork_network_members.rb +++ b/db/migrate/20170928133643_create_fork_network_members.rb @@ -21,6 +21,7 @@ class CreateForkNetworkMembers < ActiveRecord::Migration if foreign_keys_for(:fork_network_members, :forked_from_project_id).any? remove_foreign_key :fork_network_members, column: :forked_from_project_id end + drop_table :fork_network_members end end diff --git a/db/migrate/20171220191323_add_index_on_namespaces_lower_name.rb b/db/migrate/20171220191323_add_index_on_namespaces_lower_name.rb index 7cf1d0cec68..d1a039ed551 100644 --- a/db/migrate/20171220191323_add_index_on_namespaces_lower_name.rb +++ b/db/migrate/20171220191323_add_index_on_namespaces_lower_name.rb @@ -9,6 +9,7 @@ class AddIndexOnNamespacesLowerName < ActiveRecord::Migration return unless Gitlab::Database.postgresql? disable_statement_timeout + if Gitlab::Database.version.to_f >= 9.5 # Allow us to hot-patch the index manually ahead of the migration execute "CREATE INDEX CONCURRENTLY IF NOT EXISTS #{INDEX_NAME} ON namespaces (lower(name));" @@ -21,6 +22,7 @@ class AddIndexOnNamespacesLowerName < ActiveRecord::Migration return unless Gitlab::Database.postgresql? disable_statement_timeout + if Gitlab::Database.version.to_f >= 9.2 execute "DROP INDEX CONCURRENTLY IF EXISTS #{INDEX_NAME};" else diff --git a/db/post_migrate/20170518200835_rename_users_with_renamed_namespace.rb b/db/post_migrate/20170518200835_rename_users_with_renamed_namespace.rb index da0fcda87a6..17ad7de065d 100644 --- a/db/post_migrate/20170518200835_rename_users_with_renamed_namespace.rb +++ b/db/post_migrate/20170518200835_rename_users_with_renamed_namespace.rb @@ -31,6 +31,7 @@ class RenameUsersWithRenamedNamespace < ActiveRecord::Migration predicate = namespaces[:owner_id].eq(users[:id]) .and(namespaces[:type].eq(nil)) .and(users[:username].matches(path)) + update_sql = if Gitlab::Database.postgresql? "UPDATE users SET username = namespaces.path "\ "FROM namespaces WHERE #{predicate.to_sql}" diff --git a/features/steps/project/commits/commits.rb b/features/steps/project/commits/commits.rb index c623a516c47..bd3011b1cd8 100644 --- a/features/steps/project/commits/commits.rb +++ b/features/steps/project/commits/commits.rb @@ -180,11 +180,13 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps dropdown.find(".compare-dropdown-toggle").click dropdown.find('.dropdown-menu', visible: true) dropdown.fill_in("Filter by Git revision", with: selection) + if is_commit dropdown.find('input[type="search"]').send_keys(:return) else find_link(selection, visible: true).click end + dropdown.find('.dropdown-menu', visible: false) end end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index d6ce368efd5..6134ad2bfc7 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -26,6 +26,7 @@ module API check_unmodified_since!(last_updated) status 204 + if block_given? yield resource else diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 8bf53939751..063f0d6599c 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -103,6 +103,7 @@ module API elsif params[:user_id] user = User.find_by(id: params[:user_id]) end + present user, with: Entities::UserSafe end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 7aa10631d53..c99fe3ab5b3 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -175,6 +175,7 @@ module API issue = ::Issues::CreateService.new(user_project, current_user, issue_params.merge(request: request, api: true)).execute + if issue.spam? render_api_error!({ error: 'Spam detected' }, 400) end diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb index 74b3376a1f3..675c963bae2 100644 --- a/lib/api/pipelines.rb +++ b/lib/api/pipelines.rb @@ -48,6 +48,7 @@ module API current_user, declared_params(include_missing: false)) .execute(:api, ignore_skip_ci: true, save_on_errors: false) + if new_pipeline.persisted? present new_pipeline, with: Entities::Pipeline else diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb index 2ccda1c1aa1..5bed58c2d63 100644 --- a/lib/api/project_snippets.rb +++ b/lib/api/project_snippets.rb @@ -13,6 +13,7 @@ module API if errors[:project_access].any? error!(errors[:project_access], 422) end + not_found! end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index fa222bf2b1c..653126e79ea 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -154,6 +154,7 @@ module API if project.errors[:limit_reached].present? error!(project.errors[:limit_reached], 403) end + render_validation_error!(project) end end diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 4f36bbd760f..9638c53a1df 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -15,6 +15,7 @@ module API if errors[:project_access].any? error!(errors[:project_access], 422) end + not_found! end diff --git a/lib/api/v3/members.rb b/lib/api/v3/members.rb index 684860b553e..de226e4e573 100644 --- a/lib/api/v3/members.rb +++ b/lib/api/v3/members.rb @@ -67,6 +67,7 @@ module API unless member member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at]) end + if member.persisted? && member.valid? present member.user, with: ::API::Entities::Member, member: member else diff --git a/lib/api/v3/merge_requests.rb b/lib/api/v3/merge_requests.rb index 1d6d823f32b..0a24fea52a3 100644 --- a/lib/api/v3/merge_requests.rb +++ b/lib/api/v3/merge_requests.rb @@ -126,6 +126,7 @@ module API if status == :deprecated detail DEPRECATION_MESSAGE end + success ::API::V3::Entities::MergeRequest end get path do diff --git a/lib/api/v3/project_snippets.rb b/lib/api/v3/project_snippets.rb index c41fee32610..6ba425ba8c7 100644 --- a/lib/api/v3/project_snippets.rb +++ b/lib/api/v3/project_snippets.rb @@ -14,6 +14,7 @@ module API if errors[:project_access].any? error!(errors[:project_access], 422) end + not_found! end diff --git a/lib/api/v3/projects.rb b/lib/api/v3/projects.rb index 7c260b8d910..446f804124b 100644 --- a/lib/api/v3/projects.rb +++ b/lib/api/v3/projects.rb @@ -41,6 +41,7 @@ module API # private or internal, use the more conservative option, private. attrs[:visibility_level] = (publik == true) ? Gitlab::VisibilityLevel::PUBLIC : Gitlab::VisibilityLevel::PRIVATE end + attrs end @@ -201,6 +202,7 @@ module API if project.errors[:limit_reached].present? error!(project.errors[:limit_reached], 403) end + render_validation_error!(project) end end diff --git a/lib/api/v3/repositories.rb b/lib/api/v3/repositories.rb index f9a47101e27..5b54734bb45 100644 --- a/lib/api/v3/repositories.rb +++ b/lib/api/v3/repositories.rb @@ -14,6 +14,7 @@ module API if errors[:project_access].any? error!(errors[:project_access], 422) end + not_found! end end diff --git a/lib/api/v3/snippets.rb b/lib/api/v3/snippets.rb index 126ec72248e..85613c8ed84 100644 --- a/lib/api/v3/snippets.rb +++ b/lib/api/v3/snippets.rb @@ -97,6 +97,7 @@ module API attrs = declared_params(include_missing: false) UpdateSnippetService.new(nil, current_user, snippet, attrs).execute + if snippet.persisted? present snippet, with: ::API::Entities::PersonalSnippet else diff --git a/lib/backup/database.rb b/lib/backup/database.rb index d97e5d98229..5e6828de597 100644 --- a/lib/backup/database.rb +++ b/lib/backup/database.rb @@ -31,6 +31,7 @@ module Backup pgsql_args << "-n" pgsql_args << Gitlab.config.backup.pg_schema end + spawn('pg_dump', *pgsql_args, config['database'], out: compress_wr) end compress_wr.close diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb index 2a04c03919d..6715159a1aa 100644 --- a/lib/backup/repository.rb +++ b/lib/backup/repository.rb @@ -47,6 +47,7 @@ module Backup if File.exist?(path_to_wiki_repo) progress.print " * #{display_repo_path(wiki)} ... " + if empty_repo?(wiki) progress.puts " [SKIPPED]".color(:cyan) else diff --git a/lib/gitlab/ci/ansi2html.rb b/lib/gitlab/ci/ansi2html.rb index e25916528f4..35eadf6fa93 100644 --- a/lib/gitlab/ci/ansi2html.rb +++ b/lib/gitlab/ci/ansi2html.rb @@ -148,6 +148,7 @@ module Gitlab stream.seek(@offset) append = @offset > 0 end + start_offset = @offset open_new_tag @@ -155,6 +156,7 @@ module Gitlab stream.each_line do |line| s = StringScanner.new(line) until s.eos? + if s.scan(Gitlab::Regex.build_trace_section_regex) handle_section(s) elsif s.scan(/\e([@-_])(.*?)([@-~])/) @@ -168,6 +170,7 @@ module Gitlab else @out << s.scan(/./m) end + @offset += s.matched_size end end @@ -236,8 +239,10 @@ module Gitlab if @style_mask & STYLE_SWITCHES[:bold] != 0 fg_color.sub!(/fg-([a-z]{2,}+)/, 'fg-l-\1') end + css_classes << fg_color end + css_classes << @bg_color unless @bg_color.nil? STYLE_SWITCHES.each do |css_class, flag| diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 7b35c24d153..592a1956ceb 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -512,6 +512,7 @@ module Gitlab batch_size: 10_000, interval: 10.minutes ) + unless relation.model < EachBatch raise TypeError, 'The relation must include the EachBatch module' end diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb index d32616862f0..979225dd216 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb +++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb @@ -26,6 +26,7 @@ module Gitlab move_repository(project, old_full_path, new_full_path) move_repository(project, "#{old_full_path}.wiki", "#{new_full_path}.wiki") end + move_uploads(old_full_path, new_full_path) unless project.hashed_storage?(:attachments) move_pages(old_full_path, new_full_path) end diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb index b669ee5b799..0f897e6316c 100644 --- a/lib/gitlab/diff/highlight.rb +++ b/lib/gitlab/diff/highlight.rb @@ -14,6 +14,7 @@ module Gitlab else @diff_lines = diff_lines end + @raw_lines = @diff_lines.map(&:text) end diff --git a/lib/gitlab/ee_compat_check.rb b/lib/gitlab/ee_compat_check.rb index 37face8e7d0..d3b49b1ec75 100644 --- a/lib/gitlab/ee_compat_check.rb +++ b/lib/gitlab/ee_compat_check.rb @@ -156,12 +156,14 @@ module Gitlab %W[git apply --3way #{patch_path}] ) do |output, status| puts output + unless status.zero? @failed_files = output.lines.reduce([]) do |memo, line| if line.start_with?('error: patch failed:') file = line.sub(/\Aerror: patch failed: /, '') memo << file unless file =~ IGNORED_FILES_REGEX end + memo end diff --git a/lib/gitlab/email/handler/create_merge_request_handler.rb b/lib/gitlab/email/handler/create_merge_request_handler.rb index e2f7c1d0257..3436306e122 100644 --- a/lib/gitlab/email/handler/create_merge_request_handler.rb +++ b/lib/gitlab/email/handler/create_merge_request_handler.rb @@ -10,6 +10,7 @@ module Gitlab def initialize(mail, mail_key) super(mail, mail_key) + if m = /\A([^\+]*)\+merge-request\+(.*)/.match(mail_key.to_s) @project_path, @incoming_email_token = m.captures end diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 5e426b13ade..8953bc8c148 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -112,6 +112,7 @@ module Gitlab [bug['sCategory'], bug['sPriority']].each do |label| unless label.blank? labels << label + unless @known_labels.include?(label) create_label(label) @known_labels << label @@ -265,6 +266,7 @@ module Gitlab if content.blank? content = '*(No description has been entered for this issue)*' end + body << content body.join("\n\n") @@ -278,6 +280,7 @@ module Gitlab if content.blank? content = "*(No comment has been entered for this change)*" end + body << content if updates.any? diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index f371adfa109..6ada9a145db 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -668,6 +668,7 @@ module Gitlab end end end + @refs_hash end diff --git a/lib/gitlab/git/storage/forked_storage_check.rb b/lib/gitlab/git/storage/forked_storage_check.rb index 1307f400700..0a4e557b59b 100644 --- a/lib/gitlab/git/storage/forked_storage_check.rb +++ b/lib/gitlab/git/storage/forked_storage_check.rb @@ -27,6 +27,7 @@ module Gitlab status = nil while status.nil? + if deadline > Time.now.utc sleep(wait_time) _pid, status = Process.wait2(filesystem_check_pid, Process::WNOHANG) diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index ae1753ff0ae..37ba1af8d6f 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -52,6 +52,7 @@ module Gitlab ) response = GitalyClient.call(@repository.storage, :operation_service, :user_create_branch, request) + if response.pre_receive_error.present? raise Gitlab::Git::HooksService::PreReceiveError.new(response.pre_receive_error) end diff --git a/lib/gitlab/google_code_import/importer.rb b/lib/gitlab/google_code_import/importer.rb index ab38c0c3e34..46b49128140 100644 --- a/lib/gitlab/google_code_import/importer.rb +++ b/lib/gitlab/google_code_import/importer.rb @@ -302,6 +302,7 @@ module Gitlab else "#{project.namespace.full_path}/#{name}##{id}" end + text = "~~#{text}~~" if deleted text end @@ -329,6 +330,7 @@ module Gitlab if content.blank? content = "*(No comment has been entered for this change)*" end + body << content if updates.any? @@ -352,6 +354,7 @@ module Gitlab if content.blank? content = "*(No description has been entered for this issue)*" end + body << content if attachments.any? diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb index c518943be59..4b5f9f3a926 100644 --- a/lib/gitlab/import_export/project_tree_restorer.rb +++ b/lib/gitlab/import_export/project_tree_restorer.rb @@ -148,6 +148,7 @@ module Gitlab else relation_hash = relation_item[sub_relation.to_s] end + [relation_hash, sub_relation] end diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 05dbaf6322c..cb711a83433 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -267,6 +267,7 @@ module Gitlab else %w[title group_id] end + finder_hash = parsed_relation_hash.slice(*finder_attributes) if label? diff --git a/lib/gitlab/kubernetes/helm/pod.rb b/lib/gitlab/kubernetes/helm/pod.rb index 233f6bf6227..97ad3c97e95 100644 --- a/lib/gitlab/kubernetes/helm/pod.rb +++ b/lib/gitlab/kubernetes/helm/pod.rb @@ -14,6 +14,7 @@ module Gitlab generate_config_map spec['volumes'] = volumes_specification end + ::Kubeclient::Resource.new(metadata: metadata, spec: spec) end diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb index 0d9a554fc18..cde60addcf7 100644 --- a/lib/gitlab/ldap/config.rb +++ b/lib/gitlab/ldap/config.rb @@ -42,6 +42,7 @@ module Gitlab else self.class.invalid_provider(provider) end + @options = config_for(@provider) # Use @provider, not provider end diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb index 877cebf6786..ef44a13df51 100644 --- a/lib/gitlab/metrics/influx_db.rb +++ b/lib/gitlab/metrics/influx_db.rb @@ -169,6 +169,7 @@ module Gitlab end end end + @pool end end diff --git a/lib/gitlab/middleware/multipart.rb b/lib/gitlab/middleware/multipart.rb index fee741b47be..cc1e92480be 100644 --- a/lib/gitlab/middleware/multipart.rb +++ b/lib/gitlab/middleware/multipart.rb @@ -47,6 +47,7 @@ module Gitlab else value = decorate_params_value(value, @request.params[key], tmp_path) end + @request.update_param(key, value) end @@ -60,6 +61,7 @@ module Gitlab unless path_hash.is_a?(Hash) && path_hash.count == 1 raise "invalid path: #{path_hash.inspect}" end + path_key, path_value = path_hash.first unless value_hash.is_a?(Hash) && value_hash[path_key] diff --git a/lib/gitlab/multi_collection_paginator.rb b/lib/gitlab/multi_collection_paginator.rb index c22d0a84860..43921a8c1c0 100644 --- a/lib/gitlab/multi_collection_paginator.rb +++ b/lib/gitlab/multi_collection_paginator.rb @@ -37,6 +37,7 @@ module Gitlab else per_page - first_collection_last_page_size end + hash[page] = second_collection.page(second_collection_page) .per(per_page - paginated_first_collection(page).size) .padding(offset) diff --git a/lib/gitlab/quick_actions/extractor.rb b/lib/gitlab/quick_actions/extractor.rb index 3ebfa3bd4b8..c0878a34fb1 100644 --- a/lib/gitlab/quick_actions/extractor.rb +++ b/lib/gitlab/quick_actions/extractor.rb @@ -126,6 +126,7 @@ module Gitlab command << match_data[1] unless match_data[1].empty? commands << command end + content = substitution.perform_substitution(self, content) end diff --git a/lib/gitlab/redis/wrapper.rb b/lib/gitlab/redis/wrapper.rb index 8ad06480575..4178b436acf 100644 --- a/lib/gitlab/redis/wrapper.rb +++ b/lib/gitlab/redis/wrapper.rb @@ -24,6 +24,7 @@ module Gitlab # the pool will be used in a multi-threaded context size += Sidekiq.options[:concurrency] end + size end @@ -104,6 +105,7 @@ module Gitlab db_numbers = queries["db"] if queries.key?("db") config[:db] = db_numbers[0].to_i if db_numbers.any? end + config else redis_hash = ::Redis::Store::Factory.extract_host_options_from_uri(redis_url) diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index ca48c6df602..70b639501fd 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -115,6 +115,7 @@ module Gitlab else merge_requests.full_search(query) end + merge_requests.order('updated_at DESC') end diff --git a/lib/gitlab/storage_check/cli.rb b/lib/gitlab/storage_check/cli.rb index 04bf1bf1d26..9b64c8e033a 100644 --- a/lib/gitlab/storage_check/cli.rb +++ b/lib/gitlab/storage_check/cli.rb @@ -59,9 +59,11 @@ module Gitlab if response.skipped_shards.any? warnings << "Skipped shards: #{response.skipped_shards.join(', ')}" end + if response.failing_shards.any? warnings << "Failing shards: #{response.failing_shards.join(', ')}" end + logger.warn(warnings.join(' - ')) if warnings.any? end end diff --git a/lib/gitlab/testing/request_blocker_middleware.rb b/lib/gitlab/testing/request_blocker_middleware.rb index 4a8e3c2eee0..53333b9b06b 100644 --- a/lib/gitlab/testing/request_blocker_middleware.rb +++ b/lib/gitlab/testing/request_blocker_middleware.rb @@ -37,12 +37,14 @@ module Gitlab def call(env) increment_active_requests + if block_requests? block_request(env) else sleep 0.2 if slow_requests? @app.call(env) end + ensure decrement_active_requests end diff --git a/lib/gitlab/timeless.rb b/lib/gitlab/timeless.rb index b290c716f97..76a1808c8ac 100644 --- a/lib/gitlab/timeless.rb +++ b/lib/gitlab/timeless.rb @@ -9,6 +9,7 @@ module Gitlab else block.call end + ensure model.record_timestamps = original_record_timestamps end diff --git a/lib/gitlab/upgrader.rb b/lib/gitlab/upgrader.rb index 961df0468a4..3b64cb32afa 100644 --- a/lib/gitlab/upgrader.rb +++ b/lib/gitlab/upgrader.rb @@ -12,6 +12,7 @@ module Gitlab puts "You are using the latest GitLab version" else puts "Newer GitLab version is available" + answer = if ARGV.first == "-y" "yes" else @@ -77,6 +78,7 @@ module Gitlab update_commands.each do |title, cmd| puts title puts " -> #{cmd.join(' ')}" + if system(env, *cmd) puts " -> OK" else diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index ce6d0422c1f..0de183858aa 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -42,6 +42,7 @@ module Gitlab else raise "Unsupported action: #{action}" end + if feature_enabled params[:GitalyServer] = server end diff --git a/lib/system_check/simple_executor.rb b/lib/system_check/simple_executor.rb index 8b145fb4511..d268f501b4a 100644 --- a/lib/system_check/simple_executor.rb +++ b/lib/system_check/simple_executor.rb @@ -66,6 +66,7 @@ module SystemCheck if check.can_repair? $stdout.print 'Trying to fix error automatically. ...' + if check.repair! $stdout.puts 'Success'.color(:green) return diff --git a/lib/tasks/gitlab/backup.rake b/lib/tasks/gitlab/backup.rake index 9dcf44fdc3e..2383bcf954b 100644 --- a/lib/tasks/gitlab/backup.rake +++ b/lib/tasks/gitlab/backup.rake @@ -46,6 +46,7 @@ namespace :gitlab do puts 'Removing all tables. Press `Ctrl-C` within 5 seconds to abort'.color(:yellow) sleep(5) end + # Drop all tables Load the schema to ensure we don't have any newer tables # hanging out from a failed upgrade $progress.puts 'Cleaning the database ... '.color(:blue) @@ -222,6 +223,7 @@ namespace :gitlab do task restore: :environment do $progress.puts "Restoring container registry images ... ".color(:blue) + if Gitlab.config.registry.enabled Backup::Registry.new.restore $progress.puts "done".color(:green) diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 903e84359cd..31cd6bfe6e1 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -180,6 +180,7 @@ namespace :gitlab do puts "can't check, you have no projects".color(:magenta) return end + puts "" Project.find_each(batch_size: 100) do |project| @@ -210,6 +211,7 @@ namespace :gitlab do gitlab_shell_repo_base = gitlab_shell_path check_cmd = File.expand_path('bin/check', gitlab_shell_repo_base) puts "Running #{check_cmd}" + if system(check_cmd, chdir: gitlab_shell_repo_base) puts 'gitlab-shell self-check successful'.color(:green) else @@ -285,6 +287,7 @@ namespace :gitlab do return if process_count.zero? print 'Number of Sidekiq processes ... ' + if process_count == 1 puts '1'.color(:green) else diff --git a/lib/tasks/gitlab/cleanup.rake b/lib/tasks/gitlab/cleanup.rake index eb0f757aea7..04d56509ac6 100644 --- a/lib/tasks/gitlab/cleanup.rake +++ b/lib/tasks/gitlab/cleanup.rake @@ -84,6 +84,7 @@ namespace :gitlab do next unless user.ldap_user? print "#{user.name} (#{user.ldap_identity.extern_uid}) ..." + if Gitlab::LDAP::Access.allowed?(user) puts " [OK]".color(:green) else diff --git a/lib/tasks/gitlab/dev.rake b/lib/tasks/gitlab/dev.rake index ba221e44e5d..77c28615856 100644 --- a/lib/tasks/gitlab/dev.rake +++ b/lib/tasks/gitlab/dev.rake @@ -14,6 +14,7 @@ namespace :gitlab do puts "Must specify a branch as an argument".color(:red) exit 1 end + args end diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index 4507b841964..a2e68c0471b 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/lib/tasks/gitlab/gitaly.rake @@ -5,9 +5,11 @@ namespace :gitlab do require 'toml' warn_user_is_not_gitlab + unless args.dir.present? abort %(Please specify the directory where you want to install gitaly:\n rake "gitlab:gitaly:install[/home/git/gitaly]") end + args.with_defaults(repo: 'https://gitlab.com/gitlab-org/gitaly.git') version = Gitlab::GitalyClient.expected_server_version diff --git a/lib/tasks/gitlab/list_repos.rake b/lib/tasks/gitlab/list_repos.rake index b732db9db6e..d7f28691098 100644 --- a/lib/tasks/gitlab/list_repos.rake +++ b/lib/tasks/gitlab/list_repos.rake @@ -8,6 +8,7 @@ namespace :gitlab do namespace_ids = Namespace.where(['updated_at > ?', date]).pluck(:id).sort scope = scope.where('id IN (?) OR namespace_id in (?)', project_ids, namespace_ids) end + scope.find_each do |project| base = File.join(project.repository_storage_path, project.disk_path) puts base + '.git' diff --git a/lib/tasks/gitlab/update_templates.rake b/lib/tasks/gitlab/update_templates.rake index f44abc2b81b..a25f7ce59c7 100644 --- a/lib/tasks/gitlab/update_templates.rake +++ b/lib/tasks/gitlab/update_templates.rake @@ -10,6 +10,7 @@ namespace :gitlab do puts "This rake task is not meant fo production instances".red exit(1) end + admin = User.find_by(admin: true) unless admin diff --git a/lib/tasks/gitlab/workhorse.rake b/lib/tasks/gitlab/workhorse.rake index e7ac0b5859f..308ffb0e284 100644 --- a/lib/tasks/gitlab/workhorse.rake +++ b/lib/tasks/gitlab/workhorse.rake @@ -3,9 +3,11 @@ namespace :gitlab do desc "GitLab | Install or upgrade gitlab-workhorse" task :install, [:dir, :repo] => :environment do |t, args| warn_user_is_not_gitlab + unless args.dir.present? abort %(Please specify the directory where you want to install gitlab-workhorse:\n rake "gitlab:workhorse:install[/home/git/gitlab-workhorse]") end + args.with_defaults(repo: 'https://gitlab.com/gitlab-org/gitlab-workhorse.git') version = Gitlab::Workhorse.version diff --git a/lib/tasks/migrate/migrate_iids.rake b/lib/tasks/migrate/migrate_iids.rake index fc2cea8c016..aa2d01730d7 100644 --- a/lib/tasks/migrate/migrate_iids.rake +++ b/lib/tasks/migrate/migrate_iids.rake @@ -4,6 +4,7 @@ task migrate_iids: :environment do Issue.where(iid: nil).find_each(batch_size: 100) do |issue| begin issue.set_iid + if issue.update_attribute(:iid, issue.iid) print '.' else @@ -19,6 +20,7 @@ task migrate_iids: :environment do MergeRequest.where(iid: nil).find_each(batch_size: 100) do |mr| begin mr.set_iid + if mr.update_attribute(:iid, mr.iid) print '.' else @@ -34,6 +36,7 @@ task migrate_iids: :environment do Milestone.where(iid: nil).find_each(batch_size: 100) do |m| begin m.set_iid + if m.update_attribute(:iid, m.iid) print '.' else diff --git a/rubocop/cop/line_break_around_conditional_block.rb b/rubocop/cop/line_break_around_conditional_block.rb new file mode 100644 index 00000000000..3e7021e724e --- /dev/null +++ b/rubocop/cop/line_break_around_conditional_block.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # Ensures a line break around conditional blocks. + # + # @example + # # bad + # do_something + # if condition + # do_extra_stuff + # end + # do_something_more + # + # # good + # do_something + # + # if condition + # do_extra_stuff + # end + # + # do_something_more + # + # # bad + # do_something + # unless condition + # do_extra_stuff + # end + # + # do_something_more + # + # # good + # def a_method + # if condition + # do_something + # end + # end + # + # # good + # on_block do + # if condition + # do_something + # end + # end + class LineBreakAroundConditionalBlock < RuboCop::Cop::Cop + MSG = 'Add a line break around conditional blocks' + + def on_if(node) + return if node.single_line? + return unless node.if? || node.unless? + + add_offense(node, location: :expression, message: MSG) unless previous_line_valid?(node) + add_offense(node, location: :expression, message: MSG) unless last_line_valid?(node) + end + + def autocorrect(node) + lambda do |corrector| + line = range_by_whole_lines(node.source_range) + unless previous_line_valid?(node) + corrector.insert_before(line, "\n") + end + + unless last_line_valid?(node) + corrector.insert_after(line, "\n") + end + end + end + + private + + def previous_line_valid?(node) + previous_line(node).empty? || + start_clause_line?(previous_line(node)) || + block_start?(previous_line(node)) || + begin_line?(previous_line(node)) || + assignment_line?(previous_line(node)) + end + + def last_line_valid?(node) + last_line(node).empty? || + end_line?(last_line(node)) || + end_clause_line?(last_line(node)) + end + + def previous_line(node) + processed_source[node.loc.line - 2] + end + + def last_line(node) + processed_source[node.loc.last_line] + end + + def start_clause_line?(line) + line =~ /^\s*(def|=end|#|module|class|if|unless|else|elsif|ensure|when)/ + end + + def end_clause_line?(line) + line =~ /^\s*(rescue|else|elsif|when)/ + end + + def begin_line?(line) + # an assignment followed by a begin or ust a begin + line =~ /^\s*(@?(\w|\|+|=|\[|\]|\s)+begin|begin)/ + end + + def assignment_line?(line) + line =~ /^\s*.*=/ + end + + def block_start?(line) + line.match(/ (do|{)( \|.*?\|)?\s?$/) + end + + def end_line?(line) + line =~ /^\s*(end|})/ + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 2f63babc425..57af87f7fb9 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,5 +1,6 @@ require_relative 'cop/gitlab/module_with_instance_variables' require_relative 'cop/include_sidekiq_worker' +require_relative 'cop/line_break_around_conditional_block' require_relative 'cop/migration/add_column' require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_concurrent_index' diff --git a/spec/factories/protected_branches.rb b/spec/factories/protected_branches.rb index 39460834d06..60956511834 100644 --- a/spec/factories/protected_branches.rb +++ b/spec/factories/protected_branches.rb @@ -53,6 +53,7 @@ FactoryBot.define do if evaluator.default_access_level && evaluator.default_push_level protected_branch.push_access_levels.new(access_level: Gitlab::Access::MASTER) end + if evaluator.default_access_level && evaluator.default_merge_level protected_branch.merge_access_levels.new(access_level: Gitlab::Access::MASTER) end diff --git a/spec/features/issues/bulk_assignment_labels_spec.rb b/spec/features/issues/bulk_assignment_labels_spec.rb index 587ece22ec7..cf283119f36 100644 --- a/spec/features/issues/bulk_assignment_labels_spec.rb +++ b/spec/features/issues/bulk_assignment_labels_spec.rb @@ -377,6 +377,7 @@ feature 'Issues > Labels bulk assignment' do items.map do |item| click_link item end + if unmark items.map do |item| # Make sure we are unmarking the item no matter the state it has currently diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index ffa17d296e8..f246bb79ab7 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -142,6 +142,7 @@ describe API::CommitStatuses do expect(json_response['ref']).not_to be_empty expect(json_response['target_url']).to be_nil expect(json_response['description']).to be_nil + if status == 'failed' expect(CommitStatus.find(json_response['id'])).to be_api_failure end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 679d391caa5..cb66d23b77c 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -1142,6 +1142,7 @@ describe API::Runner do else { 'file' => file } end + post api("/jobs/#{job.id}/artifacts"), params, headers end end diff --git a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb new file mode 100644 index 00000000000..7ddf9141fcd --- /dev/null +++ b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb @@ -0,0 +1,411 @@ +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../rubocop/cop/line_break_around_conditional_block' + +describe RuboCop::Cop::LineBreakAroundConditionalBlock do + include CopHelper + + subject(:cop) { described_class.new } + + shared_examples 'examples with conditional' do |conditional| + it "flags violation for #{conditional} without line break before" do + source = <<~RUBY + do_something + #{conditional} condition + do_something_more + end + RUBY + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + offense = cop.offenses.first + + expect(offense.line).to eq(2) + expect(cop.highlights).to eq(["#{conditional} condition\n do_something_more\nend"]) + expect(offense.message).to eq('Add a line break around conditional blocks') + end + + it "flags violation for #{conditional} without line break after" do + source = <<~RUBY + #{conditional} condition + do_something + end + do_something_more + RUBY + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + offense = cop.offenses.first + + expect(offense.line).to eq(1) + expect(cop.highlights).to eq(["#{conditional} condition\n do_something\nend"]) + expect(offense.message).to eq('Add a line break around conditional blocks') + end + + it "doesn't flag violation for #{conditional} with line break before and after" do + source = <<~RUBY + #{conditional} condition + do_something + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by a method definition" do + source = <<~RUBY + def a_method + #{conditional} condition + do_something + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by a class definition" do + source = <<~RUBY + class Foo + #{conditional} condition + do_something + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by a module definition" do + source = <<~RUBY + module Foo + #{conditional} condition + do_something + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by a begin definition" do + source = <<~RUBY + begin + #{conditional} condition + do_something + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by an assign/begin definition" do + source = <<~RUBY + @project ||= begin + #{conditional} condition + do_something + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by a block definition" do + source = <<~RUBY + on_block(param_a) do |item| + #{conditional} condition + do_something + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by a block definition using brackets" do + source = <<~RUBY + on_block(param_a) { |item| + #{conditional} condition + do_something + end + } + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by a comment" do + source = <<~RUBY + # a short comment + #{conditional} condition + do_something + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by an assignment" do + source = <<~RUBY + foo = + #{conditional} condition + do_something + else + do_something_more + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by a multiline comment" do + source = <<~RUBY + =begin + a multiline comment + =end + #{conditional} condition + do_something + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by another conditional" do + source = <<~RUBY + #{conditional} condition_a + #{conditional} condition_b + do_something + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by an else" do + source = <<~RUBY + if condition_a + do_something + else + #{conditional} condition_b + do_something_extra + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by an elsif" do + source = <<~RUBY + if condition_a + do_something + elsif condition_b + #{conditional} condition_c + do_something_extra + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by an ensure" do + source = <<~RUBY + def a_method + ensure + #{conditional} condition_c + do_something_extra + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} preceded by a when" do + source = <<~RUBY + case field + when value + #{conditional} condition_c + do_something_extra + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} followed by an end" do + source = <<~RUBY + class Foo + + #{conditional} condition + do_something + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} followed by an else" do + source = <<~RUBY + #{conditional} condition_a + #{conditional} condition_b + do_something + end + else + do_something_extra + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} followed by a when" do + source = <<~RUBY + case + when condition_a + #{conditional} condition_b + do_something + end + when condition_c + do_something_extra + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} followed by an elsif" do + source = <<~RUBY + if condition_a + #{conditional} condition_b + do_something + end + elsif condition_c + do_something_extra + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "doesn't flag violation for #{conditional} followed by a rescue" do + source = <<~RUBY + def a_method + #{conditional} condition + do_something + end + rescue + do_something_extra + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + + it "autocorrects #{conditional} without line break before" do + source = <<~RUBY + do_something + #{conditional} condition + do_something_more + end + RUBY + autocorrected = autocorrect_source(source) + + expected_source = <<~RUBY + do_something + + #{conditional} condition + do_something_more + end + RUBY + expect(autocorrected).to eql(expected_source) + end + + it "autocorrects #{conditional} without line break after" do + source = <<~RUBY + #{conditional} condition + do_something + end + do_something_more + RUBY + autocorrected = autocorrect_source(source) + + expected_source = <<~RUBY + #{conditional} condition + do_something + end + + do_something_more + RUBY + expect(autocorrected).to eql(expected_source) + end + + it "autocorrects #{conditional} without line break before and after" do + source = <<~RUBY + do_something + #{conditional} condition + do_something_more + end + do_something_extra + RUBY + autocorrected = autocorrect_source(source) + + expected_source = <<~RUBY + do_something + + #{conditional} condition + do_something_more + end + + do_something_extra + RUBY + expect(autocorrected).to eql(expected_source) + end + end + + %w[if unless].each do |example| + it_behaves_like 'examples with conditional', example + end + + it "doesn't flag violation for if with elsif" do + source = <<~RUBY + if condition + do_something + elsif another_condition + do_something_more + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end +end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 39f6388c25e..ef68742a463 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -150,6 +150,7 @@ describe Projects::TransferService do before do group.add_owner(user) + unless gitlab_shell.add_repository(repository_storage, "#{group.full_path}/#{project.path}") raise 'failed to add repository' end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 4e640a82dfc..965fd39c967 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -727,6 +727,7 @@ describe SystemNoteService do else "#{Settings.gitlab.base_url}/#{project.namespace.path}/#{project.path}/merge_requests/#{merge_request.iid}" end + link = double(object: { 'url' => url }) links << link expect(link).to receive(:save!) diff --git a/spec/support/features/discussion_comments_shared_example.rb b/spec/support/features/discussion_comments_shared_example.rb index c24940393f9..fa94aa2ae3d 100644 --- a/spec/support/features/discussion_comments_shared_example.rb +++ b/spec/support/features/discussion_comments_shared_example.rb @@ -113,6 +113,7 @@ shared_examples 'discussion comments' do |resource_name| else expect(find(submit_selector).value).to eq 'Start discussion' end + expect(page).not_to have_selector menu_selector end @@ -200,6 +201,7 @@ shared_examples 'discussion comments' do |resource_name| else expect(find(submit_selector).value).to eq 'Comment' end + expect(page).not_to have_selector menu_selector end diff --git a/spec/support/filtered_search_helpers.rb b/spec/support/filtered_search_helpers.rb index 05021ea9054..f3f96bd1f0a 100644 --- a/spec/support/filtered_search_helpers.rb +++ b/spec/support/filtered_search_helpers.rb @@ -61,9 +61,11 @@ module FilteredSearchHelpers token_emoji = tokens[index][:emoji_name] expect(el.find('.name')).to have_content(token_name) + if token_value expect(el.find('.value')).to have_content(token_value) end + # gl-emoji content is blank when the emoji unicode is not supported if token_emoji selector = %(gl-emoji[data-name="#{token_emoji}"]) diff --git a/spec/support/generate-seed-repo-rb b/spec/support/generate-seed-repo-rb index 4ee33f9725b..876b3b8242d 100755 --- a/spec/support/generate-seed-repo-rb +++ b/spec/support/generate-seed-repo-rb @@ -24,6 +24,7 @@ def main unless system(*%W[git clone --bare #{SOURCE} #{REPO_NAME}], chdir: dir) abort "git clone failed" end + repo = File.join(dir, REPO_NAME) erb = ERB.new(DATA.read) erb.run(binding) diff --git a/spec/support/matchers/access_matchers_for_controller.rb b/spec/support/matchers/access_matchers_for_controller.rb index cdb62a5deee..42a9ed9ff34 100644 --- a/spec/support/matchers/access_matchers_for_controller.rb +++ b/spec/support/matchers/access_matchers_for_controller.rb @@ -43,6 +43,7 @@ module AccessMatchersForController user = create(:user) membership.public_send(:"add_#{role}", user) end + user end diff --git a/spec/support/select2_helper.rb b/spec/support/select2_helper.rb index 55da961e173..90618ba5b19 100644 --- a/spec/support/select2_helper.rb +++ b/spec/support/select2_helper.rb @@ -17,6 +17,7 @@ module Select2Helper selector = options.fetch(:from) first(selector, visible: false) + if options[:multiple] execute_script("$('#{selector}').select2('val', ['#{value}']).trigger('change');") else diff --git a/spec/support/stub_env.rb b/spec/support/stub_env.rb index f621463e621..695152e2d4e 100644 --- a/spec/support/stub_env.rb +++ b/spec/support/stub_env.rb @@ -4,6 +4,7 @@ module StubENV def stub_env(key_or_hash, value = nil) init_stub unless env_stubbed? + if key_or_hash.is_a? Hash key_or_hash.each { |k, v| add_stubbed_value(k, v) } else diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 664698fcbaf..25ff6094408 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -325,6 +325,7 @@ module TestEnv if component_needs_update?(install_dir, version) # Cleanup the component entirely to ensure we start fresh FileUtils.rm_rf(install_dir) + unless system('rake', task) raise ComponentFailedToInstallError end diff --git a/spec/support/wait_for_requests.rb b/spec/support/wait_for_requests.rb index f4130d68271..fda0e29f983 100644 --- a/spec/support/wait_for_requests.rb +++ b/spec/support/wait_for_requests.rb @@ -53,6 +53,7 @@ module WaitForRequests wait_until = Time.now + max_wait_time.seconds loop do break if yield + if Time.now > wait_until raise "Condition not met: #{condition_name}" else |