diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-21 14:21:10 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-21 14:21:10 +0000 |
commit | cb0d23c455b73486fd1015f8ca9479b5b7e3585d (patch) | |
tree | d7dc129a407fd74266d2dc561bebf24665197c2f /rubocop | |
parent | c3e911be175c0aabfea1eb030f9e0ef23f5f3887 (diff) | |
download | gitlab-ce-cb0d23c455b73486fd1015f8ca9479b5b7e3585d.tar.gz |
Add latest changes from gitlab-org/gitlab@12-7-stable-ee
Diffstat (limited to 'rubocop')
-rw-r--r-- | rubocop/cop/migration/add_column_with_default.rb | 45 | ||||
-rw-r--r-- | rubocop/cop/rspec/have_gitlab_http_status.rb | 119 | ||||
-rw-r--r-- | rubocop/rubocop.rb | 2 |
3 files changed, 166 insertions, 0 deletions
diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/add_column_with_default.rb new file mode 100644 index 00000000000..d9f8fe62a86 --- /dev/null +++ b/rubocop/cop/migration/add_column_with_default.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if columns are added in a way that doesn't require + # downtime. + class AddColumnWithDefault < RuboCop::Cop::Cop + include MigrationHelpers + + WHITELISTED_TABLES = [:application_settings].freeze + + MSG = '`add_column_with_default` without `allow_null: true` may cause prolonged lock situations and downtime, ' \ + 'see https://gitlab.com/gitlab-org/gitlab/issues/38060'.freeze + + def_node_matcher :add_column_with_default?, <<~PATTERN + (send _ :add_column_with_default $_ ... (hash $...)) + PATTERN + + def on_send(node) + return unless in_migration?(node) + + add_column_with_default?(node) do |table, options| + break if table_whitelisted?(table) || nulls_allowed?(options) + + add_offense(node, location: :selector) + end + end + + private + + def nulls_allowed?(options) + options.find { |opt| opt.key.value == :allow_null && opt.value.true_type? } + end + + def table_whitelisted?(symbol) + symbol && symbol.type == :sym && + WHITELISTED_TABLES.include?(symbol.children[0]) + end + end + end + end +end diff --git a/rubocop/cop/rspec/have_gitlab_http_status.rb b/rubocop/cop/rspec/have_gitlab_http_status.rb new file mode 100644 index 00000000000..6b179720060 --- /dev/null +++ b/rubocop/cop/rspec/have_gitlab_http_status.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'rack/utils' + +module RuboCop + module Cop + module RSpec + # This cops checks for `have_http_status` usages in specs. + # It also discourages the usage of numeric HTTP status codes in + # `have_gitlab_http_status`. + # + # @example + # + # # bad + # expect(response).to have_http_status(200) + # expect(response).to have_http_status(:ok) + # expect(response).to have_gitlab_http_status(200) + # + # # good + # expect(response).to have_gitlab_http_status(:ok) + # + class HaveGitlabHttpStatus < RuboCop::Cop::Cop + CODE_TO_SYMBOL = Rack::Utils::SYMBOL_TO_STATUS_CODE.invert + + MSG_MATCHER_NAME = + 'Use `have_gitlab_http_status` instead of `have_http_status`.' + + MSG_STATUS = + 'Prefer named HTTP status `%{name}` over ' \ + 'its numeric representation `%{code}`.' + + MSG_UNKNOWN = 'HTTP status `%{code}` is unknown. ' \ + 'Please provide a valid one or disable this cop.' + + MSG_DOCS_LINK = 'https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#have_gitlab_http_status' + + REPLACEMENT = 'have_gitlab_http_status(%{arg})' + + def_node_matcher :have_http_status?, <<~PATTERN + ( + send nil? + { + :have_http_status + :have_gitlab_http_status + } + _ + ) + PATTERN + + def on_send(node) + return unless have_http_status?(node) + + offenses = [ + offense_for_name(node), + offense_for_status(node) + ].compact + + return if offenses.empty? + + add_offense(node, message: message_for(offenses)) + end + + def autocorrect(node) + lambda do |corrector| + corrector.replace(node.source_range, replacement(node)) + end + end + + private + + def offense_for_name(node) + return if method_name(node) == :have_gitlab_http_status + + MSG_MATCHER_NAME + end + + def offense_for_status(node) + code = extract_numeric_code(node) + return unless code + + symbol = code_to_symbol(code) + return format(MSG_UNKNOWN, code: code) unless symbol + + format(MSG_STATUS, name: symbol, code: code) + end + + def message_for(offenses) + (offenses + [MSG_DOCS_LINK]).join(' ') + end + + def replacement(node) + code = extract_numeric_code(node) + arg = code_to_symbol(code) || argument(node).source + + format(REPLACEMENT, arg: arg) + end + + def code_to_symbol(code) + CODE_TO_SYMBOL[code]&.inspect + end + + def extract_numeric_code(node) + arg_node = argument(node) + return unless arg_node&.type == :int + + arg_node.children[0] + end + + def method_name(node) + node.children[1] + end + + def argument(node) + node.children[2] + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 1465c73d570..1479dc3384a 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -17,6 +17,7 @@ require_relative 'cop/prefer_class_methods_over_module' require_relative 'cop/put_project_routes_under_scope' require_relative 'cop/put_group_routes_under_scope' require_relative 'cop/migration/add_column' +require_relative 'cop/migration/add_column_with_default' require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_concurrent_index' require_relative 'cop/migration/add_index' @@ -39,6 +40,7 @@ require_relative 'cop/rspec/be_success_matcher' require_relative 'cop/rspec/env_assignment' require_relative 'cop/rspec/factories_in_migration_specs' require_relative 'cop/rspec/top_level_describe_path' +require_relative 'cop/rspec/have_gitlab_http_status' require_relative 'cop/qa/element_with_pattern' require_relative 'cop/qa/ambiguous_page_object_name' require_relative 'cop/sidekiq_options_queue' |