summaryrefslogtreecommitdiff
path: root/rubocop
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2022-06-20 11:10:13 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2022-06-20 11:10:13 +0000
commit0ea3fcec397b69815975647f5e2aa5fe944a8486 (patch)
tree7979381b89d26011bcf9bdc989a40fcc2f1ed4ff /rubocop
parent72123183a20411a36d607d70b12d57c484394c8e (diff)
downloadgitlab-ce-0ea3fcec397b69815975647f5e2aa5fe944a8486.tar.gz
Add latest changes from gitlab-org/gitlab@15-1-stable-eev15.1.0-rc42
Diffstat (limited to 'rubocop')
-rw-r--r--rubocop/cop/gitlab/namespaced_class.rb2
-rw-r--r--rubocop/cop/migration/background_migrations.rb28
-rw-r--r--rubocop/cop/migration/migration_record.rb8
-rw-r--r--rubocop/cop/static_translation_definition.rb121
-rw-r--r--rubocop/formatter/todo_formatter.rb46
-rw-r--r--rubocop/rubocop-migrations.yml39
-rw-r--r--rubocop/rubocop-usage-data.yml1
-rw-r--r--rubocop/todo_dir.rb63
8 files changed, 226 insertions, 82 deletions
diff --git a/rubocop/cop/gitlab/namespaced_class.rb b/rubocop/cop/gitlab/namespaced_class.rb
index 914cc8720b9..ce46e055c21 100644
--- a/rubocop/cop/gitlab/namespaced_class.rb
+++ b/rubocop/cop/gitlab/namespaced_class.rb
@@ -35,7 +35,7 @@ module RuboCop
# class Gitlab::MyDomain::MyClass
# end
class NamespacedClass < RuboCop::Cop::Cop
- MSG = 'Classes must be declared inside a module indicating a product domain namespace. For more info: https://gitlab.com/gitlab-org/gitlab/-/issues/212156'
+ MSG = 'Classes must be declared inside a module indicating a product domain namespace. For more info: https://gitlab.com/gitlab-org/gitlab/-/issues/321982'
# These namespaces are considered top-level semantically.
# Note: Nested namespace like Foo::Bar are also supported.
diff --git a/rubocop/cop/migration/background_migrations.rb b/rubocop/cop/migration/background_migrations.rb
new file mode 100644
index 00000000000..39c5496e4d3
--- /dev/null
+++ b/rubocop/cop/migration/background_migrations.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+ module Cop
+ module Migration
+ class BackgroundMigrations < RuboCop::Cop::Cop
+ include MigrationHelpers
+
+ MSG = 'Background migrations are deprecated. Please use a Batched Background Migration instead.'\
+ ' More info: https://docs.gitlab.com/ee/development/database/batched_background_migrations.html'
+
+ def on_send(node)
+ name = node.children[1]
+
+ disabled_methods = %i(
+ queue_background_migration_jobs_by_range_at_intervals
+ requeue_background_migration_jobs_by_range_at_intervals
+ migrate_in
+ )
+
+ add_offense(node, location: :selector) if disabled_methods.include? name
+ end
+ end
+ end
+ end
+end
diff --git a/rubocop/cop/migration/migration_record.rb b/rubocop/cop/migration/migration_record.rb
index bb81b3cbaf1..291644f10e3 100644
--- a/rubocop/cop/migration/migration_record.rb
+++ b/rubocop/cop/migration/migration_record.rb
@@ -11,7 +11,7 @@ module RuboCop
ENFORCED_SINCE = 2022_04_26_00_00_00
MSG = <<~MSG
- Don't inherit from ActiveRecord::Base but use MigrationRecord instead.
+ Don't inherit from ActiveRecord::Base or ApplicationRecord but use MigrationRecord instead.
See https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html#example-usage-of-activerecord-classes.
MSG
@@ -19,9 +19,13 @@ module RuboCop
(class _ (const (const _ :ActiveRecord) :Base) _)
PATTERN
+ def_node_search :inherits_from_application_record?, <<~PATTERN
+ (class _ (const _ :ApplicationRecord) _)
+ PATTERN
+
def on_class(node)
return unless relevant_migration?(node)
- return unless inherits_from_active_record_base?(node)
+ return unless inherits_from_active_record_base?(node) || inherits_from_application_record?(node)
add_offense(node, location: :expression)
end
diff --git a/rubocop/cop/static_translation_definition.rb b/rubocop/cop/static_translation_definition.rb
index 3475a2b3dca..121b0c18770 100644
--- a/rubocop/cop/static_translation_definition.rb
+++ b/rubocop/cop/static_translation_definition.rb
@@ -2,63 +2,118 @@
module RuboCop
module Cop
+ # This cop flags translation definitions in static scopes because changing
+ # locales has no effect and won't translate this text again.
+ #
+ # See https://docs.gitlab.com/ee/development/i18n/externalization.html#keep-translations-dynamic
+ #
+ # @example
+ #
+ # # bad
+ # class MyExample
+ # # Constant
+ # Translation = _('A translation.')
+ #
+ # # Class scope
+ # field :foo, title: _('A title')
+ #
+ # validates :title, :presence, message: _('is missing')
+ #
+ # # Memoized
+ # def self.translations
+ # @cached ||= { text: _('A translation.') }
+ # end
+ #
+ # included do # or prepended or class_methods
+ # self.error_message = _('Something went wrong.')
+ # end
+ # end
+ #
+ # # good
+ # class MyExample
+ # # Keep translations dynamic.
+ # Translation = -> { _('A translation.') }
+ # # OR
+ # def translation
+ # _('A translation.')
+ # end
+ #
+ # field :foo, title: -> { _('A title') }
+ #
+ # validates :title, :presence, message: -> { _('is missing') }
+ #
+ # def self.translations
+ # { text: _('A translation.') }
+ # end
+ #
+ # included do # or prepended or class_methods
+ # self.error_message = -> { _('Something went wrong.') }
+ # end
+ # end
+ #
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."
+ MSG = <<~TEXT.tr("\n", ' ')
+ Translation is defined in static scope.
+ Keep translations dynamic. See https://docs.gitlab.com/ee/development/i18n/externalization.html#keep-translations-dynamic
+ TEXT
- TRANSLATION_METHODS = %i[_ s_ n_].freeze
+ RESTRICT_ON_SEND = %i[_ s_ n_].freeze
- def_node_matcher :translation_method?, <<~PATTERN
- (send _ _ str*)
- PATTERN
+ # List of method names which are not considered real method definitions.
+ # See https://api.rubyonrails.org/classes/ActiveSupport/Concern.html
+ NON_METHOD_DEFINITIONS = %i[class_methods included prepended].to_set.freeze
- def_node_matcher :lambda_node?, <<~PATTERN
- (send _ :lambda)
- PATTERN
-
- def_node_matcher :struct_constant_assignment?, <<~PATTERN
- (casgn _ _ `(const _ :Struct))
+ def_node_matcher :translation_method?, <<~PATTERN
+ (send _ {#{RESTRICT_ON_SEND.map(&:inspect).join(' ')}} str*)
PATTERN
def on_send(node)
return unless translation_method?(node)
- method_name = node.children[1]
- return unless TRANSLATION_METHODS.include?(method_name)
-
- translation_memoized = false
+ static = true
+ memoized = false
node.each_ancestor do |ancestor|
- receiver, _ = *ancestor
- break if lambda_node?(receiver) # translations defined in lambda nodes should be allowed
-
- if constant_assignment?(ancestor) && !struct_constant_assignment?(ancestor)
- add_offense(node, location: :expression)
+ memoized = true if memoized?(ancestor)
+ if dynamic?(ancestor, memoized)
+ static = false
break
end
+ end
+
+ add_offense(node) if static
+ end
- translation_memoized = true if memoization?(ancestor)
+ private
- if translation_memoized && class_method_definition?(ancestor)
- add_offense(node, location: :expression)
+ def memoized?(node)
+ node.type == :or_asgn
+ end
- break
- end
- end
+ def dynamic?(node, memoized)
+ lambda_or_proc?(node) ||
+ named_block?(node) ||
+ instance_method_definition?(node) ||
+ unmemoized_class_method_definition?(node, memoized)
end
- private
+ def lambda_or_proc?(node)
+ node.lambda_or_proc?
+ end
- def constant_assignment?(node)
- node.type == :casgn
+ def named_block?(node)
+ return unless node.block_type?
+
+ !NON_METHOD_DEFINITIONS.include?(node.method_name) # rubocop:disable Rails/NegateInclude
end
- def memoization?(node)
- node.type == :or_asgn
+ def instance_method_definition?(node)
+ node.type == :def
end
- def class_method_definition?(node)
- node.type == :defs
+ def unmemoized_class_method_definition?(node, memoized)
+ node.type == :defs && !memoized
end
end
end
diff --git a/rubocop/formatter/todo_formatter.rb b/rubocop/formatter/todo_formatter.rb
index 14b4242063e..662cc1551ff 100644
--- a/rubocop/formatter/todo_formatter.rb
+++ b/rubocop/formatter/todo_formatter.rb
@@ -14,13 +14,6 @@ module RuboCop
# For example, this formatter stores offenses for `RSpec/VariableName`
# in `.rubocop_todo/rspec/variable_name.yml`.
class TodoFormatter < BaseFormatter
- # Disable a cop which exceeds this limit. This way we ensure that we
- # don't enable a cop by accident when moving it from
- # .rubocop_todo.yml to .rubocop_todo/.
- # We keep the cop disabled if it has been disabled previously explicitly
- # via `Enabled: false` in .rubocop_todo.yml or .rubocop_todo/.
- MAX_OFFENSE_COUNT = 15
-
class Todo
attr_reader :cop_name, :files, :offense_count
@@ -41,12 +34,20 @@ module RuboCop
end
end
- def initialize(output, options = {})
- directory = options.delete(:rubocop_todo_dir) || TodoDir::DEFAULT_TODO_DIR
+ DEFAULT_BASE_DIRECTORY = File.expand_path('../../.rubocop_todo', __dir__)
+
+ class << self
+ attr_accessor :base_directory
+ end
+
+ self.base_directory = DEFAULT_BASE_DIRECTORY
+
+ def initialize(output, _options = {})
+ @directory = self.class.base_directory
@todos = Hash.new { |hash, cop_name| hash[cop_name] = Todo.new(cop_name) }
@todo_dir = TodoDir.new(directory)
- @config_inspect_todo_dir = load_config_inspect_todo_dir(directory)
- @config_old_todo_yml = load_config_old_todo_yml(directory)
+ @config_inspect_todo_dir = load_config_inspect_todo_dir
+ @config_old_todo_yml = load_config_old_todo_yml
check_multiple_configurations!
super
@@ -71,10 +72,21 @@ module RuboCop
end
end
+ def self.with_base_directory(directory)
+ old = base_directory
+ self.base_directory = directory
+
+ yield
+ ensure
+ self.base_directory = old
+ end
+
private
+ attr_reader :directory
+
def relative_path(path)
- parent = File.expand_path('..', @todo_dir.directory)
+ parent = File.expand_path('..', directory)
path.delete_prefix("#{parent}/")
end
@@ -84,7 +96,7 @@ module RuboCop
yaml << '# Cop supports --auto-correct.' if todo.autocorrectable?
yaml << "#{todo.cop_name}:"
- if previously_disabled?(todo) && offense_count_exceeded?(todo)
+ if previously_disabled?(todo)
yaml << " # Offense count: #{todo.offense_count}"
yaml << ' # Temporarily disabled due to too many offenses'
yaml << ' Enabled: false'
@@ -99,10 +111,6 @@ module RuboCop
yaml.join("\n")
end
- def offense_count_exceeded?(todo)
- todo.offense_count > MAX_OFFENSE_COUNT
- end
-
def check_multiple_configurations!
cop_names = @config_inspect_todo_dir.keys & @config_old_todo_yml.keys
return if cop_names.empty?
@@ -121,7 +129,7 @@ module RuboCop
config['Enabled'] == false
end
- def load_config_inspect_todo_dir(directory)
+ def load_config_inspect_todo_dir
@todo_dir.list_inspect.each_with_object({}) do |path, combined|
config = YAML.load_file(path)
combined.update(config) if Hash === config
@@ -130,7 +138,7 @@ module RuboCop
# Load YAML configuration from `.rubocop_todo.yml`.
# We consider this file already old, obsolete, and to be removed soon.
- def load_config_old_todo_yml(directory)
+ def load_config_old_todo_yml
path = File.expand_path(File.join(directory, '../.rubocop_todo.yml'))
config = YAML.load_file(path) if File.exist?(path)
diff --git a/rubocop/rubocop-migrations.yml b/rubocop/rubocop-migrations.yml
index a98a059df1d..ccde12bca77 100644
--- a/rubocop/rubocop-migrations.yml
+++ b/rubocop/rubocop-migrations.yml
@@ -1,49 +1,70 @@
# Make sure to update the docs if this file moves. Docs URL: https://docs.gitlab.com/ee/development/migration_style_guide.html#when-to-use-the-helper-method
Migration/UpdateLargeTable:
Enabled: true
- HighTrafficTables: &high_traffic_tables # size in GB (>= 10 GB on GitLab.com as of 02/2020) and/or number of records
+ HighTrafficTables: &high_traffic_tables # size in GB (>= 10 GB on GitLab.com as of 06/2022) and/or number of records
+ - :alert_management_alerts
+ - :approval_merge_request_rules_users
- :audit_events
- - :ci_build_trace_sections
+ - :authentication_events
+ - :ci_build_needs
+ - :ci_build_report_results
- :ci_builds
- :ci_builds_metadata
+ - :ci_build_trace_metadata
- :ci_job_artifacts
- - :ci_pipeline_variables
+ - :ci_pipeline_messages
- :ci_pipelines
+ - :ci_pipelines_config
+ - :ci_pipeline_variables
- :ci_stages
- :deployments
+ - :description_versions
+ - :error_tracking_error_events
- :events
- :gitlab_subscriptions
+ - :gpg_signatures
- :issues
+ - :label_links
+ - :lfs_objects
+ - :lfs_objects_projects
- :members
+ - :merge_request_cleanup_schedules
- :merge_request_diff_commits
- :merge_request_diff_files
- :merge_request_diffs
- :merge_request_metrics
- :merge_requests
- - :namespace_settings
- :namespaces
+ - :namespace_settings
- :note_diff_files
- :notes
+ - :packages_package_files
- :project_authorizations
- - :projects
- :project_ci_cd_settings
- - :project_settings
+ - :project_daily_statistics
- :project_features
+ - :projects
+ - :project_settings
- :protected_branches
- :push_event_payloads
- :resource_label_events
+ - :resource_state_events
- :routes
- :security_findings
- :sent_notifications
- :system_note_metadata
- :taggings
- :todos
- - :users
- - :user_preferences
+ - :uploads
- :user_details
+ - :user_preferences
+ - :users
+ - :vulnerabilities
+ - :vulnerability_occurrence_identifiers
+ - :vulnerability_occurrence_pipelines
- :vulnerability_occurrences
+ - :vulnerability_reads
- :web_hook_logs
- - :vulnerabilities
DeniedMethods:
- :change_column_type_concurrently
- :rename_column_concurrently
diff --git a/rubocop/rubocop-usage-data.yml b/rubocop/rubocop-usage-data.yml
index a5d74a1b570..8d8966144f1 100644
--- a/rubocop/rubocop-usage-data.yml
+++ b/rubocop/rubocop-usage-data.yml
@@ -123,3 +123,4 @@ UsageData/InstrumentationSuperclass:
- :GenericMetric
- :RedisHLLMetric
- :RedisMetric
+ - :NumbersMetric
diff --git a/rubocop/todo_dir.rb b/rubocop/todo_dir.rb
index 4aca4454a06..57b9895a925 100644
--- a/rubocop/todo_dir.rb
+++ b/rubocop/todo_dir.rb
@@ -6,22 +6,36 @@ require 'active_support/inflector/inflections'
module RuboCop
# Helper class to manage file access to RuboCop TODOs in .rubocop_todo directory.
class TodoDir
- DEFAULT_TODO_DIR = File.expand_path('../.rubocop_todo', __dir__)
+ # Suffix a TODO file.
+ SUFFIX_YAML = '.yml'
# Suffix to indicate TODOs being inspected right now.
SUFFIX_INSPECT = '.inspect'
- attr_reader :directory
-
+ # Instantiates a TodoDir.
+ #
+ # @param directory [String] base directory where all TODO YAML files are written to.
+ # @param inflector [ActiveSupport::Inflector, #underscore] an object which supports
+ # converting a string to its underscored version.
def initialize(directory, inflector: ActiveSupport::Inflector)
@directory = directory
@inflector = inflector
end
- def read(cop_name, suffix = nil)
- read_suffixed(cop_name)
+ # Reads content of TODO YAML for given +cop_name+.
+ #
+ # @param cop_name [String] name of the cop rule
+ #
+ # @return [String, nil] content of the TODO YAML file if it exists
+ def read(cop_name)
+ path = path_for(cop_name)
+
+ File.read(path) if File.exist?(path)
end
+ # Saves +content+ for given +cop_name+ to TODO YAML file.
+ #
+ # @return [String] path of the written TODO YAML file
def write(cop_name, content)
path = path_for(cop_name)
@@ -31,6 +45,10 @@ module RuboCop
path
end
+ # Marks a TODO YAML file for inspection by renaming the original TODO YAML
+ # and appending the suffix +.inspect+ to it.
+ #
+ # @return [Boolean] +true+ a file was marked for inspection successfully.
def inspect(cop_name)
path = path_for(cop_name)
@@ -42,38 +60,47 @@ module RuboCop
end
end
+ # Marks all TODO YAML files for inspection.
+ #
+ # @return [Integer] number of renamed YAML TODO files.
+ #
+ # @see inspect
def inspect_all
- pattern = File.join(@directory, '**/*.yml')
+ pattern = File.join(@directory, "**/*#{SUFFIX_YAML}")
Dir.glob(pattern).count do |path|
FileUtils.mv(path, "#{path}#{SUFFIX_INSPECT}")
end
end
+ # Returns a list of TODO YAML files which are marked for inspection.
+ #
+ # @return [Array<String>] list of paths
+ #
+ # @see inspect
+ # @see inspect_all
def list_inspect
- pattern = File.join(@directory, "**/*.yml.inspect")
+ pattern = File.join(@directory, "**/*#{SUFFIX_YAML}#{SUFFIX_INSPECT}")
Dir.glob(pattern)
end
+ # Deletes a list of TODO yaml files which were marked for inspection.
+ #
+ # @return [Integer] number of deleted YAML TODO files.
+ #
+ # @see #inspect
+ # @see #inspect_all
def delete_inspected
- pattern = File.join(@directory, '**/*.yml.inspect')
-
- Dir.glob(pattern).count do |path|
+ list_inspect.count do |path|
File.delete(path)
end
end
private
- def read_suffixed(cop_name, suffix = nil)
- path = path_for(cop_name, suffix)
-
- File.read(path) if File.exist?(path)
- end
-
- def path_for(cop_name, suffix = nil)
- todo_path = "#{@inflector.underscore(cop_name)}.yml#{suffix}"
+ def path_for(cop_name)
+ todo_path = "#{@inflector.underscore(cop_name)}#{SUFFIX_YAML}"
File.join(@directory, todo_path)
end