diff options
Diffstat (limited to 'rubocop/cop')
23 files changed, 339 insertions, 47 deletions
diff --git a/rubocop/cop/gitlab/duplicate_spec_location.rb b/rubocop/cop/gitlab/duplicate_spec_location.rb index ece3b9313d9..f8c19caf351 100644 --- a/rubocop/cop/gitlab/duplicate_spec_location.rb +++ b/rubocop/cop/gitlab/duplicate_spec_location.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true -require 'rubocop/rspec/top_level_describe' +require 'rubocop/cop/rspec/base' +require 'rubocop/cop/rspec/mixin/top_level_group' module RuboCop module Cop @@ -19,17 +20,17 @@ module RuboCop # # good, spec for EE only code # ee/spec/controllers/my_spec.rb # describe MyClass # - class DuplicateSpecLocation < RuboCop::Cop::Cop - include RuboCop::RSpec::TopLevelDescribe + class DuplicateSpecLocation < RuboCop::Cop::RSpec::Base + include RuboCop::Cop::RSpec::TopLevelGroup MSG = 'Duplicate spec location in `%<path>s`.' - def on_top_level_describe(node, _args) - path = file_path_for_node(node).sub(%r{\A#{rails_root}/}, '') + def on_top_level_group(node) + path = file_path_for_node(node.send_node).sub(%r{\A#{rails_root}/}, '') duplicate_path = find_duplicate_path(path) if duplicate_path && File.exist?(File.join(rails_root, duplicate_path)) - add_offense(node, message: format(MSG, path: duplicate_path)) + add_offense(node.send_node, message: format(MSG, path: duplicate_path)) end end diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb index 8d8c84e78f5..23de0644385 100644 --- a/rubocop/cop/gitlab/mark_used_feature_flags.rb +++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb @@ -13,11 +13,8 @@ module RuboCop include RuboCop::CodeReuseHelpers FEATURE_METHODS = %i[enabled? disabled?].freeze - EXPERIMENTATION_METHODS = %i[active?].freeze EXPERIMENT_METHODS = %i[ experiment - experiment_enabled? - push_frontend_experiment ].freeze RUGGED_METHODS = %i[ use_rugged? @@ -33,7 +30,7 @@ module RuboCop limit_feature_flag_for_override= ].freeze + EXPERIMENT_METHODS + RUGGED_METHODS + WORKER_METHODS - RESTRICT_ON_SEND = FEATURE_METHODS + EXPERIMENTATION_METHODS + SELF_METHODS + RESTRICT_ON_SEND = FEATURE_METHODS + SELF_METHODS USAGE_DATA_COUNTERS_EVENTS_YAML_GLOBS = [ File.expand_path("../../../config/metrics/aggregates/*.yml", __dir__), @@ -79,15 +76,6 @@ module RuboCop else save_used_feature_flag(flag_value) end - - if experiment_method?(node) || experimentation_method?(node) - # Additionally, mark experiment-related feature flag as used as well - matching_feature_flags = defined_feature_flags.select { |flag| flag == "#{flag_value}_experiment_percentage" } - matching_feature_flags.each do |matching_feature_flag| - puts_if_debug(node, "The '#{matching_feature_flag}' feature flag tracks the #{flag_value} experiment, which is still in use, so we'll mark it as used.") - save_used_feature_flag(matching_feature_flag) - end - end elsif flag_arg_is_send_type?(flag_arg) puts_if_debug(node, "Feature flag is dynamic: '#{flag_value}.") elsif flag_arg_is_dstr_or_dsym?(flag_arg) @@ -176,14 +164,6 @@ module RuboCop class_caller(node) == "Feature::Gitaly" end - def caller_is_experimentation?(node) - class_caller(node) == "Gitlab::Experimentation" - end - - def experiment_method?(node) - EXPERIMENT_METHODS.include?(method_name(node)) - end - def rugged_method?(node) RUGGED_METHODS.include?(method_name(node)) end @@ -192,10 +172,6 @@ module RuboCop FEATURE_METHODS.include?(method_name(node)) && (caller_is_feature?(node) || caller_is_feature_gitaly?(node)) end - def experimentation_method?(node) - EXPERIMENTATION_METHODS.include?(method_name(node)) && caller_is_experimentation?(node) - end - def worker_method?(node) WORKER_METHODS.include?(method_name(node)) end @@ -205,7 +181,7 @@ module RuboCop end def trackable_flag?(node) - feature_method?(node) || experimentation_method?(node) || self_method?(node) + feature_method?(node) || self_method?(node) end # Marking all event's feature flags as used as Gitlab::UsageDataCounters::HLLRedisCounter.track_event{,context} diff --git a/rubocop/cop/gitlab/no_code_coverage_comment.rb b/rubocop/cop/gitlab/no_code_coverage_comment.rb new file mode 100644 index 00000000000..3b989930026 --- /dev/null +++ b/rubocop/cop/gitlab/no_code_coverage_comment.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Discourages the use of `# :nocov:` to exclude code from coverage report. + # + # The nocov token can be configured via `CommentToken` option and defaults + # to `'nocov'`. + # + # @example CommentToken: 'nocov' (default) + # + # # bad + # # :nocov: + # def method + # end + # # :nocov: + # + # # good + # def method + # end + # + class NoCodeCoverageComment < RuboCop::Cop::Base + include RangeHelp + + MSG = 'The use of %<nocov_comment>s is discouraged. All code must have tests. See https://docs.gitlab.com/ee/development/contributing/merge_request_workflow.html#testing' + + DEFAULT_COMMENT_TOKEN = 'nocov' + + def on_new_investigation + super + + nocov_token = cop_config.fetch('CommentToken', DEFAULT_COMMENT_TOKEN) + nocov_comment = ":#{nocov_token}:" + # See https://github.com/simplecov-ruby/simplecov/blob/v0.21.2/lib/simplecov/lines_classifier.rb#L16 + regexp = /^(?:\s*)#(?:\s*)(?::#{nocov_token}:)/ + + processed_source.comments.each do |comment| + register_offense(comment, nocov_comment) if regexp.match?(comment.text) + end + end + + private + + def register_offense(comment, nocov_comment) + range = range_of_offense(comment, nocov_comment) + message = format(MSG, nocov_comment: nocov_comment) + + add_offense(range, message: message) + end + + def range_of_offense(comment, name) + start_pos = comment_start(comment) + token_indentation(comment, name) + range_between(start_pos, start_pos + name.size) + end + + def comment_start(comment) + comment.loc.expression.begin_pos + end + + def token_indentation(comment, name) + comment.text.index(name) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/service_response.rb b/rubocop/cop/gitlab/service_response.rb new file mode 100644 index 00000000000..edde662a038 --- /dev/null +++ b/rubocop/cop/gitlab/service_response.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require_relative '../../code_reuse_helpers' + +module RuboCop + module Cop + module Gitlab + class ServiceResponse < ::RuboCop::Cop::Base + include CodeReuseHelpers + + # This cop checks that ServiceResponse object is not used with the + # deprecated attribute `http_status`. + # + # @example + # + # # bad + # ServiceResponse.error(message: "...", http_status: :forbidden) + # + # # good + # ServiceResponse.error(message: "...", reason: :insufficient_permissions) + MSG = 'Use `reason` instead of the deprecated `http_status`: https://gitlab.com/gitlab-org/gitlab/-/issues/356036' + + RESTRICT_ON_SEND = %i[error success new].freeze + METHOD_NAMES = RESTRICT_ON_SEND.map(&:inspect).join(' ').freeze + + def_node_matcher :service_response_with_http_status, <<~PATTERN + (send + (const {nil? cbase} :ServiceResponse) + {#{METHOD_NAMES}} + (hash <$(pair (sym :http_status) _) ...>) + ) + PATTERN + + def on_send(node) + pair = service_response_with_http_status(node) + return unless pair + + add_offense(pair) + end + end + end + end +end diff --git a/rubocop/cop/migration/background_migration_missing_active_concern.rb b/rubocop/cop/migration/background_migration_missing_active_concern.rb new file mode 100644 index 00000000000..417472bf512 --- /dev/null +++ b/rubocop/cop/migration/background_migration_missing_active_concern.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks `ActiveSupport::Concern` is included in EE batched background migrations + # if they define `scope_to`. + class BackgroundMigrationMissingActiveConcern < RuboCop::Cop::Base + include MigrationHelpers + + MSG = <<~MSG + Extend `ActiveSupport::Concern` in the EE background migration if it defines `scope_to`. + MSG + + def_node_matcher :prepended_block_uses_scope_to?, <<~PATTERN + (:block (:send nil? :prepended) (:args) `(:send nil? :scope_to ...)) + PATTERN + + def_node_matcher :scope_to?, <<~PATTERN + (:send nil? :scope_to ...) + PATTERN + + def_node_matcher :extend_activesupport_concern?, <<~PATTERN + (:send nil? :extend (:const (:const nil? :ActiveSupport) :Concern)) + PATTERN + + def on_block(node) + return unless in_ee_background_migration?(node) + return unless prepended_block_uses_scope_to?(node) + + return if module_extends_activesupport_concern?(node) + + node.descendants.each do |descendant| + next unless scope_to?(descendant) + + add_offense(descendant) + end + end + + private + + def module_extends_activesupport_concern?(node) + while node = node.parent + break if node.type == :module + end + + return false unless node + + node.descendants.any? do |descendant| + extend_activesupport_concern?(descendant) + end + end + end + end + end +end diff --git a/rubocop/cop/redis_queue_usage.rb b/rubocop/cop/redis_queue_usage.rb new file mode 100644 index 00000000000..d993abc6327 --- /dev/null +++ b/rubocop/cop/redis_queue_usage.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # This class complements Rubocop::Cop::SidekiqRedisCall by disallowing the use of + # Gitlab::Redis::Queues with the exception of initialising Sidekiq and monitoring. + class RedisQueueUsage < RuboCop::Cop::Base + MSG = 'Gitlab::Redis::Queues should only be used by Sidekiq initializers. '\ + 'Assignments or using its params to initialise another connection is not allowed.' + + def_node_matcher :calling_redis_queue_module_methods?, <<~PATTERN + (send (const (const (const nil? :Gitlab) :Redis) :Queues) ...) + PATTERN + + def_node_matcher :using_redis_queue_module_as_parameter?, <<~PATTERN + (send ... (const (const (const nil? :Gitlab) :Redis) :Queues)) + PATTERN + + def_node_matcher :redis_queue_assignment?, <<~PATTERN + ({lvasgn | ivasgn | cvasgn | gvasgn | casgn | masgn | op_asgn | or_asgn | and_asgn } ... + `(const (const (const nil? :Gitlab) :Redis) :Queues)) + PATTERN + + def on_send(node) + return unless using_redis_queue_module_as_parameter?(node) || calling_redis_queue_module_methods?(node) + + add_offense(node, message: MSG) + end + + # offenses caught in assignment may overlap with on_send + %i[on_lvasgn on_ivasgn on_cvasgn on_gvasgn on_casgn on_masgn on_op_asgn on_or_asgn on_and_asgn].each do |name| + define_method(name) do |node| + add_offense(node, message: MSG) if redis_queue_assignment?(node) + end + end + end + end +end diff --git a/rubocop/cop/rspec/any_instance_of.rb b/rubocop/cop/rspec/any_instance_of.rb index e1cacfebfd3..7016a76ec93 100644 --- a/rubocop/cop/rspec/any_instance_of.rb +++ b/rubocop/cop/rspec/any_instance_of.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'rubocop-rspec' + module RuboCop module Cop module RSpec diff --git a/rubocop/cop/rspec/be_success_matcher.rb b/rubocop/cop/rspec/be_success_matcher.rb index 5a011845075..1ed55762965 100644 --- a/rubocop/cop/rspec/be_success_matcher.rb +++ b/rubocop/cop/rspec/be_success_matcher.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'rubocop-rspec' + module RuboCop module Cop module RSpec diff --git a/rubocop/cop/rspec/env_assignment.rb b/rubocop/cop/rspec/env_assignment.rb index add7897c624..6994f3f0969 100644 --- a/rubocop/cop/rspec/env_assignment.rb +++ b/rubocop/cop/rspec/env_assignment.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'rubocop-rspec' + module RuboCop module Cop module RSpec diff --git a/rubocop/cop/rspec/expect_gitlab_tracking.rb b/rubocop/cop/rspec/expect_gitlab_tracking.rb index 4f92980baa4..13fc7eace71 100644 --- a/rubocop/cop/rspec/expect_gitlab_tracking.rb +++ b/rubocop/cop/rspec/expect_gitlab_tracking.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'rack/utils' +require 'rubocop-rspec' module RuboCop module Cop diff --git a/rubocop/cop/rspec/factories_in_migration_specs.rb b/rubocop/cop/rspec/factories_in_migration_specs.rb index 6dde3d4524c..7dce1264b0e 100644 --- a/rubocop/cop/rspec/factories_in_migration_specs.rb +++ b/rubocop/cop/rspec/factories_in_migration_specs.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'rubocop-rspec' + module RuboCop module Cop module RSpec diff --git a/rubocop/cop/rspec/factory_bot/avoid_create.rb b/rubocop/cop/rspec/factory_bot/avoid_create.rb new file mode 100644 index 00000000000..e9bccfb37cd --- /dev/null +++ b/rubocop/cop/rspec/factory_bot/avoid_create.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'rubocop-rspec' + +module RuboCop + module Cop + module RSpec + module FactoryBot + # This cop checks for the creation of ActiveRecord objects in serializers specs specs + # + # @example + # + # # bad + # let(:user) { create(:user) } + # let(:users) { create_list(:user, 2) } + # + # # good + # let(:user) { build_stubbed(:user) } + # let(:user) { build(:user) } + # let(:users) { build_stubbed_list(:user, 2) } + # let(:users) { build_list(:user, 2) } + class AvoidCreate < RuboCop::Cop::Base + MESSAGE = "Prefer using `build_stubbed` or similar over `%{method_name}`. See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#optimize-factory-usage" + FORBIDDEN_METHODS = %i[create create_list].freeze + RESTRICT_ON_SEND = FORBIDDEN_METHODS + + def_node_matcher :forbidden_factory_usage, <<~PATTERN + ( + send + {(const nil? :FactoryBot) nil?} + ${ #{FORBIDDEN_METHODS.map(&:inspect).join(' ')} } + ... + ) + PATTERN + + def on_send(node) + method_name = forbidden_factory_usage(node) + return unless method_name + + add_offense(node, message: format(MESSAGE, method_name: method_name)) + end + end + end + end + end +end diff --git a/rubocop/cop/rspec/factory_bot/inline_association.rb b/rubocop/cop/rspec/factory_bot/inline_association.rb index ccc6364fb73..8d7c73b99a0 100644 --- a/rubocop/cop/rspec/factory_bot/inline_association.rb +++ b/rubocop/cop/rspec/factory_bot/inline_association.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'rubocop-rspec' + module RuboCop module Cop module RSpec diff --git a/rubocop/cop/rspec/have_gitlab_http_status.rb b/rubocop/cop/rspec/have_gitlab_http_status.rb index 86ece72b4f5..29577598ba7 100644 --- a/rubocop/cop/rspec/have_gitlab_http_status.rb +++ b/rubocop/cop/rspec/have_gitlab_http_status.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'rack/utils' +require 'rubocop-rspec' module RuboCop module Cop diff --git a/rubocop/cop/rspec/httparty_basic_auth.rb b/rubocop/cop/rspec/httparty_basic_auth.rb index 1e0f7ae7af0..d188002673f 100644 --- a/rubocop/cop/rspec/httparty_basic_auth.rb +++ b/rubocop/cop/rspec/httparty_basic_auth.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'rubocop-rspec' + module RuboCop module Cop module RSpec diff --git a/rubocop/cop/rspec/modify_sidekiq_middleware.rb b/rubocop/cop/rspec/modify_sidekiq_middleware.rb index 78e3ba223b0..2e27288933f 100644 --- a/rubocop/cop/rspec/modify_sidekiq_middleware.rb +++ b/rubocop/cop/rspec/modify_sidekiq_middleware.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'rubocop-rspec' + module RuboCop module Cop module RSpec diff --git a/rubocop/cop/rspec/timecop_freeze.rb b/rubocop/cop/rspec/timecop_freeze.rb index 70e37ecfa55..b13f5050040 100644 --- a/rubocop/cop/rspec/timecop_freeze.rb +++ b/rubocop/cop/rspec/timecop_freeze.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'rubocop-rspec' + module RuboCop module Cop module RSpec diff --git a/rubocop/cop/rspec/timecop_travel.rb b/rubocop/cop/rspec/timecop_travel.rb index 586567fa0cd..03f978be349 100644 --- a/rubocop/cop/rspec/timecop_travel.rb +++ b/rubocop/cop/rspec/timecop_travel.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'rubocop-rspec' + module RuboCop module Cop module RSpec diff --git a/rubocop/cop/rspec/top_level_describe_path.rb b/rubocop/cop/rspec/top_level_describe_path.rb index 3cc1ee8df90..ee1a0bcc4b6 100644 --- a/rubocop/cop/rspec/top_level_describe_path.rb +++ b/rubocop/cop/rspec/top_level_describe_path.rb @@ -1,21 +1,20 @@ # frozen_string_literal: true -require 'rubocop/rspec/top_level_describe' +require 'rubocop/cop/rspec/base' +require 'rubocop/cop/rspec/mixin/top_level_group' module RuboCop module Cop module RSpec - class TopLevelDescribePath < RuboCop::Cop::Cop - include RuboCop::RSpec::TopLevelDescribe + class TopLevelDescribePath < RuboCop::Cop::RSpec::Base + include RuboCop::Cop::RSpec::TopLevelGroup MESSAGE = 'A file with a top-level `describe` must end in _spec.rb.' - SHARED_EXAMPLES = %i[shared_examples shared_examples_for].freeze - def on_top_level_describe(node, args) + def on_top_level_example_group(node) return if acceptable_file_path?(processed_source.buffer.name) - return if shared_example?(node) - add_offense(node, message: MESSAGE) + add_offense(node.send_node, message: MESSAGE) end private @@ -23,12 +22,6 @@ module RuboCop def acceptable_file_path?(path) File.fnmatch?('*_spec.rb', path) || File.fnmatch?('*/frontend/fixtures/*', path) || File.fnmatch?('*/docs_screenshots/*_docs.rb', path) end - - def shared_example?(node) - node.ancestors.any? do |node| - node.respond_to?(:method_name) && SHARED_EXAMPLES.include?(node.method_name) - end - end end end end diff --git a/rubocop/cop/rspec/web_mock_enable.rb b/rubocop/cop/rspec/web_mock_enable.rb index 0bef16a16b0..395375e5fc1 100644 --- a/rubocop/cop/rspec/web_mock_enable.rb +++ b/rubocop/cop/rspec/web_mock_enable.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'rubocop-rspec' + module RuboCop module Cop module RSpec diff --git a/rubocop/cop/sidekiq_api_usage.rb b/rubocop/cop/sidekiq_api_usage.rb new file mode 100644 index 00000000000..35e5ec474cd --- /dev/null +++ b/rubocop/cop/sidekiq_api_usage.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + class SidekiqApiUsage < RuboCop::Cop::Base + MSG = 'Refrain from directly using Sidekiq APIs.' \ + 'Only permitted in migrations, administrations and Sidekiq middlewares.' + + ALLOWED_WORKER_METHODS = [ + :skipping_transaction_check, + :raise_inside_transaction_exception, + :raise_exception_for_being_inside_a_transaction? + ].freeze + + def_node_matcher :using_sidekiq_api?, <<~PATTERN + (send (const (const nil? :Sidekiq) $_ ) $... ) + PATTERN + + def on_send(node) + using_sidekiq_api?(node) do |klass, methods_called| + next if klass == :Testing + + # allow methods defined in config/initializers/forbid_sidekiq_in_transactions.rb + next if klass == :Worker && ALLOWED_WORKER_METHODS.include?(methods_called[0]) + + add_offense(node, message: MSG) + end + end + end + end +end diff --git a/rubocop/cop/sidekiq_redis_call.rb b/rubocop/cop/sidekiq_redis_call.rb new file mode 100644 index 00000000000..e4ae430f7c7 --- /dev/null +++ b/rubocop/cop/sidekiq_redis_call.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # Cop that prevents manually setting a queue in Sidekiq workers. + class SidekiqRedisCall < RuboCop::Cop::Base + MSG = 'Refrain from directly using Sidekiq.redis unless for migration. For admin operations, use Sidekiq APIs.' + + def_node_matcher :using_sidekiq_redis?, <<~PATTERN + (send (const nil? :Sidekiq) :redis) + PATTERN + + def on_send(node) + add_offense(node, message: MSG) if using_sidekiq_redis?(node) + end + end + end +end diff --git a/rubocop/cop/static_translation_definition.rb b/rubocop/cop/static_translation_definition.rb index aea4dd6ae34..e7b8cdeab12 100644 --- a/rubocop/cop/static_translation_definition.rb +++ b/rubocop/cop/static_translation_definition.rb @@ -64,7 +64,7 @@ module RuboCop NON_METHOD_DEFINITIONS = %i[class_methods included prepended].to_set.freeze def_node_matcher :translation_method?, <<~PATTERN - (send _ {#{RESTRICT_ON_SEND.map(&:inspect).join(' ')}} str*) + (send _ {#{RESTRICT_ON_SEND.map(&:inspect).join(' ')}} {dstr str}+) PATTERN def on_send(node) |