diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-20 13:43:29 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-20 13:43:29 +0300 |
commit | 3b1af5cc7ed2666ff18b718ce5d30fa5a2756674 (patch) | |
tree | 3bc4a40e0ee51ec27eabf917c537033c0c5b14d4 /rubocop | |
parent | 9bba14be3f2c211bf79e15769cd9b77bc73a13bc (diff) |
Add latest changes from gitlab-org/gitlab@16-1-stable-eev16.1.0-rc42
Diffstat (limited to 'rubocop')
19 files changed, 222 insertions, 42 deletions
diff --git a/rubocop/cop/background_migration/feature_category.rb b/rubocop/cop/background_migration/feature_category.rb index ec70b5baadf..c6611a65e49 100644 --- a/rubocop/cop/background_migration/feature_category.rb +++ b/rubocop/cop/background_migration/feature_category.rb @@ -17,7 +17,7 @@ module RuboCop "https://docs.gitlab.com/ee/development/feature_categorization/#batched-background-migrations" INVALID_FEATURE_CATEGORY_MSG = "'feature_category' is invalid. " \ - "List of valid ones can be found in #{FEATURE_CATEGORIES_FILE_PATH}" + "List of valid ones can be found in #{FEATURE_CATEGORIES_FILE_PATH}".freeze RESTRICT_ON_SEND = [:feature_category].freeze diff --git a/rubocop/cop/filename_length.rb b/rubocop/cop/filename_length.rb index 948f69cc88c..4144337f00f 100644 --- a/rubocop/cop/filename_length.rb +++ b/rubocop/cop/filename_length.rb @@ -7,8 +7,8 @@ module RuboCop FILEPATH_MAX_BYTES = 256 FILENAME_MAX_BYTES = 100 - MSG_FILEPATH_LEN = "This file path is too long. It should be #{FILEPATH_MAX_BYTES} or less" - MSG_FILENAME_LEN = "This file name is too long. It should be #{FILENAME_MAX_BYTES} or less" + MSG_FILEPATH_LEN = "This file path is too long. It should be #{FILEPATH_MAX_BYTES} or less".freeze + MSG_FILENAME_LEN = "This file name is too long. It should be #{FILENAME_MAX_BYTES} or less".freeze def on_new_investigation file_path = processed_source.file_path diff --git a/rubocop/cop/gitlab/event_store_subscriber.rb b/rubocop/cop/gitlab/event_store_subscriber.rb index 7e4cc3e66cc..04dc29ec400 100644 --- a/rubocop/cop/gitlab/event_store_subscriber.rb +++ b/rubocop/cop/gitlab/event_store_subscriber.rb @@ -32,8 +32,8 @@ module RuboCop # class EventStoreSubscriber < RuboCop::Cop::Base SUBSCRIBER_MODULE_NAME = 'Gitlab::EventStore::Subscriber' - FORBID_PERFORM_OVERRIDE = "Do not override `perform` in a `#{SUBSCRIBER_MODULE_NAME}`." - REQUIRE_HANDLE_EVENT = "A `#{SUBSCRIBER_MODULE_NAME}` must implement `#handle_event(event)`." + FORBID_PERFORM_OVERRIDE = "Do not override `perform` in a `#{SUBSCRIBER_MODULE_NAME}`.".freeze + REQUIRE_HANDLE_EVENT = "A `#{SUBSCRIBER_MODULE_NAME}` must implement `#handle_event(event)`.".freeze def_node_matcher :includes_subscriber?, <<~PATTERN (send nil? :include (const (const (const nil? :Gitlab) :EventStore) :Subscriber)) diff --git a/rubocop/cop/gitlab/feature_available_usage.rb b/rubocop/cop/gitlab/feature_available_usage.rb index df4409c27e0..cbfb89d3053 100644 --- a/rubocop/cop/gitlab/feature_available_usage.rb +++ b/rubocop/cop/gitlab/feature_available_usage.rb @@ -28,6 +28,7 @@ module RuboCop feature_flags releases infrastructure + model_experiments ].freeze EE_FEATURES = %i[requirements].freeze ALL_FEATURES = (FEATURES + EE_FEATURES).freeze diff --git a/rubocop/cop/graphql/descriptions.rb b/rubocop/cop/graphql/descriptions.rb index 239f5b966a4..b096dfb148e 100644 --- a/rubocop/cop/graphql/descriptions.rb +++ b/rubocop/cop/graphql/descriptions.rb @@ -51,11 +51,12 @@ module RuboCop extend RuboCop::Cop::AutoCorrector MSG_STYLE_GUIDE_LINK = 'See the description style guide: https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#description-style-guide' - MSG_NO_DESCRIPTION = "Please add a `description` property. #{MSG_STYLE_GUIDE_LINK}" - MSG_NO_PERIOD = "`description` strings must end with a `.`. #{MSG_STYLE_GUIDE_LINK}" - MSG_BAD_START = "`description` strings should not start with \"A...\" or \"The...\". #{MSG_STYLE_GUIDE_LINK}" + MSG_NO_DESCRIPTION = "Please add a `description` property. #{MSG_STYLE_GUIDE_LINK}".freeze + MSG_NO_PERIOD = "`description` strings must end with a `.`. #{MSG_STYLE_GUIDE_LINK}".freeze + MSG_BAD_START = "`description` strings should not start with \"A...\" or \"The...\"."\ + " #{MSG_STYLE_GUIDE_LINK}".freeze MSG_CONTAINS_THIS = "`description` strings should not contain the demonstrative \"this\"."\ - " #{MSG_STYLE_GUIDE_LINK}" + " #{MSG_STYLE_GUIDE_LINK}".freeze def_node_matcher :graphql_describable?, <<~PATTERN (send nil? {:field :argument :value} ...) diff --git a/rubocop/cop/graphql/enum_names.rb b/rubocop/cop/graphql/enum_names.rb index 74847cb8d17..19ba426fdeb 100644 --- a/rubocop/cop/graphql/enum_names.rb +++ b/rubocop/cop/graphql/enum_names.rb @@ -34,9 +34,9 @@ module RuboCop module Graphql class EnumNames < RuboCop::Cop::Base SEE_SG_MSG = "See https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#enums" - CLASS_NAME_SUFFIX_MSG = "Enum class names must end with `Enum`. #{SEE_SG_MSG}" - GRAPHQL_NAME_MISSING_MSG = "A `graphql_name` must be defined for a GraphQL enum. #{SEE_SG_MSG}" - GRAPHQL_NAME_WITH_ENUM_MSG = "The `graphql_name` must not contain the string \"Enum\". #{SEE_SG_MSG}" + CLASS_NAME_SUFFIX_MSG = "Enum class names must end with `Enum`. #{SEE_SG_MSG}".freeze + GRAPHQL_NAME_MISSING_MSG = "A `graphql_name` must be defined for a GraphQL enum. #{SEE_SG_MSG}".freeze + GRAPHQL_NAME_WITH_ENUM_MSG = "The `graphql_name` must not contain the string \"Enum\". #{SEE_SG_MSG}".freeze def_node_matcher :enum_subclass, <<~PATTERN (class $(const nil? _) (const {nil? cbase} /.*Enum$/) ...) diff --git a/rubocop/cop/graphql/resource_not_available_error.rb b/rubocop/cop/graphql/resource_not_available_error.rb new file mode 100644 index 00000000000..d759e145008 --- /dev/null +++ b/rubocop/cop/graphql/resource_not_available_error.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require_relative '../../node_pattern_helper' + +module RuboCop + module Cop + module Graphql + # Encourages the use of `raise_resource_not_available_error!` method + # instead of `raise Gitlab::Graphql::Errors::ResourceNotAvailable`. + # + # @example + # + # # bad + # raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'message' + # + # # good + # raise_resource_not_available_error! 'message' + class ResourceNotAvailableError < Base + extend NodePatternHelper + extend AutoCorrector + + MSG = 'Prefer using `raise_resource_not_available_error!` instead.' + + EXCEPTION = 'Gitlab::Graphql::Errors::ResourceNotAvailable' + + RESTRICT_ON_SEND = %i[raise].freeze + + def_node_matcher :error, const_pattern(EXCEPTION) + + def_node_matcher :raise_error, <<~PATTERN + (send nil? :raise #error $...) + PATTERN + + def on_send(node) + raise_error(node) do |args| + add_offense(node) do |corrector| + replacement = +'raise_resource_not_available_error!' + replacement << " #{args.map(&:source).join(', ')}" if args.any? + + corrector.replace(node, replacement) + end + end + end + end + end + end +end diff --git a/rubocop/cop/ignored_columns.rb b/rubocop/cop/ignored_columns.rb index 542d78c59e9..8b17447c46b 100644 --- a/rubocop/cop/ignored_columns.rb +++ b/rubocop/cop/ignored_columns.rb @@ -2,40 +2,60 @@ module RuboCop module Cop - # Cop that blacklists the usage of `ActiveRecord::Base.ignored_columns=` directly + # Cop that flags the usage of `ActiveRecord::Base.ignored_columns=` directly + # + # @example + # # bad + # class User < ApplicationRecord + # self.ignored_columns = [:name] + # self.ignored_columns += [:full_name] + # end + # + # # good + # class User < ApplicationRecord + # include IgnorableColumns + # + # ignore_column :name, remove_after: '2023-05-22', remove_with: '16.0' + # ignore_column :full_name, remove_after: '2023-05-22', remove_with: '16.0' + # end class IgnoredColumns < RuboCop::Cop::Base - USE_CONCERN_MSG = 'Use `IgnoredColumns` concern instead of adding to `self.ignored_columns`.' - WRONG_MODEL_MSG = 'If the model exists in CE and EE, the column has to be ignored ' \ - 'in the CE model. If the model only exists in EE, then it has to be added there.' + USE_CONCERN_ADD_MSG = 'Use `IgnoredColumns` concern instead of adding to `self.ignored_columns`.' + USE_CONCERN_SET_MSG = 'Use `IgnoredColumns` concern instead of setting `self.ignored_columns`.' + WRONG_MODEL_MSG = <<~MSG + If the model exists in CE and EE, the column has to be ignored + in the CE model. If the model only exists in EE, then it has to be added there. + MSG - def_node_matcher :ignored_columns?, <<~PATTERN + RESTRICT_ON_SEND = %i[ignored_columns ignored_columns= ignore_column ignore_columns].freeze + + def_node_matcher :ignored_columns_add?, <<~PATTERN (send (self) :ignored_columns) PATTERN - def_node_matcher :ignore_columns?, <<~PATTERN - (send nil? :ignore_columns ...) + def_node_matcher :ignored_columns_set?, <<~PATTERN + (send (self) :ignored_columns= ...) PATTERN - def_node_matcher :ignore_column?, <<~PATTERN - (send nil? :ignore_column ...) + def_node_matcher :using_ignore_columns?, <<~PATTERN + (send nil? {:ignore_columns :ignore_column}...) PATTERN def on_send(node) - if ignored_columns?(node) - add_offense(node, message: USE_CONCERN_MSG) + if ignored_columns_add?(node) + add_offense(node.loc.selector, message: USE_CONCERN_ADD_MSG) + end + + if ignored_columns_set?(node) + add_offense(node.loc.selector, message: USE_CONCERN_SET_MSG) end - if using_ignore?(node) && used_in_wrong_model? + if using_ignore_columns?(node) && used_in_wrong_model? add_offense(node, message: WRONG_MODEL_MSG) end end private - def using_ignore?(node) - ignore_columns?(node) || ignore_column?(node) - end - def used_in_wrong_model? file_path = processed_source.file_path diff --git a/rubocop/cop/migration/prevent_index_creation.rb b/rubocop/cop/migration/prevent_index_creation.rb index 185f36cb24b..4c39032eb0f 100644 --- a/rubocop/cop/migration/prevent_index_creation.rb +++ b/rubocop/cop/migration/prevent_index_creation.rb @@ -10,7 +10,7 @@ module RuboCop FORBIDDEN_TABLES = %i[ci_builds].freeze - MSG = "Adding new index to #{FORBIDDEN_TABLES.join(", ")} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886" + MSG = "Adding new index to #{FORBIDDEN_TABLES.join(", ")} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886".freeze def on_new_investigation super diff --git a/rubocop/cop/migration/schema_addition_methods_no_post.rb b/rubocop/cop/migration/schema_addition_methods_no_post.rb index 5bb5bb63f0c..53b47dc99dc 100644 --- a/rubocop/cop/migration/schema_addition_methods_no_post.rb +++ b/rubocop/cop/migration/schema_addition_methods_no_post.rb @@ -5,7 +5,7 @@ require_relative '../../migration_helpers' module RuboCop module Cop module Migration - # Cop that checks that no background batched migration helpers are called by regular migrations. + # Cop that checks that no schema migration methods are called by post-deployment migrations. class SchemaAdditionMethodsNoPost < RuboCop::Cop::Base include MigrationHelpers @@ -22,13 +22,25 @@ module RuboCop (send nil? {#{SYMBOLIZED_MATCHER}} ...) PATTERN + def_node_matcher :rolling_back_migration, <<~PATTERN + (def :down ...) + PATTERN + def on_send(node) return unless time_enforced?(node) + return if rolling_back_migration?(node) + on_forbidden_method(node) do add_offense(node, message: MSG) end end + + private + + def rolling_back_migration?(node) + node.each_ancestor(:def).any? { |a| rolling_back_migration(a) } + end end end end diff --git a/rubocop/cop/migration/update_column_in_batches.rb b/rubocop/cop/migration/update_column_in_batches.rb index 7f4479c62e3..e97f51361a9 100644 --- a/rubocop/cop/migration/update_column_in_batches.rb +++ b/rubocop/cop/migration/update_column_in_batches.rb @@ -10,20 +10,24 @@ module RuboCop class UpdateColumnInBatches < RuboCop::Cop::Base include MigrationHelpers - MSG = 'Migration running `update_column_in_batches` must have a spec file at' \ - ' `%s`.' + MSG = 'Migration running `update_column_in_batches` must have a spec file at `%s`.' + + RESTRICT_ON_SEND = %i[update_column_in_batches].freeze def on_send(node) return unless in_migration?(node) - return unless node.children[1] == :update_column_in_batches spec_path = spec_filename(node) - return if File.exist?(File.expand_path(spec_path, rails_root)) add_offense(node, message: format(MSG, spec_path)) end + # Used by RuboCop to invalidate its cache if specs change. + def external_dependency_checksum + @external_dependency_checksum ||= checksum_filenames('{,ee/}spec/migrations/**/*_spec.rb') + end + private def spec_filename(node) @@ -39,6 +43,16 @@ module RuboCop def rails_root Pathname.new(File.expand_path('../../..', __dir__)) end + + def checksum_filenames(pattern) + digest = Digest::SHA256.new + + rails_root.glob(pattern) do |path| + digest.update(path.relative_path_from(rails_root).to_path) + end + + digest.hexdigest + end end end end diff --git a/rubocop/cop/migration/versioned_migration_class.rb b/rubocop/cop/migration/versioned_migration_class.rb index 27f1c0e16a1..d078acedb37 100644 --- a/rubocop/cop/migration/versioned_migration_class.rb +++ b/rubocop/cop/migration/versioned_migration_class.rb @@ -12,10 +12,10 @@ module RuboCop DOC_LINK = "https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning" MSG_INHERIT = "Don't inherit from ActiveRecord::Migration or old versions of Gitlab::Database::Migration. " \ - "Use Gitlab::Database::Migration[2.1] instead. See #{DOC_LINK}." + "Use Gitlab::Database::Migration[2.1] instead. See #{DOC_LINK}.".freeze MSG_INCLUDE = "Don't include migration helper modules directly. " \ - "Inherit from Gitlab::Database::Migration[2.1] instead. See #{DOC_LINK}." + "Inherit from Gitlab::Database::Migration[2.1] instead. See #{DOC_LINK}.".freeze GITLAB_MIGRATION_CLASS = 'Gitlab::Database::Migration' ACTIVERECORD_MIGRATION_CLASS = 'ActiveRecord::Migration' diff --git a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb index 5c96b38dcdc..1b0d5ed9324 100644 --- a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb +++ b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb @@ -31,7 +31,7 @@ module RuboCop create_trigger_to_sync_tables ].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(', ')}" + 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 MSG_ONLY_ONE_FK_ALLOWED = "Avoid adding more than one foreign key within the `with_lock_retries`. See https://docs.gitlab.com/ee/development/migration_style_guide.html#examples" def_node_matcher :send_node?, <<~PATTERN diff --git a/rubocop/cop/rspec/avoid_conditional_statements.rb b/rubocop/cop/rspec/avoid_conditional_statements.rb index d03c3fb5d04..c24133f6469 100644 --- a/rubocop/cop/rspec/avoid_conditional_statements.rb +++ b/rubocop/cop/rspec/avoid_conditional_statements.rb @@ -7,6 +7,8 @@ module RuboCop module RSpec # This cop checks for the usage of conditional statements in specs. # + # https://gitlab.com/gitlab-org/gitlab/-/issues/410138 + # # @example # # # bad diff --git a/rubocop/cop/rspec/factory_bot/local_static_assignment.rb b/rubocop/cop/rspec/factory_bot/local_static_assignment.rb new file mode 100644 index 00000000000..1a26bc31c78 --- /dev/null +++ b/rubocop/cop/rspec/factory_bot/local_static_assignment.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'rubocop-rspec' + +module RuboCop + module Cop + module RSpec + module FactoryBot + # Flags local assignments during factory "load time". This leads to + # static data definitions. + # + # Move these definitions into attribute block or + # `transient` block to ensure that the data is evaluated during + # "runtime" and remains dynamic. + # + # @example + # # bad + # factory :foo do + # random = rand(23) + # baz { "baz-#{random}" } + # + # trait :a_trait do + # random = rand(23) + # baz { "baz-#{random}" } + # end + # + # transient do + # random = rand(23) + # baz { "baz-#{random}" } + # end + # end + # + # # good + # factory :foo do + # baz { "baz-#{random}" } + # + # trait :a_trait do + # baz { "baz-#{random}" } + # end + # + # transient do + # random { rand(23) } + # end + # end + class LocalStaticAssignment < RuboCop::Cop::Base + MSG = 'Avoid local static assignemnts in factories which lead to static data definitions.' + + RESTRICT_ON_SEND = %i[factory transient trait].freeze + + def_node_search :local_assignment, <<~PATTERN + (begin $(lvasgn ...)) + PATTERN + + def on_send(node) + return unless node.parent&.block_type? + + node.parent.each_child_node(:begin) do |begin_node| + begin_node.each_child_node(:lvasgn) do |lvasgn_node| + add_offense(lvasgn_node) + end + end + end + end + end + end + end +end diff --git a/rubocop/cop/scalability/idempotent_worker.rb b/rubocop/cop/scalability/idempotent_worker.rb index 88b5d796def..fee9e952105 100644 --- a/rubocop/cop/scalability/idempotent_worker.rb +++ b/rubocop/cop/scalability/idempotent_worker.rb @@ -28,7 +28,7 @@ module RuboCop HELP_LINK = 'https://github.com/mperham/sidekiq/wiki/Best-Practices#2-make-your-job-idempotent-and-transactional' - MSG = <<~MSG + MSG = <<~MSG.freeze Avoid adding not idempotent workers. A worker is considered idempotent if: diff --git a/rubocop/cop/search/namespaced_class.rb b/rubocop/cop/search/namespaced_class.rb index 8824107ae61..7d0901cb6f6 100644 --- a/rubocop/cop/search/namespaced_class.rb +++ b/rubocop/cop/search/namespaced_class.rb @@ -20,8 +20,10 @@ module RuboCop # These namespaces are considered acceptable. # Note: Nested namespace like Foo::Bar are also supported. - PERMITTED_NAMESPACES = %w[Search EE::Search API::Search EE::API::Search RuboCop::Cop::Search] - .map { |x| x.split('::') }.freeze + PERMITTED_NAMESPACES = %w[ + Search EE::Search API::Search EE::API::Search API::Admin::Search RuboCop::Cop::Search + API::Entities::Search::Zoekt + ].map { |x| x.split('::') }.freeze SEARCH_REGEXES = [ /elastic/i, diff --git a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb index badd81ff138..1d421a5d017 100644 --- a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb +++ b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb @@ -28,13 +28,13 @@ module RuboCop HELP_LINK = 'https://docs.gitlab.com/ee/development/sidekiq/worker_attributes.html#job-data-consistency-strategies' - MISSING_DATA_CONSISTENCY_MSG = <<~MSG + MISSING_DATA_CONSISTENCY_MSG = <<~MSG.freeze Should define data_consistency expectation. See #{HELP_LINK} for a more detailed explanation of these settings. MSG DISCOURAGE_ALWAYS_MSG = "Refrain from using `:always` if possible." \ - "See #{HELP_LINK} for a more detailed explanation of these settings." + "See #{HELP_LINK} for a more detailed explanation of these settings.".freeze def_node_search :application_worker?, <<~PATTERN `(send nil? :include (const nil? :ApplicationWorker)) diff --git a/rubocop/node_pattern_helper.rb b/rubocop/node_pattern_helper.rb new file mode 100644 index 00000000000..ecd4560763c --- /dev/null +++ b/rubocop/node_pattern_helper.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module RuboCop + module NodePatternHelper + # Returns a nested `(const ...)` node pattern for a full qualified +name+. + # + # @examples + # const_pattern 'Foo::Bar' # => (const (const {nil? cbase} :Foo) :Bar) + # const_pattern 'Foo::Bar', parent: ':Baz' # => (const (const :Baz :Foo) :Bar) + def const_pattern(name, parent: '{nil? cbase}') + name.split('::').inject(parent) { |memo, name_part| "(const #{memo} :#{name_part})" } + end + end +end |