diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-04-20 23:50:22 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-04-20 23:50:22 +0000 |
commit | 9dc93a4519d9d5d7be48ff274127136236a3adb3 (patch) | |
tree | 70467ae3692a0e35e5ea56bcb803eb512a10bedb /rubocop | |
parent | 4b0f34b6d759d6299322b3a54453e930c6121ff0 (diff) | |
download | gitlab-ce-9dc93a4519d9d5d7be48ff274127136236a3adb3.tar.gz |
Add latest changes from gitlab-org/gitlab@13-11-stable-eev13.11.0-rc43
Diffstat (limited to 'rubocop')
39 files changed, 251 insertions, 34 deletions
diff --git a/rubocop/cop/avoid_keyword_arguments_in_sidekiq_workers.rb b/rubocop/cop/avoid_keyword_arguments_in_sidekiq_workers.rb index e1c6a984e75..da3cac073ad 100644 --- a/rubocop/cop/avoid_keyword_arguments_in_sidekiq_workers.rb +++ b/rubocop/cop/avoid_keyword_arguments_in_sidekiq_workers.rb @@ -5,7 +5,7 @@ module RuboCop # Cop that blacklists keyword arguments usage in Sidekiq workers class AvoidKeywordArgumentsInSidekiqWorkers < RuboCop::Cop::Cop MSG = "Do not use keyword arguments in Sidekiq workers. " \ - "For details, check https://github.com/mperham/sidekiq/issues/2372".freeze + "For details, check https://github.com/mperham/sidekiq/issues/2372" OBSERVED_METHOD = :perform def on_def(node) diff --git a/rubocop/cop/gitlab/change_timzone.rb b/rubocop/cop/gitlab/change_timzone.rb index 63e6dd411f3..c30a057d51c 100644 --- a/rubocop/cop/gitlab/change_timzone.rb +++ b/rubocop/cop/gitlab/change_timzone.rb @@ -5,7 +5,7 @@ module RuboCop module Gitlab class ChangeTimezone < RuboCop::Cop::Cop MSG = "Do not change timezone in the runtime (application or rspec), " \ - "it could result in silently modifying other behavior.".freeze + "it could result in silently modifying other behavior." def_node_matcher :changing_timezone?, <<~PATTERN (send (const nil? :Time) :zone= ...) diff --git a/rubocop/cop/gitlab/delegate_predicate_methods.rb b/rubocop/cop/gitlab/delegate_predicate_methods.rb new file mode 100644 index 00000000000..43b5184faab --- /dev/null +++ b/rubocop/cop/gitlab/delegate_predicate_methods.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # This cop looks for delegations to predicate methods with `allow_nil: true` option. + # This construct results in three possible results: true, false and nil. + # In other words, it does not preserve the strict Boolean nature of predicate method return value. + # This cop suggests creating a method to handle `nil` delegator and ensure only Boolean type is returned. + # + # @example + # # bad + # delegate :is_foo?, to: :bar, allow_nil: true + # + # # good + # def is_foo? + # return false unless bar + # bar.is_foo? + # end + # + # def is_foo? + # !!bar&.is_foo? + # end + class DelegatePredicateMethods < RuboCop::Cop::Cop + MSG = "Using `delegate` with `allow_nil` on the following predicate methods is discouraged: %s." + RESTRICT_ON_SEND = %i[delegate].freeze + def_node_matcher :predicate_allow_nil_option, <<~PATTERN + (send nil? :delegate + (sym $_)* + (hash <$(pair (sym :allow_nil) true) ...>) + ) + PATTERN + + def on_send(node) + predicate_allow_nil_option(node) do |delegated_methods, _options| + offensive_methods = delegated_methods.select { |method| method.end_with?('?') } + next if offensive_methods.empty? + + add_offense(node, message: format(MSG, offensive_methods.join(', '))) + end + end + end + end + end +end diff --git a/rubocop/cop/gitlab/feature_available_usage.rb b/rubocop/cop/gitlab/feature_available_usage.rb new file mode 100644 index 00000000000..b50bdd8ca43 --- /dev/null +++ b/rubocop/cop/gitlab/feature_available_usage.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Cop that checks for correct calling of #feature_available? + class FeatureAvailableUsage < RuboCop::Cop::Cop + OBSERVED_METHOD = :feature_available? + LICENSED_FEATURE_LITERAL_ARG_MSG = '`feature_available?` should not be called for features that can be licensed (`%s` given), use `licensed_feature_available?(feature)` instead.' + LICENSED_FEATURE_DYNAMIC_ARG_MSG = "`feature_available?` should not be called for features that can be licensed (`%s` isn't a literal so we cannot say if it's legit or not), using `licensed_feature_available?(feature)` may be more appropriate." + NOT_ENOUGH_ARGS_MSG = '`feature_available?` should be called with two arguments: `feature` and `user`.' + FEATURES = %i[ + issues + forking + merge_requests + wiki + snippets + builds + repository + pages + metrics_dashboard + analytics + operations + security_and_compliance + container_registry + ].freeze + EE_FEATURES = %i[requirements].freeze + ALL_FEATURES = (FEATURES + EE_FEATURES).freeze + SPECIAL_CLASS = %w[License].freeze + + def on_send(node) + return unless method_name(node) == OBSERVED_METHOD + return if caller_is_special_case?(node) + return if feature_name(node).nil? + return if ALL_FEATURES.include?(feature_name(node)) && args_count(node) == 2 + + if !ALL_FEATURES.include?(feature_name(node)) + add_offense(node, location: :expression, message: licensed_feature_message(node)) + elsif args_count(node) < 2 + add_offense(node, location: :expression, message: NOT_ENOUGH_ARGS_MSG) + end + end + + def licensed_feature_message(node) + message_template = dynamic_feature?(node) ? LICENSED_FEATURE_DYNAMIC_ARG_MSG : LICENSED_FEATURE_LITERAL_ARG_MSG + + message_template % feature_arg_name(node) + end + + private + + def method_name(node) + node.children[1] + end + + def class_caller(node) + node.children[0]&.const_name.to_s + end + + def caller_is_special_case?(node) + SPECIAL_CLASS.include?(class_caller(node)) + end + + def feature_arg(node) + node.children[2] + end + + def dynamic_feature?(node) + feature = feature_arg(node) + return false unless feature + + !feature.literal? + end + + def feature_name(node) + feature = feature_arg(node) + return unless feature + return feature.children.compact.join('.') if dynamic_feature?(node) + return feature.value if feature.respond_to?(:value) + + feature.type + end + + def feature_arg_name(node) + feature = feature_arg(node) + return unless feature + return feature.children.compact.join('.') if dynamic_feature?(node) + return feature.children[0].inspect if feature.literal? + + feature.type + end + + def args_count(node) + node.children[2..].size + end + end + end + end +end diff --git a/rubocop/cop/gitlab/rails_logger.rb b/rubocop/cop/gitlab/rails_logger.rb index ad35d2ccfbb..5a1695ce56e 100644 --- a/rubocop/cop/gitlab/rails_logger.rb +++ b/rubocop/cop/gitlab/rails_logger.rb @@ -21,7 +21,7 @@ module RuboCop # # OK # Rails.logger.level MSG = 'Use a structured JSON logger instead of `Rails.logger`. ' \ - 'https://docs.gitlab.com/ee/development/logging.html'.freeze + 'https://docs.gitlab.com/ee/development/logging.html' # See supported log methods: # https://ruby-doc.org/stdlib-2.6.6/libdoc/logger/rdoc/Logger.html diff --git a/rubocop/cop/graphql/descriptions.rb b/rubocop/cop/graphql/descriptions.rb index ec233c65874..520e34dcd16 100644 --- a/rubocop/cop/graphql/descriptions.rb +++ b/rubocop/cop/graphql/descriptions.rb @@ -54,6 +54,10 @@ module RuboCop (send nil? :value ...) PATTERN + def_node_matcher :resolver_kwarg, <<~PATTERN + (... (hash <(pair (sym :resolver) $_) ...>)) + PATTERN + def_node_matcher :description_kwarg, <<~PATTERN (... (hash <(pair (sym :description) $_) ...>)) PATTERN @@ -64,6 +68,7 @@ module RuboCop def on_send(node) return unless graphql_describable?(node) + return if resolver_kwarg(node) # Fields may inherit the description from their resolvers. description = locate_description(node) diff --git a/rubocop/cop/graphql/json_type.rb b/rubocop/cop/graphql/json_type.rb index 1e3e3d7a7ff..a8c38358535 100644 --- a/rubocop/cop/graphql/json_type.rb +++ b/rubocop/cop/graphql/json_type.rb @@ -20,7 +20,7 @@ module RuboCop module Graphql class JSONType < RuboCop::Cop::Cop MSG = 'Avoid using GraphQL::Types::JSON. See: ' \ - 'https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#json'.freeze + 'https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#json' def_node_matcher :has_json_type?, <<~PATTERN (send nil? {:field :argument} diff --git a/rubocop/cop/include_sidekiq_worker.rb b/rubocop/cop/include_sidekiq_worker.rb index e69bc018add..e39b8bf92c2 100644 --- a/rubocop/cop/include_sidekiq_worker.rb +++ b/rubocop/cop/include_sidekiq_worker.rb @@ -4,7 +4,7 @@ module RuboCop module Cop # Cop that makes sure workers include `ApplicationWorker`, not `Sidekiq::Worker`. class IncludeSidekiqWorker < RuboCop::Cop::Cop - MSG = 'Include `ApplicationWorker`, not `Sidekiq::Worker`.'.freeze + MSG = 'Include `ApplicationWorker`, not `Sidekiq::Worker`.' def_node_matcher :includes_sidekiq_worker?, <<~PATTERN (send nil? :include (const (const nil? :Sidekiq) :Worker)) diff --git a/rubocop/cop/lint/last_keyword_argument.rb b/rubocop/cop/lint/last_keyword_argument.rb index 9652c1ace8d..80f4660eeb8 100644 --- a/rubocop/cop/lint/last_keyword_argument.rb +++ b/rubocop/cop/lint/last_keyword_argument.rb @@ -10,7 +10,7 @@ module RuboCop # 1. Downloading the complete set of deprecations/ files from a CI # pipeline (see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47720) class LastKeywordArgument < Cop - MSG = 'Using the last argument as keyword parameters is deprecated'.freeze + MSG = 'Using the last argument as keyword parameters is deprecated' DEPRECATIONS_GLOB = File.expand_path('../../../deprecations/**/*.yml', __dir__) KEYWORD_DEPRECATION_STR = 'maybe ** should be added to the call' diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/add_column_with_default.rb index 355319b0dfe..afd7d93cd47 100644 --- a/rubocop/cop/migration/add_column_with_default.rb +++ b/rubocop/cop/migration/add_column_with_default.rb @@ -8,7 +8,7 @@ module RuboCop class AddColumnWithDefault < RuboCop::Cop::Cop include MigrationHelpers - MSG = '`add_column_with_default` is deprecated, use `add_column` instead'.freeze + MSG = '`add_column_with_default` is deprecated, use `add_column` instead' def on_send(node) return unless in_migration?(node) diff --git a/rubocop/cop/migration/add_concurrent_foreign_key.rb b/rubocop/cop/migration/add_concurrent_foreign_key.rb index 957bd30af63..ebab6aa653e 100644 --- a/rubocop/cop/migration/add_concurrent_foreign_key.rb +++ b/rubocop/cop/migration/add_concurrent_foreign_key.rb @@ -10,7 +10,7 @@ module RuboCop class AddConcurrentForeignKey < RuboCop::Cop::Cop include MigrationHelpers - MSG = '`add_foreign_key` requires downtime, use `add_concurrent_foreign_key` instead'.freeze + MSG = '`add_foreign_key` requires downtime, use `add_concurrent_foreign_key` instead' def_node_matcher :false_node?, <<~PATTERN (false) diff --git a/rubocop/cop/migration/add_concurrent_index.rb b/rubocop/cop/migration/add_concurrent_index.rb index 510f98ce373..bfe7c15bfdf 100644 --- a/rubocop/cop/migration/add_concurrent_index.rb +++ b/rubocop/cop/migration/add_concurrent_index.rb @@ -11,7 +11,7 @@ module RuboCop include MigrationHelpers MSG = '`add_concurrent_index` is not reversible so you must manually define ' \ - 'the `up` and `down` methods in your migration class, using `remove_concurrent_index` in `down`'.freeze + 'the `up` and `down` methods in your migration class, using `remove_concurrent_index` in `down`' def on_send(node) return unless in_migration?(node) diff --git a/rubocop/cop/migration/add_index.rb b/rubocop/cop/migration/add_index.rb index 7415880e554..327e89fb040 100644 --- a/rubocop/cop/migration/add_index.rb +++ b/rubocop/cop/migration/add_index.rb @@ -9,7 +9,7 @@ module RuboCop class AddIndex < RuboCop::Cop::Cop include MigrationHelpers - MSG = '`add_index` requires downtime, use `add_concurrent_index` instead'.freeze + MSG = '`add_index` requires downtime, use `add_concurrent_index` instead' def on_def(node) return unless in_migration?(node) diff --git a/rubocop/cop/migration/add_limit_to_text_columns.rb b/rubocop/cop/migration/add_limit_to_text_columns.rb index 126e4e21f22..f45551e60a4 100644 --- a/rubocop/cop/migration/add_limit_to_text_columns.rb +++ b/rubocop/cop/migration/add_limit_to_text_columns.rb @@ -14,7 +14,7 @@ module RuboCop include MigrationHelpers MSG = 'Text columns should always have a limit set (255 is suggested). ' \ - 'You can add a limit to a `text` column by using `add_text_limit`'.freeze + 'You can add a limit to a `text` column by using `add_text_limit`' def_node_matcher :reverting?, <<~PATTERN (def :down ...) diff --git a/rubocop/cop/migration/add_timestamps.rb b/rubocop/cop/migration/add_timestamps.rb index d16e8b1f45b..01d3f01ef4f 100644 --- a/rubocop/cop/migration/add_timestamps.rb +++ b/rubocop/cop/migration/add_timestamps.rb @@ -9,7 +9,7 @@ module RuboCop class AddTimestamps < RuboCop::Cop::Cop include MigrationHelpers - MSG = 'Do not use `add_timestamps`, use `add_timestamps_with_timezone` instead'.freeze + MSG = 'Do not use `add_timestamps`, use `add_timestamps_with_timezone` instead' # Check methods. def on_send(node) diff --git a/rubocop/cop/migration/datetime.rb b/rubocop/cop/migration/datetime.rb index 51e0c3e5a22..c605c8e1b6e 100644 --- a/rubocop/cop/migration/datetime.rb +++ b/rubocop/cop/migration/datetime.rb @@ -9,7 +9,7 @@ module RuboCop class Datetime < RuboCop::Cop::Cop include MigrationHelpers - MSG = 'Do not use the `%s` data type, use `datetime_with_timezone` instead'.freeze + MSG = 'Do not use the `%s` data type, use `datetime_with_timezone` instead' # Check methods in table creation. def on_def(node) diff --git a/rubocop/cop/migration/hash_index.rb b/rubocop/cop/migration/hash_index.rb index dba202ef0e3..8becef891af 100644 --- a/rubocop/cop/migration/hash_index.rb +++ b/rubocop/cop/migration/hash_index.rb @@ -11,7 +11,7 @@ module RuboCop include MigrationHelpers MSG = 'hash indexes should be avoided at all costs since they are not ' \ - 'recorded in the PostgreSQL WAL, you should use a btree index instead'.freeze + 'recorded in the PostgreSQL WAL, you should use a btree index instead' NAMES = Set.new([:add_index, :index, :add_concurrent_index]).freeze diff --git a/rubocop/cop/migration/prevent_strings.rb b/rubocop/cop/migration/prevent_strings.rb index bfeabd2c78d..57e29bf74ae 100644 --- a/rubocop/cop/migration/prevent_strings.rb +++ b/rubocop/cop/migration/prevent_strings.rb @@ -11,7 +11,7 @@ module RuboCop MSG = 'Do not use the `string` data type, use `text` instead. ' \ 'Updating limits on strings requires downtime. This can be avoided ' \ - 'by using `text` and adding a limit with `add_text_limit`'.freeze + 'by using `text` and adding a limit with `add_text_limit`' def_node_matcher :reverting?, <<~PATTERN (def :down ...) diff --git a/rubocop/cop/migration/remove_column.rb b/rubocop/cop/migration/remove_column.rb index f63df71467c..6a171ac948f 100644 --- a/rubocop/cop/migration/remove_column.rb +++ b/rubocop/cop/migration/remove_column.rb @@ -10,7 +10,7 @@ module RuboCop class RemoveColumn < RuboCop::Cop::Cop include MigrationHelpers - MSG = '`remove_column` must only be used in post-deployment migrations'.freeze + MSG = '`remove_column` must only be used in post-deployment migrations' def on_def(node) def_method = node.children[0] diff --git a/rubocop/cop/migration/remove_concurrent_index.rb b/rubocop/cop/migration/remove_concurrent_index.rb index 8c2c6fb157e..30dd59d97bc 100644 --- a/rubocop/cop/migration/remove_concurrent_index.rb +++ b/rubocop/cop/migration/remove_concurrent_index.rb @@ -11,7 +11,7 @@ module RuboCop include MigrationHelpers MSG = '`remove_concurrent_index` is not reversible so you must manually define ' \ - 'the `up` and `down` methods in your migration class, using `add_concurrent_index` in `down`'.freeze + 'the `up` and `down` methods in your migration class, using `add_concurrent_index` in `down`' def on_send(node) return unless in_migration?(node) diff --git a/rubocop/cop/migration/remove_index.rb b/rubocop/cop/migration/remove_index.rb index 15c2f37b4b0..ca5d4af1520 100644 --- a/rubocop/cop/migration/remove_index.rb +++ b/rubocop/cop/migration/remove_index.rb @@ -9,7 +9,7 @@ module RuboCop class RemoveIndex < RuboCop::Cop::Cop include MigrationHelpers - MSG = '`remove_index` requires downtime, use `remove_concurrent_index` instead'.freeze + MSG = '`remove_index` requires downtime, use `remove_concurrent_index` instead' def on_def(node) return unless in_migration?(node) diff --git a/rubocop/cop/migration/safer_boolean_column.rb b/rubocop/cop/migration/safer_boolean_column.rb index 06bb24707bd..1d780d96afa 100644 --- a/rubocop/cop/migration/safer_boolean_column.rb +++ b/rubocop/cop/migration/safer_boolean_column.rb @@ -21,9 +21,9 @@ module RuboCop class SaferBooleanColumn < RuboCop::Cop::Cop include MigrationHelpers - DEFAULT_OFFENSE = 'Boolean columns on the `%s` table should have a default. You may wish to use `add_column_with_default`.'.freeze - NULL_OFFENSE = 'Boolean columns on the `%s` table should disallow nulls.'.freeze - DEFAULT_AND_NULL_OFFENSE = 'Boolean columns on the `%s` table should have a default and should disallow nulls. You may wish to use `add_column_with_default`.'.freeze + DEFAULT_OFFENSE = 'Boolean columns on the `%s` table should have a default. You may wish to use `add_column_with_default`.' + NULL_OFFENSE = 'Boolean columns on the `%s` table should disallow nulls.' + DEFAULT_AND_NULL_OFFENSE = 'Boolean columns on the `%s` table should have a default and should disallow nulls. You may wish to use `add_column_with_default`.' def_node_matcher :add_column?, <<~PATTERN (send nil? :add_column $...) diff --git a/rubocop/cop/migration/timestamps.rb b/rubocop/cop/migration/timestamps.rb index 5584d49ee8c..44baf17d968 100644 --- a/rubocop/cop/migration/timestamps.rb +++ b/rubocop/cop/migration/timestamps.rb @@ -9,7 +9,7 @@ module RuboCop class Timestamps < RuboCop::Cop::Cop include MigrationHelpers - MSG = 'Do not use `timestamps`, use `timestamps_with_timezone` instead'.freeze + MSG = 'Do not use `timestamps`, use `timestamps_with_timezone` instead' # Check methods in table creation. def on_def(node) diff --git a/rubocop/cop/migration/update_column_in_batches.rb b/rubocop/cop/migration/update_column_in_batches.rb index d23e0d28380..e23042e1b9f 100644 --- a/rubocop/cop/migration/update_column_in_batches.rb +++ b/rubocop/cop/migration/update_column_in_batches.rb @@ -11,7 +11,7 @@ module RuboCop include MigrationHelpers MSG = 'Migration running `update_column_in_batches` must have a spec file at' \ - ' `%s`.'.freeze + ' `%s`.' def on_send(node) return unless in_migration?(node) diff --git a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb index f423bde8343..cb36e7413ab 100644 --- a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb +++ b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb @@ -27,6 +27,7 @@ module RuboCop foreign_key_exists? index_exists? column_exists? + 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(', ')}" diff --git a/rubocop/cop/migration/with_lock_retries_with_change.rb b/rubocop/cop/migration/with_lock_retries_with_change.rb index 36fc1f92833..9d11edcb6a1 100644 --- a/rubocop/cop/migration/with_lock_retries_with_change.rb +++ b/rubocop/cop/migration/with_lock_retries_with_change.rb @@ -10,7 +10,7 @@ module RuboCop include MigrationHelpers MSG = '`with_lock_retries` cannot be used within `change` so you must manually define ' \ - 'the `up` and `down` methods in your migration class and use `with_lock_retries` in both methods'.freeze + 'the `up` and `down` methods in your migration class and use `with_lock_retries` in both methods' def on_send(node) return unless in_migration?(node) diff --git a/rubocop/cop/project_path_helper.rb b/rubocop/cop/project_path_helper.rb index bc2454e5b1f..ec3f847faf9 100644 --- a/rubocop/cop/project_path_helper.rb +++ b/rubocop/cop/project_path_helper.rb @@ -5,7 +5,7 @@ module RuboCop class ProjectPathHelper < RuboCop::Cop::Cop MSG = 'Use short project path helpers without explicitly passing the namespace: ' \ '`foo_project_bar_path(project, bar)` instead of ' \ - '`foo_namespace_project_bar_path(project.namespace, project, bar)`.'.freeze + '`foo_namespace_project_bar_path(project.namespace, project, bar)`.' METHOD_NAME_PATTERN = /\A([a-z_]+_)?namespace_project(?:_[a-z_]+)?_(?:url|path)\z/.freeze diff --git a/rubocop/cop/qa/ambiguous_page_object_name.rb b/rubocop/cop/qa/ambiguous_page_object_name.rb index 5cd8ea25c87..a4a2c04f61f 100644 --- a/rubocop/cop/qa/ambiguous_page_object_name.rb +++ b/rubocop/cop/qa/ambiguous_page_object_name.rb @@ -19,7 +19,7 @@ module RuboCop class AmbiguousPageObjectName < RuboCop::Cop::Cop include QAHelpers - MESSAGE = "Don't use 'page' as a name for a Page Object. Use `%s` instead.".freeze + MESSAGE = "Don't use 'page' as a name for a Page Object. Use `%s` instead." def_node_matcher :ambiguous_page?, <<~PATTERN (block diff --git a/rubocop/cop/qa/element_with_pattern.rb b/rubocop/cop/qa/element_with_pattern.rb index d48f4725488..d0a42497960 100644 --- a/rubocop/cop/qa/element_with_pattern.rb +++ b/rubocop/cop/qa/element_with_pattern.rb @@ -19,7 +19,7 @@ module RuboCop class ElementWithPattern < RuboCop::Cop::Cop include QAHelpers - MESSAGE = "Don't use a pattern for element, create a corresponding `%s` instead.".freeze + MESSAGE = "Don't use a pattern for element, create a corresponding `%s` instead." def on_send(node) return unless in_qa_file?(node) diff --git a/rubocop/cop/rspec/env_assignment.rb b/rubocop/cop/rspec/env_assignment.rb index 73e108c2232..e3075e7bd90 100644 --- a/rubocop/cop/rspec/env_assignment.rb +++ b/rubocop/cop/rspec/env_assignment.rb @@ -17,7 +17,7 @@ module RuboCop # stub_env('FOO', 'bar') # end class EnvAssignment < RuboCop::Cop::Cop - MESSAGE = "Don't assign to ENV, use `stub_env` instead.".freeze + MESSAGE = "Don't assign to ENV, use `stub_env` instead." def_node_search :env_assignment?, <<~PATTERN (send (const nil? :ENV) :[]= ...) diff --git a/rubocop/cop/rspec/factories_in_migration_specs.rb b/rubocop/cop/rspec/factories_in_migration_specs.rb index 732e0d92bbd..f29bbf68cdc 100644 --- a/rubocop/cop/rspec/factories_in_migration_specs.rb +++ b/rubocop/cop/rspec/factories_in_migration_specs.rb @@ -14,7 +14,7 @@ module RuboCop # let(:users) { table(:users) } # let(:user) { users.create!(name: 'User 1', username: 'user1') } class FactoriesInMigrationSpecs < RuboCop::Cop::Cop - MESSAGE = "Don't use FactoryBot.%s in migration specs, use `table` instead.".freeze + MESSAGE = "Don't use FactoryBot.%s in migration specs, use `table` instead." FORBIDDEN_METHODS = %i[build build_list create create_list attributes_for].freeze def_node_search :forbidden_factory_usage?, <<~PATTERN diff --git a/rubocop/cop/rspec/httparty_basic_auth.rb b/rubocop/cop/rspec/httparty_basic_auth.rb index 529a5808662..c6b52ac9781 100644 --- a/rubocop/cop/rspec/httparty_basic_auth.rb +++ b/rubocop/cop/rspec/httparty_basic_auth.rb @@ -13,7 +13,7 @@ module RuboCop # # good # HTTParty.get(url, basic_auth: { username: 'foo' }) class HTTPartyBasicAuth < RuboCop::Cop::Cop - MESSAGE = "`basic_auth: { user: ... }` does not work - replace `user:` with `username:`".freeze + MESSAGE = "`basic_auth: { user: ... }` does not work - replace `user:` with `username:`" RESTRICT_ON_SEND = %i(get put post delete).freeze diff --git a/rubocop/cop/safe_params.rb b/rubocop/cop/safe_params.rb index 250c16232e4..2720732c161 100644 --- a/rubocop/cop/safe_params.rb +++ b/rubocop/cop/safe_params.rb @@ -3,7 +3,7 @@ module RuboCop module Cop class SafeParams < RuboCop::Cop::Cop - MSG = 'Use `safe_params` instead of `params` in url_for.'.freeze + MSG = 'Use `safe_params` instead of `params` in url_for.' METHOD_NAME_PATTERN = :url_for UNSAFE_PARAM = :params diff --git a/rubocop/cop/sidekiq_options_queue.rb b/rubocop/cop/sidekiq_options_queue.rb index 499c712175e..2574a229ec2 100644 --- a/rubocop/cop/sidekiq_options_queue.rb +++ b/rubocop/cop/sidekiq_options_queue.rb @@ -4,7 +4,7 @@ module RuboCop module Cop # Cop that prevents manually setting a queue in Sidekiq workers. class SidekiqOptionsQueue < RuboCop::Cop::Cop - MSG = 'Do not manually set a queue; `ApplicationWorker` sets one automatically.'.freeze + MSG = 'Do not manually set a queue; `ApplicationWorker` sets one automatically.' def_node_matcher :sidekiq_options?, <<~PATTERN (send nil? :sidekiq_options $...) diff --git a/rubocop/cop/static_translation_definition.rb b/rubocop/cop/static_translation_definition.rb index 55956d9665b..ac50fd94884 100644 --- a/rubocop/cop/static_translation_definition.rb +++ b/rubocop/cop/static_translation_definition.rb @@ -3,7 +3,7 @@ module RuboCop module Cop class StaticTranslationDefinition < RuboCop::Cop::Cop - MSG = "The text you're translating will be already in the translated form when it's assigned to the constant. When a users changes the locale, these texts won't be translated again. Consider moving the translation logic to a method.".freeze + MSG = "The text you're translating will be already in the translated form when it's assigned to the constant. When a users changes the locale, these texts won't be translated again. Consider moving the translation logic to a method." TRANSLATION_METHODS = %i[_ s_ n_].freeze diff --git a/rubocop/cop/style/regexp_literal_mixed_preserve.rb b/rubocop/cop/style/regexp_literal_mixed_preserve.rb new file mode 100644 index 00000000000..4dc38d82f45 --- /dev/null +++ b/rubocop/cop/style/regexp_literal_mixed_preserve.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # This cop is based on `Style/RegexpLiteral` but adds a new + # `EnforcedStyle` option `mixed_preserve`. + # + # This cop will be removed once the upstream PR is merged and RuboCop upgraded. + # + # See https://github.com/rubocop/rubocop/pull/9688 + class RegexpLiteralMixedPreserve < RuboCop::Cop::Style::RegexpLiteral + module Patch + private + + def allowed_slash_literal?(node) + super || allowed_mixed_preserve?(node) + end + + def allowed_percent_r_literal?(node) + super || allowed_mixed_preserve?(node) + end + + def allowed_mixed_preserve?(node) + style == :mixed_preserve && !contains_disallowed_slash?(node) + end + end + + prepend Patch + end + end + end +end diff --git a/rubocop/cop/usage_data/distinct_count_by_large_foreign_key.rb b/rubocop/cop/usage_data/distinct_count_by_large_foreign_key.rb index 9fdf52dac8b..3aad089d961 100644 --- a/rubocop/cop/usage_data/distinct_count_by_large_foreign_key.rb +++ b/rubocop/cop/usage_data/distinct_count_by_large_foreign_key.rb @@ -13,7 +13,7 @@ module RuboCop # distinct_count(Ci::Build, :commit_id) # class DistinctCountByLargeForeignKey < RuboCop::Cop::Cop - MSG = 'Avoid doing `%s` on foreign keys for large tables having above 100 million rows.'.freeze + MSG = 'Avoid doing `%s` on foreign keys for large tables having above 100 million rows.' def_node_matcher :distinct_count?, <<-PATTERN (send _ $:distinct_count $...) diff --git a/rubocop/cop/user_admin.rb b/rubocop/cop/user_admin.rb new file mode 100644 index 00000000000..3ba0e770ec1 --- /dev/null +++ b/rubocop/cop/user_admin.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # Cop that rejects the usage of `User#admin?` + class UserAdmin < RuboCop::Cop::Cop + MSG = 'Direct calls to `User#admin?` to determine admin status should be ' \ + 'avoided as they will not take into account the policies framework ' \ + 'and will ignore Admin Mode if enabled. Please use a policy check ' \ + 'with `User#can_admin_all_resources?` or `User#can_read_all_resources?`.' + + def_node_matcher :admin_call?, <<~PATTERN + ({send | csend} _ :admin? ...) + PATTERN + + def on_send(node) + on_handler(node) + end + + def on_csend(node) + on_handler(node) + end + + private + + def on_handler(node) + return unless admin_call?(node) + + add_offense(node, location: :selector) + end + end + end +end diff --git a/rubocop/rubocop-migrations.yml b/rubocop/rubocop-migrations.yml index c1ad3cc8abb..c8f50016710 100644 --- a/rubocop/rubocop-migrations.yml +++ b/rubocop/rubocop-migrations.yml @@ -19,6 +19,7 @@ Migration/UpdateLargeTable: - :merge_request_diffs - :merge_request_metrics - :merge_requests + - :namespace_settings - :namespaces - :note_diff_files - :notes |