diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-11-14 11:41:52 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-11-14 11:41:52 +0300 |
commit | 585826cb22ecea5998a2c2a4675735c94bdeedac (patch) | |
tree | 5b05f0b30d33cef48963609e8a18a4dff260eab3 /rubocop | |
parent | df221d036e5d0c6c0ee4d55b9c97f481ee05dee8 (diff) |
Add latest changes from gitlab-org/gitlab@16-6-stable-eev16.6.0-rc42
Diffstat (limited to 'rubocop')
-rw-r--r-- | rubocop/batched_background_migrations_dictionary.rb (renamed from rubocop/batched_background_migrations.rb) | 19 | ||||
-rw-r--r-- | rubocop/cop/background_migration/dictionary_file.rb (renamed from rubocop/cop/background_migration/missing_dictionary_file.rb) | 31 | ||||
-rw-r--r-- | rubocop/cop/gitlab/avoid_gitlab_instance_checks.rb | 2 | ||||
-rw-r--r-- | rubocop/cop/gitlab/doc_url.rb | 2 | ||||
-rw-r--r-- | rubocop/cop/gitlab/mark_used_feature_flags.rb | 11 | ||||
-rw-r--r-- | rubocop/cop/migration/migration_with_milestone.rb | 28 | ||||
-rw-r--r-- | rubocop/cop/migration/prevent_index_creation.rb | 4 | ||||
-rw-r--r-- | rubocop/cop/migration/unfinished_dependencies.rb | 4 | ||||
-rw-r--r-- | rubocop/cop/migration/with_lock_retries_disallowed_method.rb | 8 | ||||
-rw-r--r-- | rubocop/cop/style/inline_disable_annotation.rb | 51 | ||||
-rw-r--r-- | rubocop/rubocop-code_reuse.yml | 1 | ||||
-rw-r--r-- | rubocop/rubocop.rb | 6 |
12 files changed, 143 insertions, 24 deletions
diff --git a/rubocop/batched_background_migrations.rb b/rubocop/batched_background_migrations_dictionary.rb index ce7115e5cd5..286f0a57bad 100644 --- a/rubocop/batched_background_migrations.rb +++ b/rubocop/batched_background_migrations_dictionary.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module RuboCop - class BatchedBackgroundMigrations + class BatchedBackgroundMigrationsDictionary DICTIONARY_BASE_DIR = 'db/docs/batched_background_migrations' attr_reader :queued_migration_version @@ -14,6 +14,7 @@ module RuboCop next unless dictionary['queued_migration_version'].present? data[dictionary['queued_migration_version'].to_s] = { + introduced_by_url: dictionary['introduced_by_url'], finalize_after: dictionary['finalize_after'], finalized_by: dictionary['finalized_by'].to_s } @@ -26,7 +27,21 @@ module RuboCop end def finalized_by - self.class.dictionary_data.dig(queued_migration_version.to_s, :finalized_by) + dictionary_data&.dig(:finalized_by) + end + + def finalize_after + dictionary_data&.dig(:finalize_after) + end + + def introduced_by_url + dictionary_data&.dig(:introduced_by_url) + end + + private + + def dictionary_data + @dictionary_data ||= self.class.dictionary_data[queued_migration_version.to_s] end end end diff --git a/rubocop/cop/background_migration/missing_dictionary_file.rb b/rubocop/cop/background_migration/dictionary_file.rb index 9158b268bf9..9ae47260e79 100644 --- a/rubocop/cop/background_migration/missing_dictionary_file.rb +++ b/rubocop/cop/background_migration/dictionary_file.rb @@ -1,17 +1,23 @@ # frozen_string_literal: true require_relative '../../migration_helpers' +require_relative '../../batched_background_migrations_dictionary' module RuboCop module Cop module BackgroundMigration # Checks the batched background migration has the corresponding dictionary file - class MissingDictionaryFile < RuboCop::Cop::Base + class DictionaryFile < RuboCop::Cop::Base include MigrationHelpers - MSG = "Missing %{file_name}. " \ - "Use the generator 'batched_background_migration' to create dictionary files automatically. " \ - "For more details refer: https://docs.gitlab.com/ee/development/database/batched_background_migrations.html#generator" + MSG = { + missing_key: "Mandatory key '%{key}' is missing from the dictionary. Please add with an appropriate value.", + missing_dictionary: <<-MESSAGE.delete("\n").squeeze(' ').strip + Missing %{file_name}. + Use the generator 'batched_background_migration' to create dictionary files automatically. + For more details refer: https://docs.gitlab.com/ee/development/database/batched_background_migrations.html#generator + MESSAGE + }.freeze DICTIONARY_DIR = "db/docs/batched_background_migrations" @@ -35,9 +41,10 @@ module RuboCop migration_name_node.value end - return if dictionary_file?(migration_name) + error_code, msg_params = validate_dictionary_file(migration_name, node) + return unless error_code.present? - add_offense(node, message: format(MSG, file_name: dictionary_file_path(migration_name))) + add_offense(node, message: format(MSG[error_code], msg_params)) end private @@ -50,6 +57,18 @@ module RuboCop File.join(rails_root, DICTIONARY_DIR, "#{migration_class_name.underscore}.yml") end + def validate_dictionary_file(migration_name, node) + unless dictionary_file?(migration_name) + return [:missing_dictionary, { file_name: dictionary_file_path(migration_name) }] + end + + bbm_dictionary = RuboCop::BatchedBackgroundMigrationsDictionary.new(version(node)) + + return [:missing_key, { key: :finalize_after }] unless bbm_dictionary.finalize_after.present? + + return [:missing_key, { key: :introduced_by_url }] unless bbm_dictionary.introduced_by_url.present? + end + def rails_root @rails_root ||= File.expand_path('../../..', __dir__) end diff --git a/rubocop/cop/gitlab/avoid_gitlab_instance_checks.rb b/rubocop/cop/gitlab/avoid_gitlab_instance_checks.rb index 962a58cfb4a..6aac3649b04 100644 --- a/rubocop/cop/gitlab/avoid_gitlab_instance_checks.rb +++ b/rubocop/cop/gitlab/avoid_gitlab_instance_checks.rb @@ -17,7 +17,7 @@ module RuboCop # end # # # good - # if Gitlab::Saas.feature_available?('purchases/additional_minutes') + # if Gitlab::Saas.feature_available?(:purchases_additional_minutes) # Ci::Runner::FORM_EDITABLE + Ci::Runner::MINUTES_COST_FACTOR_FIELDS # else # Ci::Runner::FORM_EDITABLE diff --git a/rubocop/cop/gitlab/doc_url.rb b/rubocop/cop/gitlab/doc_url.rb index 41a1c2f8b36..79e25142f26 100644 --- a/rubocop/cop/gitlab/doc_url.rb +++ b/rubocop/cop/gitlab/doc_url.rb @@ -19,7 +19,7 @@ module RuboCop include RangeHelp MSG = 'Use `#help_page_url` instead of directly including link. ' \ - 'See https://docs.gitlab.com/ee/development/documentation/#linking-to-help-in-ruby.' + 'See https://docs.gitlab.com/ee/development/documentation/help#linking-to-help.' DOCS_URL_REGEXP = %r{https://docs.gitlab.com/ee/[\w#%./-]+} diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb index 65a1731fc28..4c6cc6c6778 100644 --- a/rubocop/cop/gitlab/mark_used_feature_flags.rb +++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb @@ -16,9 +16,6 @@ module RuboCop EXPERIMENT_METHODS = %i[ experiment ].freeze - RUGGED_METHODS = %i[ - use_rugged? - ].freeze WORKER_METHODS = %i[ data_consistency deduplicate @@ -28,7 +25,7 @@ module RuboCop push_force_frontend_feature_flag limit_feature_flag= limit_feature_flag_for_override= - ].freeze + EXPERIMENT_METHODS + RUGGED_METHODS + WORKER_METHODS + ].freeze + EXPERIMENT_METHODS + WORKER_METHODS RESTRICT_ON_SEND = FEATURE_METHODS + SELF_METHODS @@ -119,7 +116,7 @@ module RuboCop pair.key.value == :feature_flag end&.value else - arg_index = rugged_method?(node) ? 3 : 2 + arg_index = 2 node.children[arg_index] end @@ -156,10 +153,6 @@ module RuboCop class_caller(node) == "Feature::Gitaly" end - def rugged_method?(node) - RUGGED_METHODS.include?(method_name(node)) - end - def feature_method?(node) FEATURE_METHODS.include?(method_name(node)) && (caller_is_feature?(node) || caller_is_feature_gitaly?(node)) end diff --git a/rubocop/cop/migration/migration_with_milestone.rb b/rubocop/cop/migration/migration_with_milestone.rb new file mode 100644 index 00000000000..d06850dee5b --- /dev/null +++ b/rubocop/cop/migration/migration_with_milestone.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Migration + # Cop that checks that any 2.2+ migration is incldued with a call to 'milestone' + class MigrationWithMilestone < RuboCop::Cop::Base + MSG = 'Version 2.2 migrations must specify a milestone.' + + def_node_matcher :gitlab_migration?, <<-PATTERN + (class (const nil? _) (send (const (const (const nil? :Gitlab) :Database) :Migration) :[] (float $_)) ...) + PATTERN + + def_node_search :milestone_call?, '(begin <(send nil? :milestone (str $_)) ...>)' + + def on_class(node) + version = gitlab_migration?(node) + return unless version && version >= 2.2 + + body_node = node.body + return unless body_node + + add_offense(node, message: MSG) unless milestone_call?(body_node) + end + end + end + end +end diff --git a/rubocop/cop/migration/prevent_index_creation.rb b/rubocop/cop/migration/prevent_index_creation.rb index aa0ab7b1e50..0a1dccccbc8 100644 --- a/rubocop/cop/migration/prevent_index_creation.rb +++ b/rubocop/cop/migration/prevent_index_creation.rb @@ -8,11 +8,11 @@ module RuboCop class PreventIndexCreation < RuboCop::Cop::Base include MigrationHelpers - FORBIDDEN_TABLES = %i[ci_builds namespaces].freeze + FORBIDDEN_TABLES = %i[ci_builds namespaces projects users].freeze MSG = "Adding new index to #{FORBIDDEN_TABLES.join(", ")} is forbidden. " \ "For `ci_builds` see https://gitlab.com/gitlab-org/gitlab/-/issues/332886, " \ - "for `namespaces` see https://gitlab.com/groups/gitlab-org/-/epics/11543".freeze + "for `namespaces`, `projects`, and `users` see https://gitlab.com/groups/gitlab-org/-/epics/11543".freeze def on_new_investigation super diff --git a/rubocop/cop/migration/unfinished_dependencies.rb b/rubocop/cop/migration/unfinished_dependencies.rb index 1e0741c8411..56ba7d405c5 100644 --- a/rubocop/cop/migration/unfinished_dependencies.rb +++ b/rubocop/cop/migration/unfinished_dependencies.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require_relative '../../migration_helpers' -require_relative '../../batched_background_migrations' +require_relative '../../batched_background_migrations_dictionary' module RuboCop module Cop @@ -43,7 +43,7 @@ module RuboCop private def fetch_finalized_by(queued_migration_version) - BatchedBackgroundMigrations.new(queued_migration_version).finalized_by + BatchedBackgroundMigrationsDictionary.new(queued_migration_version).finalized_by end end end diff --git a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb index 1b0d5ed9324..e019e5bd0cf 100644 --- a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb +++ b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb @@ -29,6 +29,14 @@ module RuboCop index_exists? column_exists? create_trigger_to_sync_tables + lock_tables + swap_columns + swap_columns_default + swap_foreign_keys + swap_indexes + reset_trigger_function + cleanup_conversion_of_integer_to_bigint + revert_initialize_conversion_of_integer_to_bigint ].sort.freeze MSG = "The method is not allowed to be called within the `with_lock_retries` block, the only allowed methods are: #{ALLOWED_MIGRATION_METHODS.join(', ')}".freeze diff --git a/rubocop/cop/style/inline_disable_annotation.rb b/rubocop/cop/style/inline_disable_annotation.rb new file mode 100644 index 00000000000..c3db541fe82 --- /dev/null +++ b/rubocop/cop/style/inline_disable_annotation.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # rubocop:disable Lint/RedundantCopDisableDirective -- For examples + # Checks that rubocop inline disabling is formatted according + # to guidelines. + # See: https://docs.gitlab.com/ee/development/rubocop_development_guide.html#disabling-rules-inline, + # https://gitlab.com/gitlab-org/gitlab/-/issues/428762 + # + # # bad + # # rubocop:disable Some/Cop, Another/Cop + # + # # good + # # rubocop:disable Some/Cop, Another/Cop -- Some reason + # + # rubocop:enable Lint/RedundantCopDisableDirective + class InlineDisableAnnotation < RuboCop::Cop::Base + include RangeHelp + + COP_DISABLE = '#\s*rubocop\s*:\s*(?:disable|todo)\s+' + BAD_DISABLE = %r{\A(?<line>(?<disabling>#{COP_DISABLE}(?:[\w/]+(?:\s*,\s*[\w/]+)*))\s*)(?!.*\s*--\s\S).*} + COP_DISABLE_LINE = /\A(?<line>#{COP_DISABLE}.*)\Z/ + MSG = <<~MESSAGE + Inline disabling a cop needs to follow the format of `%{disable} -- Some reason`. + See https://docs.gitlab.com/ee/development/rubocop_development_guide.html#disabling-rules-inline. + MESSAGE + + def on_new_investigation + processed_source.comments.each do |comment| + candidate_match = COP_DISABLE_LINE.match(comment.text) + # Pre-filter to ensure we are on a comment that is for a rubocop disabling + next unless candidate_match + + bad_match = BAD_DISABLE.match(comment.text) + # Only the badly formatted lines make it past this. + next unless bad_match + + add_offense( + source_range( + processed_source.buffer, comment.loc.line, comment.loc.column, candidate_match[:line].length + ), + message: format(MSG, disable: bad_match[:disabling]) + ) + end + end + end + end + end +end diff --git a/rubocop/rubocop-code_reuse.yml b/rubocop/rubocop-code_reuse.yml index f96de5caf99..2bd3339368d 100644 --- a/rubocop/rubocop-code_reuse.yml +++ b/rubocop/rubocop-code_reuse.yml @@ -24,6 +24,7 @@ CodeReuse/ActiveRecord: - danger/**/*.rb - lib/backup/**/*.rb - lib/banzai/**/*.rb + - lib/click_house/migration_support/**/*.rb - lib/gitlab/background_migration/**/*.rb - lib/gitlab/cycle_analytics/**/*.rb - lib/gitlab/counters/**/*.rb diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 42882966b85..708be988f3a 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,7 +1,11 @@ # rubocop:disable Naming/FileName # frozen_string_literal: true +# Load ActiveSupport to ensure that core extensions like `Enumerable#exclude?` +# are available in cop rules like `Performance/CollectionLiteralInLoop`. +require 'active_support/all' + # Auto-require all cops under `rubocop/cop/**/*.rb` -Dir[File.join(__dir__, 'cop', '**', '*.rb')].sort.each { |file| require file } +Dir[File.join(__dir__, 'cop', '**', '*.rb')].each { |file| require file } # rubocop:enable Naming/FileName |