diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-05-17 16:05:49 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-05-17 16:05:49 +0000 |
commit | 43a25d93ebdabea52f99b05e15b06250cd8f07d7 (patch) | |
tree | dceebdc68925362117480a5d672bcff122fb625b /rubocop/cop | |
parent | 20c84b99005abd1c82101dfeff264ac50d2df211 (diff) | |
download | gitlab-ce-16.0.0-rc42.tar.gz |
Add latest changes from gitlab-org/gitlab@16-0-stable-eev16.0.0-rc4216-0-stable
Diffstat (limited to 'rubocop/cop')
-rw-r--r-- | rubocop/cop/background_migration/missing_dictionary_file.rb | 59 | ||||
-rw-r--r-- | rubocop/cop/gettext/static_identifier.rb | 84 | ||||
-rw-r--r-- | rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb | 33 | ||||
-rw-r--r-- | rubocop/cop/gitlab/feature_available_usage.rb | 2 | ||||
-rw-r--r-- | rubocop/cop/gitlab/json.rb | 6 | ||||
-rw-r--r-- | rubocop/cop/gitlab/mark_used_feature_flags.rb | 26 | ||||
-rw-r--r-- | rubocop/cop/graphql/authorize_types.rb | 2 | ||||
-rw-r--r-- | rubocop/cop/graphql/id_type.rb | 2 | ||||
-rw-r--r-- | rubocop/cop/migration/add_reference.rb | 2 | ||||
-rw-r--r-- | rubocop/cop/rspec/avoid_conditional_statements.rb | 35 | ||||
-rw-r--r-- | rubocop/cop/rspec/factory_bot/inline_association.rb | 2 | ||||
-rw-r--r-- | rubocop/cop/rspec/misspelled_aggregate_failures.rb | 35 | ||||
-rw-r--r-- | rubocop/cop/rspec/shared_groups_metadata.rb | 52 | ||||
-rw-r--r-- | rubocop/cop/ruby_interpolation_in_translation.rb | 28 | ||||
-rw-r--r-- | rubocop/cop/search/namespaced_class.rb | 74 | ||||
-rw-r--r-- | rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb | 29 |
16 files changed, 359 insertions, 112 deletions
diff --git a/rubocop/cop/background_migration/missing_dictionary_file.rb b/rubocop/cop/background_migration/missing_dictionary_file.rb new file mode 100644 index 00000000000..9158b268bf9 --- /dev/null +++ b/rubocop/cop/background_migration/missing_dictionary_file.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module BackgroundMigration + # Checks the batched background migration has the corresponding dictionary file + class MissingDictionaryFile < 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" + + DICTIONARY_DIR = "db/docs/batched_background_migrations" + + def_node_matcher :batched_background_migration_name_node, <<~PATTERN + `(send nil? :queue_batched_background_migration $_ ...) + PATTERN + + def_node_matcher :migration_constant_value, <<~PATTERN + `(casgn nil? %const_name ({sym|str} $_)) + PATTERN + + def on_class(node) + return unless time_enforced?(node) && in_post_deployment_migration?(node) + + migration_name_node = batched_background_migration_name_node(node) + return unless migration_name_node + + migration_name = if migration_name_node.const_name.present? + migration_constant_value(node, const_name: migration_name_node.const_name.to_sym) + else + migration_name_node.value + end + + return if dictionary_file?(migration_name) + + add_offense(node, message: format(MSG, file_name: dictionary_file_path(migration_name))) + end + + private + + def dictionary_file?(migration_class_name) + File.exist?(dictionary_file_path(migration_class_name)) + end + + def dictionary_file_path(migration_class_name) + File.join(rails_root, DICTIONARY_DIR, "#{migration_class_name.underscore}.yml") + end + + def rails_root + @rails_root ||= File.expand_path('../../..', __dir__) + end + end + end + end +end diff --git a/rubocop/cop/gettext/static_identifier.rb b/rubocop/cop/gettext/static_identifier.rb new file mode 100644 index 00000000000..9ca1c88f4b6 --- /dev/null +++ b/rubocop/cop/gettext/static_identifier.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gettext + # Ensure that gettext identifiers are statically defined and not + # interpolated, formatted, or concatenated. + # + # @example + # + # # bad + # _('Hi #{name}') + # _('Hi %{name}' % { name: 'Luki' }) + # _(format('Hi %{name}', name: 'Luki')) + # + # # good + # _('Hi %{name}') % { name: 'Luki' } + # format(_('Hi %{name}', name: 'Luki')) + # + # # also good + # var = "Hi" + # _(var) + # _(some_method_call) + # _(CONST) + class StaticIdentifier < RuboCop::Cop::Base + MSG = 'Ensure to pass static strings to translation method `%{method_name}(...)`.' + + # Number of parameters to check for translation methods. + PARAMETERS_TO_CHECK = { + _: 1, + s_: 1, + N_: 1, + n_: 2 + }.freeze + + # RuboCop-specific optimization for `on_send`. + RESTRICT_ON_SEND = PARAMETERS_TO_CHECK.keys.freeze + + DENIED_METHOD_CALLS = %i[% format + concat].freeze + + def on_send(node) + method_name = node.method_name + arguments = node.arguments + + each_invalid_argument(method_name, arguments) do |argument_node| + message = format(MSG, method_name: method_name) + + add_offense(argument_node || node, message: message) + end + end + + private + + def each_invalid_argument(method_name, argument_nodes) + number = PARAMETERS_TO_CHECK.fetch(method_name) + + argument_nodes.take(number).each do |argument_node| + yield argument_node unless valid_argument?(argument_node) + end + end + + def valid_argument?(node) + return false unless node + + basic_type?(node) || multiline_string?(node) || allowed_method_call?(node) + end + + def basic_type?(node) + node.str_type? || node.lvar_type? || node.const_type? + end + + def multiline_string?(node) + node.dstr_type? && node.children.all?(&:str_type?) + end + + def allowed_method_call?(node) + return false unless node.send_type? + + !DENIED_METHOD_CALLS.include?(node.method_name) # rubocop:disable Rails/NegateInclude + end + end + end + end +end diff --git a/rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb b/rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb deleted file mode 100644 index 58411357eeb..00000000000 --- a/rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -require 'rack/utils' - -module RuboCop - module Cop - module Gitlab - # This cop prevents from using deprecated `track_redis_hll_event` method. - # - # @example - # - # # bad - # track_redis_hll_event :show, name: 'p_analytics_valuestream' - # - # # good - # track_event :show, name: 'g_analytics_valuestream', destinations: [:redis_hll] - class DeprecateTrackRedisHLLEvent < RuboCop::Cop::Base - MSG = '`track_redis_hll_event` is deprecated. Use `track_event` helper instead. ' \ - 'See https://docs.gitlab.com/ee/development/service_ping/implement.html#add-new-events' - - def_node_matcher :track_redis_hll_event_used?, <<~PATTERN - (send _ :track_redis_hll_event ...) - PATTERN - - def on_send(node) - return unless track_redis_hll_event_used?(node) - - add_offense(node.loc.selector) - end - end - end - end -end diff --git a/rubocop/cop/gitlab/feature_available_usage.rb b/rubocop/cop/gitlab/feature_available_usage.rb index fcf4992a19d..df4409c27e0 100644 --- a/rubocop/cop/gitlab/feature_available_usage.rb +++ b/rubocop/cop/gitlab/feature_available_usage.rb @@ -39,7 +39,7 @@ module RuboCop 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)) + if !ALL_FEATURES.include?(feature_name(node)) # rubocop:disable Rails/NegateInclude add_offense(node, message: licensed_feature_message(node)) elsif args_count(node) < 2 add_offense(node, message: NOT_ENOUGH_ARGS_MSG) diff --git a/rubocop/cop/gitlab/json.rb b/rubocop/cop/gitlab/json.rb index cf2ed0ba536..8b10850b894 100644 --- a/rubocop/cop/gitlab/json.rb +++ b/rubocop/cop/gitlab/json.rb @@ -6,9 +6,9 @@ module RuboCop class Json < RuboCop::Cop::Base extend RuboCop::Cop::AutoCorrector - MSG = <<~EOL + MSG = <<~TEXT Prefer `Gitlab::Json` over calling `JSON` directly. See https://docs.gitlab.com/ee/development/json.html - EOL + TEXT AVAILABLE_METHODS = %i[parse parse! load decode dump generate encode pretty_generate].to_set.freeze @@ -41,7 +41,7 @@ module RuboCop end def cbased(node) - return unless %r{/ee/}.match?(node.location.expression.source_buffer.name) + return unless node.location.expression.source_buffer.name.include?('/ee/') "::" end diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb index ffd59c8fffc..65a1731fc28 100644 --- a/rubocop/cop/gitlab/mark_used_feature_flags.rb +++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb @@ -32,12 +32,6 @@ module RuboCop RESTRICT_ON_SEND = FEATURE_METHODS + SELF_METHODS - USAGE_DATA_COUNTERS_EVENTS_YAML_GLOBS = [ - File.expand_path("../../../config/metrics/aggregates/*.yml", __dir__), - File.expand_path("../../../lib/gitlab/usage_data_counters/known_events/*.yml", __dir__), - File.expand_path("../../../ee/lib/ee/gitlab/usage_data_counters/known_events/*.yml", __dir__) - ].freeze - class << self # We track feature flags in `on_new_investigation` only once per # rubocop whole run instead once per file. @@ -52,8 +46,6 @@ module RuboCop return if self.class.feature_flags_already_tracked self.class.feature_flags_already_tracked = true - - track_usage_data_counters_known_events! end def on_casgn(node) @@ -181,23 +173,7 @@ module RuboCop end def trackable_flag?(node) - feature_method?(node) || self_method?(node) - end - - # Marking all event's feature flags as used as Gitlab::UsageDataCounters::HLLRedisCounter.track_event{,context} - # is mostly used with dynamic event name. - def track_usage_data_counters_known_events! - usage_data_counters_known_event_feature_flags.each { |feature_flag_name| save_used_feature_flag(feature_flag_name) } - end - - def usage_data_counters_known_event_feature_flags - USAGE_DATA_COUNTERS_EVENTS_YAML_GLOBS.each_with_object(Set.new) do |glob, memo| - Dir.glob(glob).each do |path| - YAML.safe_load(File.read(path))&.each do |hash| - memo << hash['feature_flag'] if hash['feature_flag'] - end - end - end + feature_method?(node) || self_method?(node) || worker_method?(node) end def defined_feature_flags diff --git a/rubocop/cop/graphql/authorize_types.rb b/rubocop/cop/graphql/authorize_types.rb index 7bd2cd9f7ef..db79a4732d9 100644 --- a/rubocop/cop/graphql/authorize_types.rb +++ b/rubocop/cop/graphql/authorize_types.rb @@ -12,7 +12,7 @@ module RuboCop QueryType GraphQL::Schema BaseUnion BaseInputObject].freeze def_node_search :authorize?, <<~PATTERN - (send nil? :authorize ...) + (send nil? :authorize sym+) PATTERN def on_class(node) diff --git a/rubocop/cop/graphql/id_type.rb b/rubocop/cop/graphql/id_type.rb index eb677aa4507..c9d9b4ea6eb 100644 --- a/rubocop/cop/graphql/id_type.rb +++ b/rubocop/cop/graphql/id_type.rb @@ -21,7 +21,7 @@ module RuboCop private def does_not_match?(arg) - !WHITELISTED_ARGUMENTS.include?(arg) + !WHITELISTED_ARGUMENTS.include?(arg) # rubocop:disable Rails/NegateInclude end end end diff --git a/rubocop/cop/migration/add_reference.rb b/rubocop/cop/migration/add_reference.rb index 02a0ef899b4..8daa85749fd 100644 --- a/rubocop/cop/migration/add_reference.rb +++ b/rubocop/cop/migration/add_reference.rb @@ -41,7 +41,7 @@ module RuboCop private def existing_table?(new_tables, table) - !new_tables.include?(table) + !new_tables.include?(table) # rubocop:disable Rails/NegateInclude end def create_table?(node) diff --git a/rubocop/cop/rspec/avoid_conditional_statements.rb b/rubocop/cop/rspec/avoid_conditional_statements.rb new file mode 100644 index 00000000000..d03c3fb5d04 --- /dev/null +++ b/rubocop/cop/rspec/avoid_conditional_statements.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'rubocop-rspec' + +module RuboCop + module Cop + module RSpec + # This cop checks for the usage of conditional statements in specs. + # + # @example + # + # # bad + # + # page.has_css?('[data-testid="begin-commit-button"]') ? find('[data-testid="begin-commit-button"]').click : nil + # + # if page.has_css?('[data-testid="begin-commit-button"]') + # find('[data-testid="begin-commit-button"]').click + # end + # + # unless page.has_css?('[data-testid="begin-commit-button"]') + # find('[data-testid="begin-commit-button"]').click + # end + class AvoidConditionalStatements < RuboCop::Cop::Base + MESSAGE = "Don't use `%{conditional}` conditional statement in specs, it might create flakiness. " \ + "See https://gitlab.com/gitlab-org/gitlab/-/issues/385304#note_1345437109" + + def on_if(node) + conditional = node.ternary? ? node.source : node.keyword + + add_offense(node, message: format(MESSAGE, conditional: conditional)) + 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 8d7c73b99a0..acd2c10a63d 100644 --- a/rubocop/cop/rspec/factory_bot/inline_association.rb +++ b/rubocop/cop/rspec/factory_bot/inline_association.rb @@ -99,7 +99,7 @@ module RuboCop def inside_assocation_definition?(node) node.each_ancestor(:block).any? do |parent| name = association_definition(parent) - name && !SKIP_NAMES.include?(name) + name && !SKIP_NAMES.include?(name) # rubocop:disable Rails/NegateInclude end end end diff --git a/rubocop/cop/rspec/misspelled_aggregate_failures.rb b/rubocop/cop/rspec/misspelled_aggregate_failures.rb new file mode 100644 index 00000000000..04eec515318 --- /dev/null +++ b/rubocop/cop/rspec/misspelled_aggregate_failures.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'rubocop/cop/rspec/base' + +module RuboCop + module Cop + module RSpec + class MisspelledAggregateFailures < RuboCop::Cop::RSpec::Base + extend RuboCop::Cop::AutoCorrector + + CORRECT_SPELLING = :aggregate_failures + MSG = "Use `#{CORRECT_SPELLING.inspect}` to aggregate failures.".freeze + + # @!method aggregate_tag(node) + def_node_matcher :aggregate_tag, <<~PATTERN + (block + (send #rspec? {#ExampleGroups.all #Examples.all} + <$(sym /^aggregate[a-z]*_[a-z]+$/) ...> + ) + ... + ) + PATTERN + + def on_block(node) + tag_node = aggregate_tag(node) + return if tag_node.nil? || tag_node.value == CORRECT_SPELLING + + add_offense(tag_node) do |corrector| + corrector.replace(tag_node, CORRECT_SPELLING.inspect) + end + end + end + end + end +end diff --git a/rubocop/cop/rspec/shared_groups_metadata.rb b/rubocop/cop/rspec/shared_groups_metadata.rb new file mode 100644 index 00000000000..ac2406e54d2 --- /dev/null +++ b/rubocop/cop/rspec/shared_groups_metadata.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'rubocop/cop/rspec/base' + +module RuboCop + module Cop + module RSpec + # Ensures that shared examples and shared context don't have any metadata. + # + # @example + # + # # bad + # RSpec.shared_examples 'an external link with rel attribute', feature_category: :team_planning do + # end + # + # RSpec.shared_examples 'an external link with rel attribute', :aggregate_failures do + # end + # + # RSpec.shared_context 'an external link with rel attribute', :aggregate_failures do + # end + # + # # good + # RSpec.shared_examples 'an external link with rel attribute' do + # end + # + # shared_examples 'an external link with rel attribute' do + # end + # + # it 'adds rel="nofollow" to external links', feature_category: :team_planning do + # end + class SharedGroupsMetadata < RuboCop::Cop::RSpec::Base + MSG = 'Avoid using metadata on shared examples and shared context. They might cause flaky tests. See https://gitlab.com/gitlab-org/gitlab/-/issues/404388' + + # @!method metadata_value(node) + def_node_matcher :metadata_value, <<~PATTERN + (block + (send #rspec? {#SharedGroups.all} _description $_ ...) + ... + ) + PATTERN + + def on_block(node) + value_node = metadata_value(node) + + return unless value_node + + add_offense(value_node, message: MSG) + end + end + end + end +end diff --git a/rubocop/cop/ruby_interpolation_in_translation.rb b/rubocop/cop/ruby_interpolation_in_translation.rb deleted file mode 100644 index fec550bf7c6..00000000000 --- a/rubocop/cop/ruby_interpolation_in_translation.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Cop - class RubyInterpolationInTranslation < RuboCop::Cop::Base - MSG = "Don't use ruby interpolation \#{} inside translated strings, instead use \%{}" - - TRANSLATION_METHODS = ':_ :s_ :N_ :n_' - - def_node_matcher :translation_method?, <<~PATTERN - (send nil? {#{TRANSLATION_METHODS}} $dstr ...) - PATTERN - - def_node_matcher :plural_translation_method?, <<~PATTERN - (send nil? :n_ str $dstr ...) - PATTERN - - def on_send(node) - interpolation = translation_method?(node) || plural_translation_method?(node) - return unless interpolation - - interpolation.descendants.each do |possible_violation| - add_offense(possible_violation, message: MSG) if possible_violation.type != :str - end - end - end - end -end diff --git a/rubocop/cop/search/namespaced_class.rb b/rubocop/cop/search/namespaced_class.rb new file mode 100644 index 00000000000..8824107ae61 --- /dev/null +++ b/rubocop/cop/search/namespaced_class.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Search + # Cop that enforces use of Search namespace for search related code. + # + # @example + # # bad + # class MySearchClass + # end + # + # # good + # module Search + # class MySearchClass + # end + # end + class NamespacedClass < RuboCop::Cop::Base + MSG = 'Search related code must be declared inside Search top level namespace. For more info: https://gitlab.com/gitlab-org/gitlab/-/issues/398207' + + # 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 + + SEARCH_REGEXES = [ + /elastic/i, + /zoekt/i, + /search/i + ].freeze + + def on_module(node) + add_identifiers(node) + + run_search_namespace_cop(node) if node.child_nodes.none? { |n| n.module_type? || n.class_type? } + end + + def on_class(node) + add_identifiers(node) + run_search_namespace_cop(node) + end + + private + + def run_search_namespace_cop(node) + add_offense(node.loc.name) if !namespace_allowed? && namespace_search_related? + end + + def add_identifiers(node) + identifiers.concat(identifiers_for(node)) + end + + def identifiers + @identifiers ||= [] + end + + def identifiers_for(node) + source = node.respond_to?(:identifier) ? node.identifier.source : node.source + source.sub(/^::/, '').split('::') + end + + def namespace_allowed? + PERMITTED_NAMESPACES.any? do |namespaces| + identifiers.first(namespaces.size) == namespaces + end + end + + def namespace_search_related? + SEARCH_REGEXES.any? { |x| x.match?(identifiers.join('::')) } + end + end + end + end +end diff --git a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb index 7fe308d6b40..badd81ff138 100644 --- a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb +++ b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb @@ -28,36 +28,29 @@ module RuboCop HELP_LINK = 'https://docs.gitlab.com/ee/development/sidekiq/worker_attributes.html#job-data-consistency-strategies' - MSG = <<~MSG + MISSING_DATA_CONSISTENCY_MSG = <<~MSG Should define data_consistency expectation. - - It is encouraged for workers to use database replicas as much as possible by declaring - data_consistency to use the :delayed or :sticky modes. Mode :always will result in the - worker always hitting the primary database node for both reads and writes, which limits - scalability. - - Some guidelines: - - 1. If your worker mostly writes or reads its own writes, use mode :always. TRY TO AVOID THIS. - 2. If your worker performs mostly reads and can tolerate small delays, use mode :delayed. - 3. If your worker performs mostly reads but cannot tolerate any delays, use mode :sticky. - 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." + def_node_search :application_worker?, <<~PATTERN `(send nil? :include (const nil? :ApplicationWorker)) PATTERN - def_node_search :data_consistency_defined?, <<~PATTERN - `(send nil? :data_consistency ...) + def_node_matcher :data_consistency_value, <<~PATTERN + `(send nil? :data_consistency $(sym _) ...) PATTERN def on_class(node) - return unless in_worker?(node) && application_worker?(node) - return if data_consistency_defined?(node) + return unless application_worker?(node) + + consistency = data_consistency_value(node) + return add_offense(node, message: MISSING_DATA_CONSISTENCY_MSG) if consistency.nil? - add_offense(node) + add_offense(consistency, message: DISCOURAGE_ALWAYS_MSG) if consistency.value == :always end end end |